Closed Bug 956491 Opened 7 years ago Closed 7 years ago

Don't use Toolbar-inverted.png for the tabbar on Win7 glass (without lwtheme)

Categories

(Firefox :: Theme, defect)

x86_64
Windows 7
defect
Not set
normal

Tracking

()

VERIFIED FIXED
Firefox 30
Tracking Status
firefox29 --- verified
firefox30 --- verified

People

(Reporter: ge3k0s, Assigned: dao)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [Australis:P3][good first verify][testday-20140509])

Attachments

(3 files)

New tab, list all tabs and panorama icons are logically not inverted in the tab bar, but all others AFAICT are inverted with Aero. Icons shouldn't be inverted in Win 7 Aero in the tab bar.
Whiteboard: [Australis:P4]
Summary: Some icons are inverted in the tab bar other not → Some icons are inverted in the tab bar others not
(In reply to Guillaume C. [:ge3k0s] from comment #0)
> New tab, list all tabs and panorama icons are logically not inverted in the
> tab bar, but all others AFAICT are inverted with Aero. Icons shouldn't be
> inverted in Win 7 Aero in the tab bar.

Why not? They are unreadable otherwise...
Without LWT theme it's absolutely readable on the aero fog. Buttons seem more out of place in white especially with tab text, new tab button and list all tabs arrow all dark coloured.
We need a decision here. Note that the Panorama icon is now missing when placed in the tabbar.
(In reply to Guillaume C. [:ge3k0s] from comment #3)
> We need a decision here. Note that the Panorama icon is now missing when
> placed in the tabbar.

https://bugzilla.mozilla.org/show_bug.cgi?id=961481

Assigning this to Stephen for a decision.
Assignee: nobody → shorlander
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Whiteboard: [Australis:P4] → [Australis:P3]
(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Guillaume C. [:ge3k0s] from comment #3)
> > We need a decision here. Note that the Panorama icon is now missing when
> > placed in the tabbar.
> 
> https://bugzilla.mozilla.org/show_bug.cgi?id=961481


Actually, that doesn't make sense. That's a 10.9 non-hidpi bug. Were you really talking about Windows 7?
Flags: needinfo?(ge3k0s)
(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > (In reply to Guillaume C. [:ge3k0s] from comment #3)
> > > We need a decision here. Note that the Panorama icon is now missing when
> > > placed in the tabbar.
> > 
> > https://bugzilla.mozilla.org/show_bug.cgi?id=961481
> 
> 
> Actually, that doesn't make sense. That's a 10.9 non-hidpi bug. Were you
> really talking about Windows 7?

Hrm, seems that although I updated:

http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/Toolbar-inverted.png?force=1

I didn't update

http://mxr.mozilla.org/mozilla-central/source/browser/themes/windows/Toolbar-inverted-aero.png?force=1

They should be identical. I've filed bug 961532.
Flags: needinfo?(ge3k0s)
(In reply to :Gijs Kruitbosch from comment #5)
> (In reply to :Gijs Kruitbosch from comment #4)
> > (In reply to Guillaume C. [:ge3k0s] from comment #3)
> > > We need a decision here. Note that the Panorama icon is now missing when
> > > placed in the tabbar.
> > 
> > https://bugzilla.mozilla.org/show_bug.cgi?id=961481
> 
> 
> Actually, that doesn't make sense. That's a 10.9 non-hidpi bug. Were you
> really talking about Windows 7?

Yeah it was about Win 7. ;)
Summary: Some icons are inverted in the tab bar others not → Choose to show the icons inverted or not in the tabbar
I'm not really sure what this bug is about with the meandering path between comment 0 and now.

Screenshot showing the problem?
Flags: needinfo?(shorlander)
(In reply to Justin Dolske [:Dolske] from comment #8)
> I'm not really sure what this bug is about with the meandering path between
> comment 0 and now.
> 
> Screenshot showing the problem?

The problem was, I will have to double check if this is still the case, that icons in the tabstrip are not consistently inverted.

We used to invert them on glass because they were illegible otherwise. Now that we have the fog I am not sure that is the right thing to do.
Flags: needinfo?(shorlander)
Hope this helps, it looks like all icons are inverted on the tab-bar, with the exception of:
* the New Tab button
* the List All Tabs button
* the text for the current zoom level
* the Left and Right arrows of tab overflow
This is a screenshot with the inverted icons disabled.
(In reply to Stephen Horlander [:shorlander] from comment #9)
> (In reply to Justin Dolske [:Dolske] from comment #8)
> > I'm not really sure what this bug is about with the meandering path between
> > comment 0 and now.
> > 
> > Screenshot showing the problem?
> 
> The problem was, I will have to double check if this is still the case, that
> icons in the tabstrip are not consistently inverted.
> 
> We used to invert them on glass because they were illegible otherwise. Now
> that we have the fog I am not sure that is the right thing to do.

Needinfo'ing for this decision, which I thought I saw pass by on IRC but don't remember anymore.

The inversion CSS has caused various small issues/regressions with buttons that have their own states (e.g. the sync, bookmarks or panorama buttons). I expect similar issues when we change stuff here, so I'd rather do it sooner than later...
Flags: needinfo?(shorlander)
Since we are no longer putting them directly on glass due to the fog I think it makes sense to just use the regular (non-inverted) icons here.
Flags: needinfo?(shorlander)
Per comment #13.
Summary: Choose to show the icons inverted or not in the tabbar → Don't use Toolbar-inverted.png for the tabbar or menubar on Win7 glass (without lwtheme)
Assignee: nobody → dao
Attached patch patchSplinter Review
What about the menu toolbar?
Attachment #8376694 - Flags: ui-review?(shorlander)
Comment on attachment 8376694 [details] [diff] [review]
patch

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

(In reply to Dão Gottwald [:dao] from comment #15)
> Created attachment 8376694 [details] [diff] [review]
> patch
> 
> What about the menu toolbar?

Looks good! 

I think we should keep the inverted icons for the menubar. The background doesn't go up far enough so it's still pretty easy to get them into a state where they are illegible. 

Thanks!
Attachment #8376694 - Flags: ui-review?(shorlander) → ui-review+
Comment on attachment 8376694 [details] [diff] [review]
patch

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

::: browser/themes/windows/browser-aero.css
@@ +222,5 @@
>      -moz-appearance: none;
>      background-color: #556;
>    }
>  
>    /* Use inverted icons for glassed toolbars */

Can you please change this to:
/* Use inverted icons for non-fogged glassed toolbars */
Attachment #8376694 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/a87f7974e8fe
Summary: Don't use Toolbar-inverted.png for the tabbar or menubar on Win7 glass (without lwtheme) → Don't use Toolbar-inverted.png for the tabbar on Win7 glass (without lwtheme)
https://hg.mozilla.org/mozilla-central/rev/a87f7974e8fe
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 30
Comment on attachment 8376694 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Australis tab bar styling
User impact if declined: needlessly inverted icons on the tab bar
Testing completed (on m-c, etc.): landed on m-c
Risk to taking this patch (and alternatives if risky): low
String or IDL/UUID changes made by this patch: none
Attachment #8376694 - Flags: approval-mozilla-aurora?
Attachment #8376694 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Whiteboard: [Australis:P3] → [Australis:P3][good first verify]
Nice job guys! Verified in 30b3
Status: RESOLVED → VERIFIED
Whiteboard: [Australis:P3][good first verify] → [Australis:P3][good first verify][testday-20140509]
You need to log in before you can comment on or make changes to this bug.