Bug 220581 – Facebook pauses video in PiP during scroll

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

Facebook pauses video in PiP during scroll

[Jer Noble]

<rdar://67273166>

[Jer Noble]

Created attachment 417514 [details]
Patch

[Sam Weinig]

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?

[Jer Noble]

(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.

[Kate Cheney]

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).

[Jer Noble]

(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.

[Jer Noble]

Created attachment 417553 [details]
Patch

[EWS]

Committed r271470: <https://trac.webkit.org/changeset/271470>

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