Closed Bug 1674829 Opened 4 years ago Closed 4 years ago

Video control should not listen to `MozNoControlsBlockedVideo`

Categories

(Toolkit :: Video/Audio Controls, defect)

defect

Tracking

()

RESOLVED FIXED
85 Branch
Tracking Status
firefox85 --- fixed

People

(Reporter: alwu, Assigned: alwu)

References

Details

Attachments

(2 files)

Currenly NoControlsMobileImplWidget is listening to MozNoControlsBlockedVideo event, but there is actually no such event.

In media element, the event that we would dispatch for mobile when media gets blocked is MozAutoplayMediaBlocked, that is what video control should listen to.

[1] https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/toolkit/content/widgets/videocontrols.js#2801
[2] https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/dom/html/HTMLMediaElement.cpp#4352-4356

Summary: Video control should listen to `MozAutoplayMediaBlocked`, not `MozNoControlsBlockedVideo` → Video control should not listen to `MozNoControlsBlockedVideo`

In our old code, we would listen to MozAutoplayMediaBlocked and convert it to another event MozNoControlsBlockedVideo for video control. But that code is no longer existing (test also got removed :( )

Jared, do you have any idea about that? I wonder if we have any other code doing same stuff? Because MozAutoplayMediaBlocked is only dispatching to chrome code, so we are not able to listen to that on videocontrols.js which runs in content.

Thank you.

[1] https://hg.mozilla.org/integration/autoland/rev/7f1d96b0004f

Flags: needinfo?(jaws)

Does Fenix / Android Components actually still use this component for mobile video controls?

It looks like there's other dead code there, e.g. https://searchfox.org/mozilla-central/rev/16d30bafd4e5276d6d3c632fb52a6c71e739cc44/toolkit/content/widgets/videocontrols.js#2460

Is there a tracker of migrating the video integration from Fennec to Fenix and what features are still used etc. ?

(In reply to Alastor Wu [:alwu] from comment #1)

Jared, do you have any idea about that? I wonder if we have any other code doing same stuff? Because MozAutoplayMediaBlocked is only dispatching to chrome code, so we are not able to listen to that on videocontrols.js which runs in content.

What does "same stuff" mean here?

Flags: needinfo?(kbrosnan)
Flags: needinfo?(alwu)

The problem here is that the click-to-play button of video control on mobile apparently has been broken for a while, because no one is doing the event convertion (MozAutoplayMediaBlocked -> MozNoControlsBlockedVideo). I didn't know the implementation details for video control, but I suppose this videocontrols.js would be run on mobile as well.

"The same stuffs" here means the event convertion, but I didn't know the reason why we used that way. If we want to let that work, we have to allow video control to receive the block event from media element.

Flags: needinfo?(alwu)

Looks like Sebastian has worked on the media Android-component. Redirecting.

Flags: needinfo?(kbrosnan) → needinfo?(s.kaspari)

This was the best I could find,
https://hg.mozilla.org/mozilla-central/rev/dc1cbf8e88a9

I don't know of anything else changing the event name and redispatching it.

Flags: needinfo?(jaws)

Hey all. I just wanted to highlight that this bug causes a significant amount of reports on webcompat.com.
The problem that people experience in Fenix is that they visit websites and see black boxes where videos should be, with no way to interact with them. This situation occurs when autoplay is blocked and the website is set to hide the video UI.

Example cases are https://www.womenshealthmag.com/fitness/a19982843/best-leg-exercises/ and https://dropdead.world/

I am willing to help on this but I need more information from Fenix/GeckoView team.

  1. the original way to use CastingApps.js to convert the event MozNoControlsBlockedVideo to MozNoControlsBlockedVideo and then dispatch that to media control component. I don't understand the purpose of CastingApps, and don't know if we need this again.

  2. If we don't need that, how to dispatch a customized event thatt only video control can receive? Because videocontrol.js is in the content process, so dispatching an chrome-only event is not working.

(In reply to Alastor Wu [:alwu] from comment #7)

  1. If we don't need that, how to dispatch a customized event thatt only video control can receive? Because videocontrol.js is in the content process, so dispatching an chrome-only event is not working.

Can't we dispatch a non-chrome-only event on the video controls? It looks like that's what we do for resizevideocontrols, and it doesn't look like the webpage gets access to those events.

Flags: needinfo?(alwu)

If possible, I would like to avoid dispatching non-specced events. That would be my last option, so I would like to know if there is any other solution.

Flags: needinfo?(alwu)

(In reply to Alastor Wu [:alwu] from comment #9)

If possible, I would like to avoid dispatching non-specced events.

We were already dispatching non-specced events, but they were only visible to our code. The net result from my suggestion would be the same - we're dispatching into the (closed) shadow root, so AIUI the event would never be visible to web code if we didn't do the equivalent of setting composed: true. Why would this be objectionable?

That would be my last option, so I would like to know if there is any other solution.

You could build a C++ implementation of EventTarget (or a similar callback mechanism) into a UA-widget-exposed webidl interface such that only the controls code can add/remove listeners, and you could dispatch something to that event target instead of the DOM. But that seems overly complex compared to what I suggested without a clear benefit. Ultimately, the goal here appears to be to push information from the platform layer to the UA widget privileged code so I don't see a way of doing that without some kind of interface - we've used events in the past, and it still seems like the most obvious route.

Flags: needinfo?(alwu)

(In reply to :Gijs (he/him) from comment #10)

We were already dispatching non-specced events, but they were only visible to our code. The net result from my suggestion would be the same - we're dispatching into the (closed) shadow root, so AIUI the event would never be visible to web code if we didn't do the equivalent of setting composed: true.

I don't know well how video control handles events from video element, so that is what I was looking for. If we can guarantee that the event won't be visible to web code, then I would totally agree for this way.

So the solution would be that we make media element dispatch a normal event MozNoControlsBlockedVideo and simply let video control listen to that. In addition, I can copy the old test [1] for this event which got removed unexpectedly when we abandoned Fennec. Do you know where the good place is to put this test?

Thank you.

[1] https://hg.mozilla.org/integration/autoland/file/7f1d96b0004f2d12a131dbf1116cda7bf3410dfb/mobile/android/tests/browser/chrome/test_mozAutoplayMediaBlocked.html

Flags: needinfo?(alwu) → needinfo?(gijskruitbosch+bugs)

(In reply to Alastor Wu [:alwu] from comment #11)

(In reply to :Gijs (he/him) from comment #10)

We were already dispatching non-specced events, but they were only visible to our code. The net result from my suggestion would be the same - we're dispatching into the (closed) shadow root, so AIUI the event would never be visible to web code if we didn't do the equivalent of setting composed: true.

I don't know well how video control handles events from video element, so that is what I was looking for.

So to be clear, I'm suggesting dispatching the event on the video controls node and/or shadowroot, which isn't web-visible

If we can guarantee that the event won't be visible to web code, then I would totally agree for this way.

https://jsbin.com/sotetumoqa/edit?html,js,console,output

AFAICT the event isn't visible outside of the closed shadow root unless you specify composed: true (you can try editing the jsbin).

So the solution would be that we make media element dispatch a normal event MozNoControlsBlockedVideo and simply let video control listen to that. In addition, I can copy the old test [1] for this event which got removed unexpectedly when we abandoned Fennec. Do you know where the good place is to put this test?

Probably somewhere under https://searchfox.org/mozilla-central/source/toolkit/content/tests/ ? But I don't know much about our mobile testing.

Thank you.

[1] https://hg.mozilla.org/integration/autoland/file/7f1d96b0004f2d12a131dbf1116cda7bf3410dfb/mobile/android/tests/browser/chrome/test_mozAutoplayMediaBlocked.html

Flags: needinfo?(gijskruitbosch+bugs)

Thanks for explanation, I didn't have experience with event handling on shadow dom, what you share is definitely something good to know.

Ok, I will put the test under toolkit/content/tests/ and only run if when platform is android. However, I'm not able to run mochitest on either my device (pixel5) or emulator, still trying to get it work.

(In reply to Alastor Wu [:alwu] from comment #13)

However, I'm not able to run mochitest on either my device (pixel5) or emulator, still trying to get it work.

Not sure if you're running into the same thing I did - geckoview in my emulator somehow struggled with network access, and allowing data roaming in the emulator (!?) made it work... But I'm not a geckoview/fenix expert, perhaps the folks in the relevant matrix channel can help. :-)

Attached file Bug 1674829 - part2 : add test. (deleted) —

Depends on D97505

Flags: needinfo?(s.kaspari)
Pushed by alwu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3a1bbbea1aa7 part1 : dispatch 'MozNoControlsBlockedVideo' to video control. r=Gijs https://hg.mozilla.org/integration/autoland/rev/507e844790c8 part2 : add test. r=Gijs
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 85 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: