1487143 - Click-to-play icon doesn't show on Fennec video control

Status: VERIFIED FIXED

(Toolkit :: Video/Audio Controls, defect, P1)

Description

[Alastor Wu [:alwu]]

Click-to-play icon (which was added in bug1166961) should appear when the autoplay media which has no showing control interface was blocked.

---

STR.
1. go to "Settings" >> "Advanced" >> "Allow autoplay" and change the setting to "Block autoplay"
2. visit https://alastor0325.github.io/htmltests/autoplay_tests/autoplay_test2.html

Expect.
3. show click-to-play icon 

Actual.
4. no click-to-play icon

Comment 1

[Jared Wein [:jaws] (please needinfo? me)]

Brindusa, can you help find someone to narrow down the regression window for this?

Comment 2

[Daniel Bodea [:danibodea]]

Hello,

I am attempting to obtain a regression range for this issue, but it seems that only the Nightly63 browser version has the "For websites that autoplay sound" preference in the browser's options, in which case, the regression cannot be performed.

I can only confirm that it reproduces in Nightly (63.0a1 - 2018-08-30) and that a "click-to-play" icon is not being displayed, but I do not know whether there should really be a button since there isn't one on most of the other browsers including Chrome.

If this button really is missing, then it may have never been implemented when this preference was put in the browser's options ("For websites that autoplay sound" preference), sometime in Nightly63.

If there is another way I could obtain a regression range, tell me how and I'll do it. Maybe there is a pref that I could change in older versions of the nightly browser so that I could turn off the auto-playing of videos?

Thank you!

Comment 3

[Alastor Wu [:alwu]]

(In reply to Bodea Daniel [:danibodea] from comment #2)
> Hello,
> 
> I am attempting to obtain a regression range for this issue, but it seems
> that only the Nightly63 browser version has the "For websites that autoplay
> sound" preference in the browser's options, in which case, the regression
> cannot be performed.
> 
> I can only confirm that it reproduces in Nightly (63.0a1 - 2018-08-30) and
> that a "click-to-play" icon is not being displayed, but I do not know
> whether there should really be a button since there isn't one on most of the
> other browsers including Chrome.
> 
> If this button really is missing, then it may have never been implemented
> when this preference was put in the browser's options ("For websites that
> autoplay sound" preference), sometime in Nightly63.
> 
> If there is another way I could obtain a regression range, tell me how and
> I'll do it. Maybe there is a pref that I could change in older versions of
> the nightly browser so that I could turn off the auto-playing of videos?
> 
> Thank you!

Set the pref "media.autoplay.enabled=false", and then you can disable autoplay.

Comment 4

[Daniel Bodea [:danibodea]]

I have used the pref above (set as false) and tried to find an older version of the browser that displays a "click-to-play" icon on the video provided in the description.
I can confidently say that this is not a regression. I went as far back as Nightly v55 and Release v55 and a "click-to-play" icon is not being displayed at all. 

Furthermore, Release 55 version does not even play the video at all, while the "failed count" is working normally (when a "click-to-play" icon should be displayed). Older versions have even worse behaviours that make trying to find a "good version" very difficult. 

In conclusion, I still insist that this is not a regression, but a misdesign or bad original implementation at most. 
If you agree, please change the Importance from "Normal" to "Enhancement" and remove the regression related tags.
If you still think this is a regression and need some more help investigating the issue, tell me how I can help.

Thank you!

Comment 5

[:Gijs (out for now; he/him)]

[Tracking Requested - why for this release]:

Even if this isn't a regression per se, the pref is only user-visible as of 63, so ideally we should fix this as part of the 63 release.

Alastor, is this a bug you can investigate?

Comment 6

[Alastor Wu [:alwu]]

I'm not working on video-control, maybe jaws know who can help with?

Comment 7

[Tim Guan-tin Chien [:timdream] (please needinfo)]

I really can't make sure something is still working while it is already broken, so I guess I would have to fix this.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: patch

Upon opening the JS Debugger and probe into videocontrols.xml, I realized it never receives MozAutoplayMediaBlocked event, because CastingApps._blocked weak map has the key set at the document node, not the video element.

It takes aEvent.target, which should have been the element, not the document.

https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/mobile/android/chrome/content/CastingApps.js#194

The original patch in bug 1166961 had the target property correctly set.

https://hg.mozilla.org/mozilla-central/rev/dc1cbf8e88a9#l1.14

Alaster, I don't have a full Fennec build environment available, so could you test this patch and see if the button reappears? Perhaps you can take the bug by complete this patch (which should have been part of bug 1485189) with a test case?

Thank you so much!

Comment 9

[Alastor Wu [:alwu]]

Hi, Tim,

I've tried your patch but the there is still no button appears.

In addition, I'm wondering if we can add this button on desktop as well, it could help the bug1489291.

Comment 10

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Thanks for trying. I'll prepare my local build environment then.

I don't think we can do bug 1489291 right now without talos regression on desktop YouTube (because we will be running extra JS on <video> without controls). We'll need to think about what to do there.

Comment 11

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: Bug 1487143 - Properly dispatch MozAutoplayMediaBlocked event to content, r=alwu

The MozAutoplayMediaBlocked event should have its target set to the video
element, not the document.

Also, MozNoControlsBlockedVideo event has to initialized from the CustomEvent
constructor of the right window for the XBL binding to access it. I don't know
when it stopped working.

Test is added to ensure the entire UI won't break.

Comment 12

[Tim Guan-tin Chien [:timdream] (please needinfo)]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f06926e23d8aade846994178250687af0561a40

Comment 13

[Alastor Wu [:alwu]]

Comment on attachment 9008875 [details]
Bug 1487143 - Properly dispatch MozAutoplayMediaBlocked event to content, r=alwu

Alastor Wu [:alwu] has approved the revision.

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f1d96b0004f
Properly dispatch MozAutoplayMediaBlocked event to content, r=alwu

Comment 15

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/7f1d96b0004f

Comment 16

[Daniel Bodea [:danibodea]]

Hello Tim Guan-tin Chien,

I have attempted to verify this fix, but it appears invalid. The Click-to-play icon still isn't displayed on the video, indifferently to the set value of the "media.autoplay.enabled.user-gestures-needed" pref.

Can you look into it?

Comment 17

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Make sure you don't run into bug 1491134 when doing the tests. Let me know if you still couldn't reproduce it.

Comment 18

[Daniel Bodea [:danibodea]]

Hey Tim. I could never see the "click-to-play" icon. Are there any other prefs I need to change in order to see the button?
What am I missing here?

Comment 19

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Attachment: image.png

Here is what I did.

1. With the phone attached and debug turned on, run

./mach mozregression --launch 2018-09-18 -n fennec

This command will download the build and launch it with a new profile.

2. Go to the page in comment 0

and I can see the click-to-play icon at the center of the black blank video frame.

There is no setting to turn on because autoplay is turned on by default on nightly.

Comment 20

[Bogdan Surd, QA [:bsurd, NI]]

Devices:
 - Xiaomi Mi4i (Android 5.0.2);
 - OnePlus 5T (Android 8.1.0);
 - Nexus 5 (Android 6.0.1).

Verified using the instructions in Comment 19 & Comment 0, a click-to-play icon is being displayed for the video on all of the devices.

Checked on FF Desktop and the icon was not available as Daniel mentioned. Seeing as this fix was meant for Android I am marking this as verified.

Comment 21

[Julien Cristau [:jcristau]]

Please request uplift to beta63 when you get a chance

Comment 22

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 9008875 [details]
Bug 1487143 - Properly dispatch MozAutoplayMediaBlocked event to content, r=alwu

Approval Request Comment
[Feature/Bug causing the regression]: regression, bug 1463919 and other changes
[User impact if declined]: There won't be a click-to-play button on the video element when the video is blocked.
[Is this code covered by automated tests?]: Yes
[Has the fix been verified in Nightly?]: Yes
[Needs manual test from QE? If yes, steps to reproduce]: Tested already, see comment 19
[List of other uplifts needed for the feature/fix]: no. bug 1485189 already in Beta 63
[Is the change risky?]: No
[Why is the change risky/not risky?]: Localized and unlikely to cause side effects
[String changes made/needed]: None

Comment 23

[Pascal Chevrel:pascalc]

Comment on attachment 9008875 [details]
Bug 1487143 - Properly dispatch MozAutoplayMediaBlocked event to content, r=alwu

Functional fix on a P1 bug, approved for 63 beta, thanks.

Comment 24

[Dorel Luca [:dluca]]

https://hg.mozilla.org/releases/mozilla-beta/rev/34cfa5617f98

Comment 25

[Bogdan Surd, QA [:bsurd, NI]]

Device:
 - Samsung Galaxy Note 8 (Android 8.0)

Verified in 63.0b13. The click-to-play icon is displayed over the black blank video frame.