1214624 - tab-sound-icon disappears when buffering is stopped despite during playback

Status: VERIFIED FIXED

(Firefox :: Tabbed Browser, defect)

Description

[Alice0775 White]

Reproduced on Windows7, Windows10 and Ubuntu14.04, Nightly44.0a1, Aurora43.a2.
It seems to be unaffected to Beta42.b6.

tab-sound-icon disappears when buffering is stopped(completed) 

Steps to reproduce:
1. Open https://videos.cdn.mozilla.net/uploads/mozillaorg/
2. Click "A different kind of browser.webm" link 
   --- observe, tab-sound-icon indicator

Actual results:
 tab-sound-icon suddenly disappear when buffering is stopped despite during playback

Comment 1

[Liz Henry (:lizzard) (relman/hg->git project)]

Tracking since this is a bug in a new feature. 
Ehsan, can you help find someone to take a look at this?

Comment 2

[(no longer active)]

Let me bisect to see what broke this.

Comment 3

[(no longer active)]

Bug 1193085 has regressed this.

Comment 4

[Liz Henry (:lizzard) (relman/hg->git project)]

Looks like this is still affecting 43 (I just tried it).  
Andrew is there someone on your team who can have a look?

Comment 5

[Andrea Marchesini [:baku]]

Attachment: audio.patch

Comment 6

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

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

This looks fine to me but I'd like to get a second opinion from Marco.

Comment 7

[Marco Bonardo [:mak]]

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

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

the new high-level location to do the removal sounds better than the old low-level one.

::: browser/base/content/tabbrowser.xml
@@ +756,5 @@
>                  if (this.mBrowser.userTypedClear > 0 ||
>                      ((aFlags & Ci.nsIWebProgressListener.LOCATION_CHANGE_ERROR_PAGE) &&
>                       aLocation.spec != "about:blank") ||
>                       aFlags && Ci.nsIWebProgressListener.LOCATION_CHANGE_SAME_DOCUMENT)
>                    this.mBrowser.userTypedValue = null;

please while here brace this if body, the indentation in this area is not enough to figure the flow at first sight...

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/72cf0adcfb27

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/72cf0adcfb27

Comment 10

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

Baku, do we need to uplift this to Aurora44?

Comment 11

[Jared Wein [:jaws] (please needinfo? me)]

Yes, this should be uplifted to 44. Ideally 43, but it's likely too late for that.

Comment 12

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

(In reply to Jared Wein [:jaws] (please needinfo? me) from comment #11)
> Yes, this should be uplifted to 44. Ideally 43, but it's likely too late for
> that.

Thanks Jared. Would you be able to nominate the patch for uplift to Aurora in that case and fill up the risk assessment and test coverage info?

Comment 13

[Jared Wein [:jaws] (please needinfo? me)]

Baku can handle that.

Comment 14

[Liz Henry (:lizzard) (relman/hg->git project)]

We still have beta 9. I'd love to have this fix in 43 even if it is last-minute.

Comment 15

[Andrea Marchesini [:baku]]

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

Approval Request Comment
[Feature/regressing bug #]: Audio icon per tab
[User impact if declined]: sometimes the icon is not shown when buffering stopped
[Describe test coverage new/current, TreeHerder]: none
[Risks and why]: We don't have a valid test for this patch but it's a very simple code move from OnStopRequest to OnLocationChanged.
[String/UUID change made/needed]: none

Comment 16

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

Alice0775 White, could you please verify this fix works as expected on a latest Nightly build? Thanks!

Comment 17

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

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

This fix has been on Nightly for almost a week, let's uplift to Aurora44.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

baku can you request uplift to beta also? Will this apply for 43?

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/3f3715d4f448

Comment 20

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-b2g44_v2_5/rev/3f3715d4f448

Comment 21

[Alice0775 White]

(In reply to Ritu Kothari (:ritu) from comment #16)
> Alice0775 White, could you please verify this fix works as expected on a
> latest Nightly build? Thanks!


I cannot reproduce the problem on Nightly45.0a1 and Aurora44.0a2 anymore.

https://hg.mozilla.org/mozilla-central/rev/31fc97d173b3e3da5de35d54018855e6c905787e
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Firefox/45.0 ID:20151203030226

https://hg.mozilla.org/releases/mozilla-aurora/rev/7aad7dee85151ba464c2a5d5aa9e522bad9287e3
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:44.0) Gecko/20100101 Firefox/44.0 ID:20151203004003

Comment 22

[Andrea Marchesini [:baku]]

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

Approval Request Comment
Read comment 15.
This patch should apply to m-b, but I didn't test it by myself. In case I can easily upload a new version for m-b.

Comment 23

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 8687668 [details] [diff] [review]
audio.patch

Minor fix for tab audio indicator, tested on aurora, please uplift to beta.

Comment 24

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/d183c57dc0be

Comment 25

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-release/rev/5ddc4e2c8cde

Comment 26

[Camelia Badau [:cbadau], Desktop Test Engineering]

I've tested on Windows 7 64bit, Mac OSX 10.9.5 and Ubuntu 13.04 32bit using Firefox 43 RC (buildID: 20151208100201) and I didn't see the issue. 

Although I have one mention here: I saw this issue on Windows 7 64bit using Firefox 43 RC on the following page: https://videos.cdn.mozilla.net/uploads/mozillaorg/ -> A different kind of browser.mp4 : sometimes, tab-sound-icon disappears when buffering is completed.