1183044 - [New Tab Page] Sound on pinned websites starts playing when just opening a new tab

Status: VERIFIED FIXED

(Core :: Web Audio, defect)

Description

[[:philipp]]

str in a new profile:
- pin "http://lotto.pch.com/" to the new tab page
- restart firefox
- open a new tab & after a few seconds the background audio of the website will start to play (in addition there are a number of other network requests in the background that you wouldn't expect from a static tile)

https://support.mozilla.org/en-US/questions/1071847

Comment 1

[Jefferson]

From what I can see, Firefox is not able to capture the canvas of that page to a thumbnail during the user's visit to the site. There is no corresponding thumbnail in the folder, a blank area appears on about:newtab, and the background image URL returns a file not found: moz-page-thumb://thumbnail?url=http%3A%2F%2Flotto.pch.com%2F 

On some subsequent visits to about:newtab, the Fiddler debugging proxy shows Firefox retrieving the page in the background, perhaps to try to generate the missing thumbnail, and during that process the (annoying) MP3 plays.

The error page for the moz-page-thumb URL references a specifically named .png file that does not exist. Duplicating an existing thumbnail file and giving it the name of the missing file seems to prevent the background retrieval of the site when visiting about:newtab.

I was searching around and if this was a thumbnail capture, the audio should have been muted: bug 759964, bug 879111, bug 870103. Can't imagine this was an intentional change.

Comment 2

[[:philipp]]

can i needinfo you here, because you worked on the other bugs mentioned in comment 1

Comment 3

[Drew Willcoxon :adw]

Yikes, this is terrible.  Thanks for filing!  I'll look into it.

Comment 4

[Drew Willcoxon :adw]

Andrea, Ehsan, looks like bug 923517 regressed this (in Firefox 27!).  The problem is that AudioContext::AudioContext calls aWindow->AddAudioContext(this) *before* creating mDestination.  The problem with that is that aWindow->AddAudioContext() mutes the context if the docshell doesn't allow media, but AudioContext::Mute can't actually mute anything if !mDestination.

According to the comment in AudioContext::AudioContext though, the ordering has to be that way even though it unknowingly caused this bug.

The part about the thumbnail not actually being created is a separate bug that I'll file.

Comment 5

[Drew Willcoxon :adw]

Attachment: patch

This fixes it, not sure whether it's the best way.

Comment 6

[(no longer active)]

Comment on attachment 8639549 [details] [diff] [review]
patch

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

Thanks for the fix, but I have a better idea to fix this in a slightly cleaner way, please see below.

Is this something that we can easily add a test for?  That would be super valuable.  One thing which might be useful is using the APIs that I added in bug 1180421 for detecting audio playback...

::: dom/media/webaudio/AudioContext.cpp
@@ +104,2 @@
>  {
>    aWindow->AddAudioContext(this);

I think a much cleaner way to fix this would be to make AddAudioContext return a boolean set to true when the AudioContext needs to be muted, and then capturing the return value here, create the destination, and then call Mute() if that value is true.  That way, you won't need this dummy member.

Comment 7

[Drew Willcoxon :adw]

Attachment: patch v2

I wrote a test but it didn't work because it looks like the media-playback notification is not broadcasted on AudioContext::Mute().  Mute() works by simply setting the destination's volume to zero.  I was looking at AudioDestinationNode.cppand InputMutedRunnable it looks like now there's a better-supported way of muting inputs that wasn't there when we hooked up docshell.allowMedia to Web Audio?

Comment 8

[(no longer active)]

Comment on attachment 8639995 [details] [diff] [review]
patch v2

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

Hmm, yeah, I can't think of a great way to test this...

Comment 9

[Pulsebot]

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

Comment 10

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #4)
> The part about the thumbnail not actually being created is a separate bug
> that I'll file.

bug 1188665

Comment 11

[Ryan VanderMeulen [:RyanVM]]

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

Comment 12

[Drew Willcoxon :adw]

Attachment: Aurora/Beta patch

Approval Request Comment
[Feature/regressing bug #]:
Bug 923517 (Firefox 27)

[User impact if declined]:
Tiles on about:newtab that don't have thumbnails trigger page loads in the hidden window so that thumbnails can be made for them.  If a page uses Web Audio to make noises, then the user will hear it when they open about:newtab.  If for some reason the thumbnail is not able to be captured, then the next time the user opens about:newtab, the thumbnail capture process will start again, so the user will hear the noise again.  If the thumbnail is never able to be captured, then the user will hear noise every time they open about:newtab.  Since the number of tiles shown on about:newtab depends on the window size, and about:newtab fetches thumbnails for tiles that aren't currently visible, the tile doesn't even need to be visible for this to happen.

But this bug has been around since Firefox 27.

[Describe test coverage new/current, TreeHerder]:
No automated tests because they'd be hard to write, but I tested this patch manually on all three branches.

[Risks and why]:
Low risk, the patch is small, and I manually tested it.

[String/UUID change made/needed]:
None

Comment 13

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8640650 [details] [diff] [review]
Aurora/Beta patch

It's too late in the beta cycle to take this fix. I think we're good to ship in this state as we've shipped 12 releases with this issue to date. Beta-

We can consider this fix for Aurora but given how long it has been in release I don't think there is a strong case to uplift this fix.

Comment 14

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8640650 [details] [diff] [review]
Aurora/Beta patch

Patch seems safe to uplift to Aurora as the fix has been in nightly for the past few days.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Philipp, could you please verify that the fix works as expected on a latest Nightly build? Thanks!

Comment 16

[[:philipp]]

yes, i can confirm that the issue is fixed in current nightly builds & my ears are thankful for that :-)

Comment 17

[Drew Willcoxon :adw]

Thanks again for filing this bug, philipp.

https://hg.mozilla.org/releases/mozilla-aurora/rev/600d06c75e2c

Comment 18

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked as it seems to be a regression. Please let me know if that is not the case.

Comment 19

[Gabriela [:gaby2300]]

I can repro using Windows 7 64 bit.
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:41.0) Gecko/20100101 Firefox/41.0
Build ID: 20150820142145

Comment 20

[Kazi Nuzhat Tasnem]

I have reproduced the bug with Firefox release 38.0.1(build id-20150513174244)on windows 7,64 bit with the instruction with the instructions from comment 0 

This bug's fix is verified in latest Firefox beta 42.0b3!

Build Id:        20151001142456  
User Agent:      Mozilla/5.0 (Windows NT 6.1; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0

[testday-20151002]

Comment 21

[Bolaram Paul [:ancon]]

This bug's fix is verified in latest Firefox beta 

Build Id:        20151001142456 
User Agent:      Mozilla/5.0 (Windows NT 6.3; WOW64; rv:42.0) Gecko/20100101 Firefox/42.0 

[testday-20151002]

Comment 22

[Mohammed Adam]

This Bug's fix is verified in latest Firefox Beta 42.0b7 

Build Id:  	20151015151621 
User Agent: Mozilla/5.0 (X11; Linux i686; rv:42.0) Gecko/20100101 Firefox/42.0 

[testday-20151016]

Comment 23

[preethi.dhinesh]

This Bug Fix is Verified  in Latest Firefox 42.0 Beta 7
[testday-20151016]

Comment 24

[Petruta Horea [:phorea], PTO until 8/22/25]

Marking as verified based on above comments.