1040810 - WebRTC sharing indicator on the toolbar doesn't get the right image region applied (except for hi-DPI OS X)

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[Florian Quèze [:florian]]

Attachment: screenshot

See attached screenshot.

The cause of the regression is very likely bug 1037017.

The code at
http://mxr.mozilla.org/mozilla-central/source/browser/themes/shared/toolbarbuttons.inc.css?force=1#151
151 #webrtc-status-button:not(@inAnyPanel@) {
152   -moz-image-region: rect(0, 594px, 18px, 576px);
153 }

becomes this after it's preprocessed:
#webrtc-status-button:not(:-moz-any(:not([cui-areatype="toolbar"]), [overflowedItem=true])) {
  -moz-image-region: rect(0, 594px, 18px, 576px);
}

Which means the rule image-region rule won't be applied if the webrtc status toolbar button doesn't have the cui-areatype="toolbar" attribute.

Comment 1

[Dão Gottwald [:dao]]

Attachment: patch

Comment 4

[:Gijs (out for now; he/him)]

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

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

::: browser/themes/shared/toolbarbuttons.inc.css
@@ +147,5 @@
>  #zoom-controls:not(@inAnyPanel@) > #zoom-in-button {
>    -moz-image-region: rect(0, 576px, 18px, 558px);
>  }
>  
> +#webrtc-status-button {

Err, surely this by itself is enough here? Why all the Linux changes?

Comment 5

[:Gijs (out for now; he/him)]

(In reply to :Gijs Kruitbosch (PTO july 17-21) from comment #4)
> Comment on attachment 8458722 [details] [diff] [review]
> patch
> 
> Review of attachment 8458722 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: browser/themes/shared/toolbarbuttons.inc.css
> @@ +147,5 @@
> >  #zoom-controls:not(@inAnyPanel@) > #zoom-in-button {
> >    -moz-image-region: rect(0, 576px, 18px, 558px);
> >  }
> >  
> > +#webrtc-status-button {
> 
> Err, surely this by itself is enough here? Why all the Linux changes?

(well, that plus removing the bogus webrtc rule on Linux - but the other rules should probably be fixed in a separate bug, rather than assuming we can live without them, especially because this will want uplifting to 33)

Comment 6

[:Gijs (out for now; he/him)]

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

Getting this out of my queue for my post-PTO catchup.

rs=me for the toolbarbuttons.inc.css change and this:

-#webrtc-status-button {
-  -moz-image-region: rect(0px 192px 24px 168px);
-}
-

in browser.css.

Please provide a new bug/patch + rationale for the other bits if necessary.

Comment 8

[Florian Quèze [:florian]]

I thought this bug was Mac-only, but bug 1042023 is a report on Windows. When verifying, QA should look at all supported OSes.

Comment 9

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch (PTO july 17-21) from comment #6)
> Comment on attachment 8458722 [details] [diff] [review]
> patch
> 
> Getting this out of my queue for my post-PTO catchup.
> 
> rs=me for the toolbarbuttons.inc.css change and this:
> 
> -#webrtc-status-button {
> -  -moz-image-region: rect(0px 192px 24px 168px);
> -}
> -
> 
> in browser.css.
> 
> Please provide a new bug/patch + rationale for the other bits if necessary.

It's dead code just like the #webrtc-status-button rule.

Comment 10

[Dão Gottwald [:dao]]

https://hg.mozilla.org/integration/fx-team/rev/0ed96b26d1bd

Comment 11

[Dão Gottwald [:dao]]

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

Approval Request Comment
[Feature/regressing bug #]: bug 1037017
[User impact if declined]: see attachment 8458711 [details]
[Describe test coverage new/current, TBPL]: theme code, no test coverage
[Risks and why]: trivial CSS, no risk
[String/UUID change made/needed]: none

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/0ed96b26d1bd

Comment 13

[Marco Mucci [:MarcoM]]

Hi Florin, can a QA contact be assigned for verification of this bug.

Comment 14

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/bd29e4e81633

Comment 15

[Bogdan Maris, Desktop Test Engineering]

Reproduced the issue on Nightly (2014-07-18), verified that the issue is fixed on Windows 7 64bit, Mac OS X 10.9.4 and Ubuntu 13.04 64bit using latest Nightly and latest Aurora.