Closed
Bug 1183700
Opened 9 years ago
Closed 9 years ago
Don't prevent playback of media elements in a muted tab on desktop
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
mozilla42
| Tracking | Status | |
|---|---|---|
| firefox42 | --- | fixed |
People
(Reporter: ehsan.akhgari, Assigned: baku)
References
Details
Attachments
(1 file)
|
(deleted),
patch
|
ehsan.akhgari
:
review+
|
Details | Diff | Splinter Review |
STR:
1. Apply my UI patches in bug 486262.
2. Go to a tab with an audio element, start playing it, and mute the tab.
3. Navigate to a new page, and play a new audio element. The element remains paused, but it should keep making progress.
What is happening is because HTMLMediaElement::WindowVolumeChanged() prevents the playback of the element if aMuted. Instead, we need to just mute the audio and let the playback proceed as normal.
We should also have a test for this, something like browser_mediaPlayback.js and browser_mute.js.
|
Assignee | |
Comment 1•9 years ago
|
||
Attachment #8634155 -
Flags: review?(ehsan)
|
|
Assignee | |
Comment 2•9 years ago
|
||
The previous patch does this:
1. expose mozinterruptbegin/end only when AudioChannelAPI is enabled
2. Window refresh works only with outer window
3. testing for all the desktop tab/muting features
4. expose computedVolume/computedMuted for testing
|
Reporter | |
Comment 3•9 years ago
|
||
Comment on attachment 8634155 [details] [diff] [review]
desktop.patch
Review of attachment 8634155 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/base/nsGlobalWindow.cpp
@@ +3738,5 @@
> void
> nsPIDOMWindow::RefreshMediaElements()
> {
> nsRefPtr<AudioChannelService> service = AudioChannelService::GetOrCreate();
> + service->RefreshAgentsVolume(this);
Why are you passing this? I think you should pass GetOuterWindow(), which I did in bug 1183356. Please just remove this hunk.
::: dom/html/HTMLMediaElement.cpp
@@ +4481,5 @@
> DispatchAsyncEvent(NS_LITERAL_STRING("mozinterruptend"));
> }
> }
>
> +#ifdef MOZ_B2G
I wonder if MOZ_B2G is the right condition here.. Please watch out for mulet. I honestly don't know what MOZ_B2G means these days!
@@ +4782,5 @@
> +
> +bool
> +HTMLMediaElement::ComputedMuted() const
> +{
> + return mMuted;
Did you need to & with MUTED_BY_AUDIO_CHANNEL?
::: toolkit/content/tests/browser/browser_mediaPlayback_mute.js
@@ +8,5 @@
> });
> }
>
> +function get_audio_element(browser) {
> + var list = browser.contentDocument.getElementsByTagName('audio');
This is not very e10s friendly, even though it might work because of the existing CPOW shims. I asked mconley, and the better way to do this is to use ContentTask.spawn(). That will let you run a function in the context of the child process.
::: toolkit/content/tests/browser/file_mediaPlayback2.html
@@ +1,2 @@
> <!DOCTYPE html>
> +<body><div id="content"></div><body>
Please remove this. document.body exists even without this!
Attachment #8634155 -
Flags: review?(ehsan) → review+
|
|
Assignee | |
Comment 4•9 years ago
|
||
> Please remove this. document.body exists even without this!
not really. It doesn't work for the iframe.
Pushed on try.
|
|
Reporter | |
Comment 5•9 years ago
|
||
(In reply to Andrea Marchesini (:baku) from comment #4)
> > Please remove this. document.body exists even without this!
>
> not really. It doesn't work for the iframe.
OK, no big deal if you keep it!
|
|
Assignee | |
Comment 6•9 years ago
|
||
|
|
Assignee | |
Comment 7•9 years ago
|
||
|
||
Comment 8•9 years ago
|
||
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox42:
--- → fixed
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
Depends on: 1186225
|
||
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•