Bug 211932

Summary: Add a quirk to allow an embedded Twitter video to play with one tapping
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: New BugsAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, mjs, philipj, sergio, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 248199    
Attachments:
Description Flags
Patch
none
Revise the patch based on Youenn's comment none

Description Peng Liu 2020-05-14 16:11:14 PDT 
Add a quirk to allow an embedded Twitter video to play with one tapping
Comment 1 Peng Liu 2020-05-14 16:12:35 PDT 
<rdar://problem/60604893>
Comment 2 Peng Liu 2020-05-14 16:23:31 PDT 
Created attachment 399426 [details]
Patch
Comment 3 Maciej Stachowiak 2020-05-14 23:01:51 PDT 
Comment on attachment 399426 [details]
Patch

Do we have any way to add tests for quirk? It would be better to add a test if at all possible.
Comment 4 youenn fablet 2020-05-15 07:49:26 PDT 
Comment on attachment 399426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399426&action=review

> Source/WebCore/html/MediaElementSession.cpp:319
> +    if (topDocument.hasHadUserInteraction() && document.quirks().shouldAutoplayForArbitraryUserGesture())

We should probably update MediaElementSession::updateMediaUsageIfChanged as well, since it is doing document.hasHadUserInteraction() && document.quirks().shouldAutoplayForArbitraryUserGesture() as well.

Maybe we should add a test for that document/topDocument change where the iframe did not interact but the top document (or another iframe) did.
That would require Internals to be able to set the document quirks shouldAutoplayForArbitraryUserGesture() value.
Comment 5 Peng Liu 2020-05-18 14:51:05 PDT 
Created attachment 399675 [details]
Revise the patch based on Youenn's comment
Comment 6 Peng Liu 2020-05-18 14:57:08 PDT 
(In reply to Maciej Stachowiak from comment #3)
> Comment on attachment 399426 [details]
> Patch
> 
> Do we have any way to add tests for quirk? It would be better to add a test
> if at all possible.

We don't have the support to test quirks for now. We may need a non-trivial patch to enable it. Filed a bug for it: <webkit.org/b/212047>.
Comment 7 Peng Liu 2020-05-18 15:00:00 PDT 
Comment on attachment 399426 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=399426&action=review

>> Source/WebCore/html/MediaElementSession.cpp:319
>> +    if (topDocument.hasHadUserInteraction() && document.quirks().shouldAutoplayForArbitraryUserGesture())
> 
> We should probably update MediaElementSession::updateMediaUsageIfChanged as well, since it is doing document.hasHadUserInteraction() && document.quirks().shouldAutoplayForArbitraryUserGesture() as well.
> 
Fixed it after confirmed with Eric.
> Maybe we should add a test for that document/topDocument change where the iframe did not interact but the top document (or another iframe) did.
> That would require Internals to be able to set the document quirks shouldAutoplayForArbitraryUserGesture() value.

Filed a bug to add the support to test quirks <webkit.org/b/212047>.
Comment 8 Maciej Stachowiak 2020-05-18 15:10:07 PDT 
Comment on attachment 399675 [details]
Revise the patch based on Youenn's comment

r=me
Comment 9 EWS 2020-05-18 16:40:10 PDT 
Committed r261839: <https://trac.webkit.org/changeset/261839>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399675 [details].