Facebook pauses video in PiP during scroll
<rdar://67273166>
Created attachment 417514 [details] Patch
Comment on attachment 417514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417514&action=review > Source/WebCore/ChangeLog:12 > + Add a Quirk which blocks Facebook from pausing videos in Picture-in-Picture mode without that > + pause() occurring during a User Gesture. This blocks Facebook from pausing a PiP'd video when > + the <video> element hosting that video scrolls out of the viewport, without blocking Facebook's > + own custom pause control from working correctly. Hey Jer, I've definitely seen this issue on there sites as well (I know this because I don't use Facebook and have seen this, but can't recall where). What are your thoughts on just making this the rule for all sites: pausing PIP requires a user gesture?
(In reply to Sam Weinig from comment #3) > Comment on attachment 417514 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417514&action=review > > > Source/WebCore/ChangeLog:12 > > + Add a Quirk which blocks Facebook from pausing videos in Picture-in-Picture mode without that > > + pause() occurring during a User Gesture. This blocks Facebook from pausing a PiP'd video when > > + the <video> element hosting that video scrolls out of the viewport, without blocking Facebook's > > + own custom pause control from working correctly. > > Hey Jer, I've definitely seen this issue on there sites as well (I know this > because I don't use Facebook and have seen this, but can't recall where). > What are your thoughts on just making this the rule for all sites: pausing > PIP requires a user gesture? I'm sympathetic. Since we added an "enter pip" button in the default controls (and similar affordance elsewhere), it's easy for sites to completely miss that the user has put the video into PiP. However, I can also definitely see pitfalls. We've evangelized our autoplay rules which state that once you've "unlocked" a media element with a user gesture, you don't need additional ones to change the play state. Sites with complicated React.js style re-dispatching event handlers might have unexpected behavior. It might still be worth it though.
Comment on attachment 417514 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=417514&action=review > Source/WebCore/page/Quirks.cpp:1269 > + m_requiresUserGestureToPauseInPictureInPicture = equalLettersIgnoringASCIICase(host, "facebook.com") || host.endsWithIgnoringASCIICase(".facebook.com"); If you wanted, I think you could do something like: auto domain = RegistrableDomain(m_document->topDocument().url()); m_requiresUserGestureToPauseInPictureInPicture = domain.string() == "facebook.com"_s; It would make it so you don't need the extra check for the subdomain case .facebook.com (RegistrableDomain will parse the domain for you).
(In reply to katherine_cheney from comment #5) > Comment on attachment 417514 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=417514&action=review > > > Source/WebCore/page/Quirks.cpp:1269 > > + m_requiresUserGestureToPauseInPictureInPicture = equalLettersIgnoringASCIICase(host, "facebook.com") || host.endsWithIgnoringASCIICase(".facebook.com"); > > If you wanted, I think you could do something like: > > auto domain = RegistrableDomain(m_document->topDocument().url()); > m_requiresUserGestureToPauseInPictureInPicture = domain.string() == > "facebook.com"_s; > > It would make it so you don't need the extra check for the subdomain case > .facebook.com (RegistrableDomain will parse the domain for you). Oooh, super glad someone finally made this utility class. Will adopt.
Created attachment 417553 [details] Patch
Committed r271470: <https://trac.webkit.org/changeset/271470> All reviewed patches have been landed. Closing bug and clearing flags on attachment 417553 [details].