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

VERIFIED FIXED in Firefox 33

Status

()

Firefox
Theme
--
major
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: florian, Assigned: dao)

Tracking

({regression})

Trunk
Firefox 34
regression
Points:
1
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox33 verified, firefox34 verified)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Created attachment 8458711 [details]
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.
(Assignee)

Comment 1

4 years ago
Created attachment 8458722 [details] [diff] [review]
patch
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #8458722 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Updated

4 years ago
Iteration: --- → 33.3
Points: --- → 1
Component: General → Theme
Flags: firefox-backlog+
OS: Mac OS X → All
Hardware: x86 → All
Summary: WebRTC toolbar sharing indicator looking broken on non-retina Mac → WebRTC sharing indicator on the toolbar doesn't get the right image region applied (except for hi-DPI OS X)

Updated

4 years ago
QA Whiteboard: [qa?]
(Assignee)

Updated

4 years ago
QA Whiteboard: [qa?] → [qa+]
Duplicate of this bug: 1041504

Updated

4 years ago
Iteration: 33.3 → 34.1
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1041837

Comment 4

4 years ago
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

4 years ago
(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)
Flags: needinfo?(dao)

Updated

4 years ago
status-firefox33: --- → affected
status-firefox34: --- → affected

Comment 6

4 years ago
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.
Attachment #8458722 - Flags: review?(gijskruitbosch+bugs) → review+
(Reporter)

Updated

4 years ago
Duplicate of this bug: 1042023
(Reporter)

Comment 8

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

Comment 9

4 years ago
(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.
Flags: needinfo?(dao)
(Assignee)

Comment 11

4 years ago
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
Attachment #8458722 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/0ed96b26d1bd
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 34
Hi Florin, can a QA contact be assigned for verification of this bug.
Flags: needinfo?(florin.mezei)
status-firefox34: affected → fixed
Attachment #8458722 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Flags: needinfo?(florin.mezei)
QA Contact: bogdan.maris
(Assignee)

Comment 14

4 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/bd29e4e81633
status-firefox33: affected → fixed
Depends on: 1043949
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.
Status: RESOLVED → VERIFIED
status-firefox33: fixed → verified
status-firefox34: fixed → verified
QA Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.