Closed Bug 1231364 Opened 4 years ago Closed 4 years ago

Sound indicator not displayed on multiple videos opened once

Categories

(Firefox :: Tabbed Browser, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 45
Tracking Status
firefox43 + wontfix
firefox44 + wontfix
firefox45 + verified
firefox46 --- verified

People

(Reporter: bogdan_maris, Assigned: baku)

References

Details

(Keywords: regression)

Attachments

(2 files)

Affected builds:
- Firefox 43.0RC build 1
- latest Developer Edition 44.0a2
- latest Nightly 45.0a1

Affected OS's:
- Windows 7 64-bit
- Windows 10 32-bit
- Mac OS X 10.9.5
- Ubuntu 14.04 32-bit

STR:
1. Start Firefox
2. Open any youtube video (eg: https://www.youtube.com/watch?v=7LcUOEP7Brc)
3. Pin the tab
4. Open some more videos in new tabs from the pined tab

Expected results: All the other videos opened have the sound indicator.

Actual results: Not all the opened video have the indicator displayed, none of the other do. This depends on how fast you open the other videos.

Notes:
1. This is a regression since this issue does not reproduce on old Nightly 2015-08-10, will post a regression range ASAP.
2. Gif showing the issue attached
Can't attach the gif to bug because of it's size. Added to dropbox: https://db.tt/2MEgHLJa
You can skip the step 3.
Simple STR:
1. Open youtube.com
2. Open 2 videos in new tabs with middle click button
AR: the sound indicators are displayed for 1-2 sec while the videos are loading, then disappear.
Summary: Sound indicator not displayed on videos opened from a pinned tab → Sound indicator not displayed on multiple videos opened once
Assignee: nobody → amarchesini
Noting this works for me in 43.0b9 (OS X 10.10.5). 
Tracking for 43+ since this sounds like a regression.
Ehsan can you have a look? We don't have a regression window yet but this seems like one of your areas of expertise.
Flags: needinfo?(ehsan)
I see the issue in 43 RC build 1 (https://ftp.mozilla.org/pub/firefox/candidates/43.0-candidates/build1) in  OS X 10.10.5.   

May be a regression from bug 1214624 ?
Flags: needinfo?(amarchesini)
Talking baku now.  

Also, noting I don't see this happening every time and not on every tab. I agree with Cornel, this doesn't need to block release. We could potentially back out the patch or fix the issue with the planned dot release.
Flags: needinfo?(ehsan)
Attached patch ac2.patchSplinter Review
It turned out that tabbrowser.xml calls onLocationChange recursively, also when the location is not actually changed. Here a patch that does a check based on the URI.
Flags: needinfo?(amarchesini)
Attachment #8697135 - Flags: review?(ehsan)
Attachment #8697135 - Flags: review?(ehsan) → review+
Liz, do we want to have this patch in aurora and beta?
https://hg.mozilla.org/mozilla-central/rev/6ba1161b0fe1
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Flags: needinfo?(lhenry)
It is too late for the 43 RC which went to build 2 days ago.   If we can verify the fix, please request uplift for aurora, beta, and release (since potentially we could take this for the dot release next week)
Flags: needinfo?(lhenry) → qe-verify+
Since the latest update (to 43) I see no sound indicator at all, anywhere, in any case. And it looks like I am not alone:

https://www.reddit.com/r/firefox/comments/3w8ajl/mute_tab_icon_gone_setting_to_true_doesnt_bring/

Should this be new bug, or it is the same as this one? People in discussion speculate that this might be related to Flash.
(In reply to User Dderss from comment #13)
> Since the latest update (to 43) I see no sound indicator at all, anywhere,
> in any case. And it looks like I am not alone:
> 
> https://www.reddit.com/r/firefox/comments/3w8ajl/
> mute_tab_icon_gone_setting_to_true_doesnt_bring/
> 
> Should this be new bug, or it is the same as this one? People in discussion
> speculate that this might be related to Flash.
bug 1231244
Baku, Ehsan, could you please provide a patch for uplift to m-r, m-b and m-a? It seems like this is a key scenario that we ought to fix in 43 dot release, Beta44 and Aurora45. Thanks!
Flags: needinfo?(ehsan)
Flags: needinfo?(amarchesini)
I can still reproduce the problem on 46.0a1 (2015-12-16), Win 7.
Screencast: https://drive.google.com/file/d/0By0_Tw9EIIVvTXFrOFFiUlZDYkE/view?usp=sharing
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
I think Andrea is going to take a look.
Flags: needinfo?(ehsan)
Too late for 43 at this point. We could still take a patch for 44, I think.
Andrew, this is a key scenario that is broken at the moment and I hope we can get a fix in Beta44 soon. Any help is appreciated.
Flags: needinfo?(overholt)
I spoke with baku about it and he's going to look into this tomorrow.
Flags: needinfo?(overholt)
Attached patch ac3.patchSplinter Review
nsIURI is different but the spec is the same.
Flags: needinfo?(amarchesini)
Attachment #8705089 - Flags: review?(ehsan)
Attachment #8705089 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/4226516b9b52
Status: REOPENED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
I have reproduced this bug on Nightly 45.0a1 (2015-12-09) on ubuntu 14.04 LTS, 32 bit!

The bug's fix is now verified on Latest Nightly 46.0a1!

Build ID: 20160112030227
User Agent: Mozilla/5.0 (X11; Linux i686; rv:46.0) Gecko/20100101 Firefox/46.0
QA Whiteboard: [bugday-20160113]
Status: RESOLVED → VERIFIED
Baku, Andrew: Should we uplift this to Beta45? Too late for Fx44.
Flags: needinfo?(overholt)
Flags: needinfo?(amarchesini)
I'll let baku and/or ehsan make that call.
Flags: needinfo?(overholt) → needinfo?(ehsan)
Comment on attachment 8705089 [details] [diff] [review]
ac3.patch

Approval Request Comment
[Feature/regressing bug #]: Audio icon per tab
[User impact if declined]: the icon is not visible sometimes
[Describe test coverage new/current, TreeHerder]: no tests
[Risks and why]: none
[String/UUID change made/needed]: none
Flags: needinfo?(amarchesini)
Attachment #8705089 - Flags: approval-mozilla-aurora?
Attachment #8697135 - Flags: approval-mozilla-aurora?
Flags: needinfo?(ehsan)
Comment on attachment 8705089 [details] [diff] [review]
ac3.patch

I think this is already fixed on 46 and this request is for beta.
Attachment #8705089 - Flags: approval-mozilla-beta?
Attachment #8705089 - Flags: approval-mozilla-aurora?
Attachment #8705089 - Flags: approval-mozilla-aurora+
Comment on attachment 8705089 [details] [diff] [review]
ac3.patch

Let's take as it is a regression.

Should be in 45 beta 2.
Attachment #8705089 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Verified fixed on Windows 7 64bit, Ubuntu 13.10 32bit and Mac OSX 10.9.5 using Firefox 45 Beta 2 (buildID: 20160201143558).
Comment on attachment 8705089 [details] [diff] [review]
ac3.patch

This shouldn't need uplift, it's already in aurora 46. Fixing the flag.
Attachment #8705089 - Flags: approval-mozilla-aurora+ → approval-mozilla-aurora-
Comment on attachment 8697135 [details] [diff] [review]
ac2.patch

Looks like this needs beta uplift and is already in aurora 46.
Attachment #8697135 - Flags: approval-mozilla-beta?
Attachment #8697135 - Flags: approval-mozilla-aurora?
Attachment #8697135 - Flags: approval-mozilla-aurora-
Comment on attachment 8697135 [details] [diff] [review]
ac2.patch

Same for aurora, already landed.
Attachment #8697135 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.