Bug 220887

Summary: Twitter PiP video pauses when scrolling
Product: WebKit Reporter: Peng Liu <peng.liu6>
Component: MediaAssignee: Peng Liu <peng.liu6>
Status: RESOLVED FIXED    
Severity: Normal CC: calvaris, cdumez, changseok, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, kondapallykalyan, philipj, sergio, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: iPhone / iPad   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=220939
Attachments:
Description Flags
WIP patch
none
WIP patch
none
WIP patch
none
Patch
eric.carlson: review+
Revise the patch based on Eric's comments none

Peng Liu
Reported 2021-01-22 20:18:29 PST
Twitter PiP video pauses when scrolling
Attachments
WIP patch (9.45 KB, patch)
2021-01-22 22:45 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
WIP patch (9.46 KB, patch)
2021-01-22 22:58 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
WIP patch (7.90 KB, patch)
2021-01-22 23:33 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
Patch (9.45 KB, patch)
2021-01-25 11:02 PST, Peng Liu 		eric.carlson: review+
DetailsFormatted DiffDiff
Revise the patch based on Eric's comments (9.55 KB, patch)
2021-01-25 12:16 PST, Peng Liu 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Peng Liu
Comment 1 2021-01-22 20:19:37 PST
<rdar://73369869>
Peng Liu
Comment 2 2021-01-22 22:45:24 PST
Created attachment 418214 [details] WIP patch
Peng Liu
Comment 3 2021-01-22 22:58:23 PST
Created attachment 418215 [details] WIP patch
Peng Liu
Comment 4 2021-01-22 23:33:00 PST
Created attachment 418218 [details] WIP patch
Peng Liu
Comment 5 2021-01-25 11:02:02 PST
Created attachment 418309 [details] Patch
Eric Carlson
Comment 6 2021-01-25 11:49:18 PST
Comment on attachment 418309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418309&action=review > Source/WebCore/html/MediaElementSession.cpp:279 > +SuccessOr<MediaPlaybackDenialReason> MediaElementSession::playbackPermitted(MediaPlaybackOperation operation) const As we discussed, I'm not happy with this enum. Please add a FIXME here and note the url of the bug you filed to fix this. > Source/WebCore/page/Quirks.cpp:1269 > + // Facebook and twitter will naively pause a <video> element that has scrolled out of the viewport, s/twitter/Twitter/ > Source/WebCore/page/Quirks.cpp:1276 > auto domain = RegistrableDomain(m_document->topDocument().url()); auto domain = RegistrableDomain(m_document->topDocument().url()).string(); m_requiresUserGestureToPauseInPictureInPicture = domain == "facebook.com"_s || domain == "twitter.com"_s;
Peng Liu
Comment 7 2021-01-25 12:16:45 PST
Created attachment 418318 [details] Revise the patch based on Eric's comments
Peng Liu
Comment 8 2021-01-25 12:18:47 PST
Comment on attachment 418309 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=418309&action=review >> Source/WebCore/html/MediaElementSession.cpp:279 >> +SuccessOr<MediaPlaybackDenialReason> MediaElementSession::playbackPermitted(MediaPlaybackOperation operation) const > > As we discussed, I'm not happy with this enum. Please add a FIXME here and note the url of the bug you filed to fix this. Filed bug 220939 to track the refactoring. >> Source/WebCore/page/Quirks.cpp:1269 >> + // Facebook and twitter will naively pause a <video> element that has scrolled out of the viewport, > > s/twitter/Twitter/ Fixed. >> Source/WebCore/page/Quirks.cpp:1276 >> auto domain = RegistrableDomain(m_document->topDocument().url()); > > auto domain = RegistrableDomain(m_document->topDocument().url()).string(); > m_requiresUserGestureToPauseInPictureInPicture = domain == "facebook.com"_s || domain == "twitter.com"_s; Good point! Fixed.
EWS
Comment 9 2021-01-25 16:58:09 PST
Committed r271870: <https://trac.webkit.org/changeset/271870> All reviewed patches have been landed. Closing bug and clearing flags on attachment 418318 [details].
Note You need to log in before you can comment on or make changes to this bug.