Closed Bug 1231364 Opened 4 years ago Closed 4 years ago
Sound indicator not displayed on multiple videos opened once
1.30 KB, patch
|Details | Diff | Splinter Review|
1.41 KB, patch
|Details | Diff | Splinter Review|
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
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.
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 ?
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.
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.
Attachment #8697135 - Flags: review?(ehsan)
Fastest patch in the West!
Liz, do we want to have this patch in aurora and beta?
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!
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.
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.
I spoke with baku about it and he's going to look into this tomorrow.
nsIURI is different but the spec is the same.
Attachment #8705089 - Flags: review?(ehsan)
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]
Baku, Andrew: Should we uplift this to Beta45? Too late for Fx44.
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
Attachment #8705089 - Flags: approval-mozilla-aurora?
Comment on attachment 8705089 [details] [diff] [review] ac3.patch I think this is already fixed on 46 and this request is for beta.
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.
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.