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

Peng Liu
Reported 2020-05-14 16:11:14 PDT
Add a quirk to allow an embedded Twitter video to play with one tapping
Attachments
Patch (4.22 KB, patch)
2020-05-14 16:23 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
Revise the patch based on Youenn's comment (5.12 KB, patch)
2020-05-18 14:51 PDT, Peng Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Peng Liu
Comment 1 2020-05-14 16:12:35 PDT
<rdar://problem/60604893>
Peng Liu
Comment 2 2020-05-14 16:23:31 PDT
Created attachment 399426 [details] Patch
Maciej Stachowiak
Comment 3 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.
youenn fablet
Comment 4 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.
Peng Liu
Comment 5 2020-05-18 14:51:05 PDT
Created attachment 399675 [details] Revise the patch based on Youenn's comment
Peng Liu
Comment 6 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>.
Peng Liu
Comment 7 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>.
Maciej Stachowiak
Comment 8 2020-05-18 15:10:07 PDT
Comment on attachment 399675 [details] Revise the patch based on Youenn's comment r=me
EWS
Comment 9 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].
Note You need to log in before you can comment on or make changes to this bug.