1166961 - Show click to play button on Fennec when autoplay is blocked

Status: RESOLVED FIXED

(Core :: Audio/Video: Playback, defect)

Description

[Randall Barker [:rbarker]]

Re-enable the error overlay for video elements that do not have controls. Bug 821695 disabled the error message overlay for video elements that do not have controls. Additionally, when an autoplay video is blocked, a click to play overlay should be displayed.

Comment 1

[Randall Barker [:rbarker]]

Attachment: Click-to-play UI for blocked autoplay video elements.

Comment 2

[Randall Barker [:rbarker]]

Attachment: Restored error screen for video elements with out controls.

Comment 3

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Part-1-Re-enable-missing-video-UI-when-element-does-not-have-controls-15052616-1de94f2.patch

Comment 4

[Randall Barker [:rbarker]]

Attachment: Fennec click-to-play UI for blocked autoplay video elements.

Comment 5

[Randall Barker [:rbarker]]

Attachment: Fennec restored error screen for video elements with out controls.

Comment 6

[Randall Barker [:rbarker]]

Attachment: Fennec click-to-play UI for blocked autoplay video elements.

Updated with play button in correct location.

Comment 7

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Part-1-Re-enable-missing-video-UI-when-element-does-not-have-controls-15060215-47d3639.patch

Comment 8

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Re-enable-missing-video-UI-when-element-does-not-have-controls-15061116-556658c.patch

Comment 9

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Re-enable-missing-video-UI-when-element-does-not-have-controls-15061216-178330a.patch

Comment 10

[Justin Dolske [:Dolske]]

Comment on attachment 8621897 [details] [diff] [review]
0001-Bug-1166961-Re-enable-missing-video-UI-when-element-does-not-have-controls-15061216-178330a.patch

Review of attachment 8621897 [details] [diff] [review]:
-----------------------------------------------------------------

So, I'm generally pretty pessimistic about doing this. Trying to show our own click-to-play control over a media element where the page has its own controls feels like a hack that's not always going to work well, and so can still leave the user confused with weird UI. I'm open to doing _something_, but feels like there are a lot of problems with the current approach and is a big footgun.

Biggest non-starter is that YouTube and Vimeo are both severely broken...

Loading a YouTube video page given me a black video with no overlay button visible. Clicking the video shows you YT "pause" animation, clicking it again causes this overlay to finally show up + a YT "play" animation, clicking it a 3rd time makes the overlay go away + the video plays for a fraction of a second + the YT "pause" animation plays again, further clicks just trigger the YT "play" animation but nothing plays.

Loading a Vimeo page does not autoplay the video. Clicking the Vimeo "play" button works fine, but clicking the video itself just makes this overlay come up. Clicking the overlay's play button has no effect, I'm stuck with a video that won't play and a button that won't go away.

This is also going to be a little weird, but I suppose technically expected, for WebRTC (http://mozilla.github.io/webrtc-landing/gum_test.html). The user still has to click "play" on the "video", even after explicitly granting camera access.


Our own video controls and media features are also broken with the pref/overlay...

For <video controls> (without any autoplay), our usual CTP button is broken by the pref. Clicking it has no effect. (https://people.mozilla.org/~dolske/tmp/rickroll.html)

Speaking of WebRTC, the pref seems to break Hello as well. I get no video, and can't even click on it to make it start working.

::: b2g/chrome/content/content.css
@@ +91,5 @@
>  }
>  
> +/* Preven videocontrols.xml#noControls from being created for B2G */
> +video:not([controls]) > xul|videocontrols,
> +audio:not([controls]) > xul|videocontrols {

Why this is important to fix on Desktop & Android, but not B2G?

::: browser/base/content/tab-content.js
@@ +651,5 @@
>    }
>  };
>  DOMFullscreenHandler.init();
> +
> +let MozBlockAutoplayHandler = {

Seems like this could all go away if the media element had a mozAutoplayMediaBlocked property on it, so the binding could determine what it needs to do when it attaches.

In fact, should this be something generally addressed by the HTML spec? If this is a popular pref to enable, seems like a popular site (like YouTube) should be able to detect and handle this properly itself, without the browser always needing to do a hacky overlay.

(Of course not every site it going to handle it, so probably needs a clever solution where the browser can do a fallback UI only if needed.)

::: layout/style/html.css
@@ +739,5 @@
> +/* display errors and click to play when autoplay is blocked for videos */
> +video:not([controls]) > xul|videocontrols {
> +  visibility: visible;
> +  -moz-binding: url("chrome://global/content/bindings/videocontrols.xml#noControls");
> +}

This might be a significant performance issue for B2G -- see bug 821695.

::: toolkit/content/widgets/videocontrols.xml
@@ +1805,5 @@
>      </handlers>
>  
>    </binding>
> +
> +  <binding id="noControls">

This binding seems _far_ more complex than it should be. I'd expect this to do little more than show a clickable button that, when clicked, calls .play() and hides itself.

The comment in html.css says you want to show error messages -- why? This is the page's responsibility, because they're providing their own controls.

There's also a ton of dead code here. (As one example, this is setting up listeners for the |stalled| event to show a throbber, but there is no throbber in this binding.)

Comment 11

[Justin Dolske [:Dolske]]

Some of the above issues make me think bug 659285 should be backed out or made Nightly-only due to it causing lots of secondary problems.

Comment 12

[Justin Dolske [:Dolske]]

(In reply to Justin Dolske [:Dolske] from comment #10)
> I'm open to doing _something_,
> but feels like there are a lot of problems with the current approach and is
> a big footgun.

Along those lines...

This does feel like something that could be handled as a permission prompt, although it's not a perfect / natural fit. But I think it would be good to explore, as it seems like it's closer to working out.

As a quick and handwavy strawman: something like the mixed-content prompt. When the pref is enabled, a page attempting to autoplay would show an icon indicator in the url bar. Clicking that gives a permission prompt ("Firefox prevented a video on this page from automatically playing. Do you want to let it play?"), with options to Allow and Always Allow. Granting permission reloads the page, like plugin CTP, since that's the only way to ensure the page will be 100% functional.

Assumptions here are that (1) lots of autoplay annoyances are actually undesirable content, and so we don't want to irritate the user further by always showing the prompt and (2) an always-allow would be useful to whitelist sites where the user _does_ want autoplay to work.

Comment 13

[Randall Barker [:rbarker]]

There is a patch in review (bug 1173844) that stops blocking auto play after there has been human interaction with the element which appears to make things better but doesn't address all of your concerns. To implement your suggestions would require some help from front end developers. 

(In reply to Justin Dolske [:Dolske] from comment #10)
> Comment on attachment 8621897 [details] [diff] [review]
> 0001-Bug-1166961-Re-enable-missing-video-UI-when-element-does-not-have-
> controls-15061216-178330a.patch
> 
> Review of attachment 8621897 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> So, I'm generally pretty pessimistic about doing this. Trying to show our
> own click-to-play control over a media element where the page has its own
> controls feels like a hack that's not always going to work well, and so can
> still leave the user confused with weird UI. I'm open to doing _something_,
> but feels like there are a lot of problems with the current approach and is
> a big footgun.
> 

The main motivation for the click to play button on elements with no controls is for sites that convert animated GIF files to video. They typically do not have any UI. On android, since the video is not loaded until the video starts playing, there is no visual queue that there is something to see unless the user accidentally long presses on the element and selects play. Maybe just limit the click to play button to android?

> Biggest non-starter is that YouTube and Vimeo are both severely broken...
> 

The patch for bug 1173844 makes YouTube better but not fixed. The main problem I encountered on YouTube was when the ad would begin auto playing, the youtube player would not wait for the "playing" event to toggle the play/pause button so it would need to be clicked twice to cycle it through. After that things seemed to work because the video element was no longer being blocked.

> Loading a YouTube video page given me a black video with no overlay button
> visible. Clicking the video shows you YT "pause" animation, clicking it
> again causes this overlay to finally show up + a YT "play" animation,
> clicking it a 3rd time makes the overlay go away + the video plays for a
> fraction of a second + the YT "pause" animation plays again, further clicks
> just trigger the YT "play" animation but nothing plays.
> 
> Loading a Vimeo page does not autoplay the video. Clicking the Vimeo "play"
> button works fine, but clicking the video itself just makes this overlay
> come up. Clicking the overlay's play button has no effect, I'm stuck with a
> video that won't play and a button that won't go away.
> 
> This is also going to be a little weird, but I suppose technically expected,
> for WebRTC (http://mozilla.github.io/webrtc-landing/gum_test.html). The user
> still has to click "play" on the "video", even after explicitly granting
> camera access.
> 
> 
> Our own video controls and media features are also broken with the
> pref/overlay...
> 
> For <video controls> (without any autoplay), our usual CTP button is broken
> by the pref. Clicking it has no effect.
> (https://people.mozilla.org/~dolske/tmp/rickroll.html)
> 
> Speaking of WebRTC, the pref seems to break Hello as well. I get no video,
> and can't even click on it to make it start working.
> 

Disabling the autoplay attribute already breaks some things. My hope was to address what we could as it comes up as with bug 1173844.

> ::: b2g/chrome/content/content.css
> @@ +91,5 @@
> >  }
> >  
> > +/* Preven videocontrols.xml#noControls from being created for B2G */
> > +video:not([controls]) > xul|videocontrols,
> > +audio:not([controls]) > xul|videocontrols {
> 
> Why this is important to fix on Desktop & Android, but not B2G?
> 

Well since, B2G broke it for everyone for performance, I figured they would not want it re-enabled. I had originally only done this fix for Android but there was push back from users that the auto play ads annoy desktop users as well so I put in a little extra effort to get it to work just as poorly in desktop.

> ::: browser/base/content/tab-content.js
> @@ +651,5 @@
> >    }
> >  };
> >  DOMFullscreenHandler.init();
> > +
> > +let MozBlockAutoplayHandler = {
> 
> Seems like this could all go away if the media element had a
> mozAutoplayMediaBlocked property on it, so the binding could determine what
> it needs to do when it attaches.
> 
> In fact, should this be something generally addressed by the HTML spec? If
> this is a popular pref to enable, seems like a popular site (like YouTube)
> should be able to detect and handle this properly itself, without the
> browser always needing to do a hacky overlay.
> 
> (Of course not every site it going to handle it, so probably needs a clever
> solution where the browser can do a fallback UI only if needed.)
> 

I modeled my solution after screen sharing code already in videocontrols.xml. If there is a better way to do this, could you point me towards something that implements this in a way you would prefer? 

> ::: layout/style/html.css
> @@ +739,5 @@
> > +/* display errors and click to play when autoplay is blocked for videos */
> > +video:not([controls]) > xul|videocontrols {
> > +  visibility: visible;
> > +  -moz-binding: url("chrome://global/content/bindings/videocontrols.xml#noControls");
> > +}
> 
> This might be a significant performance issue for B2G -- see bug 821695.
> 

Which is why I disabled it for B2G (I think?).

> ::: toolkit/content/widgets/videocontrols.xml
> @@ +1805,5 @@
> >      </handlers>
> >  
> >    </binding>
> > +
> > +  <binding id="noControls">
> 
> This binding seems _far_ more complex than it should be. I'd expect this to
> do little more than show a clickable button that, when clicked, calls
> .play() and hides itself.
> 
> The comment in html.css says you want to show error messages -- why? This is
> the page's responsibility, because they're providing their own controls.
> 

As I stated above, there are video elements such as converted animated GIFs that have no controls and obviously don't provide an error message. If you feel that the error messages do more harm than good I can remove them.

> There's also a ton of dead code here. (As one example, this is setting up
> listeners for the |stalled| event to show a throbber, but there is no
> throbber in this binding.)

The throbber is part of the statusIcon which is part of this binding. It wouldn't surprise me that there is some dead code but I did try to remove as much as I could.

The preference to disable autoplay only has a public UI in Fennec. Desktop is only affect if people go into about:config and toggle it manually. Of course I would prefer to get it working in desktop as well. For fennec, the autoplay HTML5 video isn't just an annoyance but also a batter drain. With facebook and now twitter making it the default, I hope we can come up with a viable solution.

Comment 14

[Randall Barker [:rbarker]]

(In reply to Justin Dolske [:Dolske] from comment #12)
> (In reply to Justin Dolske [:Dolske] from comment #10)
> > I'm open to doing _something_,
> > but feels like there are a lot of problems with the current approach and is
> > a big footgun.
> 
> Along those lines...
> 
> This does feel like something that could be handled as a permission prompt,
> although it's not a perfect / natural fit. But I think it would be good to
> explore, as it seems like it's closer to working out.
> 
> As a quick and handwavy strawman: something like the mixed-content prompt.
> When the pref is enabled, a page attempting to autoplay would show an icon
> indicator in the url bar. Clicking that gives a permission prompt ("Firefox
> prevented a video on this page from automatically playing. Do you want to
> let it play?"), with options to Allow and Always Allow. Granting permission
> reloads the page, like plugin CTP, since that's the only way to ensure the
> page will be 100% functional.
> 
> Assumptions here are that (1) lots of autoplay annoyances are actually
> undesirable content, and so we don't want to irritate the user further by
> always showing the prompt and (2) an always-allow would be useful to
> whitelist sites where the user _does_ want autoplay to work.

Implementing some sort of indicator in the url bar with a message would need some front end intervention. :snorp recommended I bring android front end into this conversation.

Comment 15

[:Margaret Leibovic]

Sorry I've been so slow to get to this... can you summarize what the problem is and what your proposed solution is?

If we do something like a permission prompt, we could integrate this with the ever-evolving site identity popup, but we don't have room for additional notification icons on the left side of the urlbar, the way desktop does. We could also use a page action icon on the right side of the urlbar, but those usually correspond to immediate actions, so that doesn't feel right.

A icon in the urlbar also doesn't feel totally appropriate if this is about individual video elements in the content. It sounds like dolske had problems with implementing additional controls in the video content in the page, but I feel like that's actually our best option if we can find a way to make that work.

Comment 16

[Randall Barker [:rbarker]]

The patch I created does enable a click to play UI for the video elements. I guess the issue is that a lot of custom media player controls have their own click to play that cover the video element so it can look bad if the overlay has any transparency. I think the way an icon in url bar would work is it would serve as a visual indication that some media has been blocked on the page. Touching the icon would give the user an option to white list the site and potentially unbreak the site. For example YouTube is a little broken when the autoplay block is enabled (you have to click the play button twice).

Comment 17

[:Margaret Leibovic]

bbermes or antlam, can you help us figure out what to do about this bug?

Comment 18

[Anthony Lam (:antlam) Find me, @antlam on Slack!]

An overlay is fine in the situation. It may generate 1 more tap for users but enabling this pref comes with assumed expectations I think.

We should use the icons in bug 704229 rather than the older one's we're using here.

As for the overlay, we can just center the play icon, with a #363B40 (tabs tray grey) overlay, set at 80% opacity. The icon should be centered and have a max size at 25% of the video size (if it's really small the play button shouldn't take up the whole area)

Comment 19

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Show-click-to-play-button-on-Fennec-when-autoplay-is-blocked-15082816-3dfcc89.patch

Comment 20

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Show-click-to-play-button-on-Fennec-when-autoplay-is-blocked-15082816-31db28e.patch

Comment 21

[Randall Barker [:rbarker]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=1f05f868a057

Comment 22

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8654429 [details] [diff] [review]
0001-Bug-1166961-Show-click-to-play-button-on-Fennec-when-autoplay-is-blocked-15082816-31db28e.patch

I am fine with trying this out. I don't like adding code to BrowserApp for this though. Can we move to CastingApps or PluginHelper?

Comment 23

[Randall Barker [:rbarker]]

Attachment: 0001-Bug-1166961-Show-click-to-play-button-on-Fennec-when-autoplay-is-blocked-15091616-cb82fd7.patch

Address review comments. Cary forward r+ from :mfinkle

Comment 24

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/dc1cbf8e88a9

Comment 25

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/dc1cbf8e88a9