Bug 211932 - Add a quirk to allow an embedded Twitter video to play with one tapping
Summary: Add a quirk to allow an embedded Twitter video to play with one tapping
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Peng Liu
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-05-14 16:11 PDT by Peng Liu
Modified: 2020-05-18 16:40 PDT (History)
13 users (show)

See Also:


Attachments
Patch (4.22 KB, patch)
2020-05-14 16:23 PDT, Peng Liu
no flags Details | Formatted Diff | Diff
Revise the patch based on Youenn's comment (5.12 KB, patch)
2020-05-18 14:51 PDT, Peng Liu
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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].