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

Status: VERIFIED FIXED

(Firefox :: Theme, defect)

Description

[u428464]

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.

Comment 1

[:Gijs (he/him)]

(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...

Comment 2

[u428464]

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.

Comment 3

[u428464]

We need a decision here. Note that the Panorama icon is now missing when placed in the tabbar.

Comment 4

[:Gijs (he/him)]

(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.

Comment 5

[:Gijs (he/him)]

(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?

Comment 6

[:Gijs (he/him)]

(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.

Comment 7

[u428464]

(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. ;)

Comment 8

[Justin Dolske [:Dolske]]

I'm not really sure what this bug is about with the meandering path between comment 0 and now.

Screenshot showing the problem?

Comment 9

[Stephen Horlander [: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.

Comment 10

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

Attachment: Screenshot of inverted icons on the tab-bar

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

Comment 11

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

Attachment: Screenshot of regular icons on the tab-bar

This is a screenshot with the inverted icons disabled.

Comment 12

[:Gijs (he/him)]

(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...

Comment 13

[Stephen Horlander [: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.

Comment 14

[:Gijs (he/him)]

Per comment #13.

Comment 15

[Dão Gottwald [:dao]]

Attachment: patch

What about the menu toolbar?

Comment 16

[Stephen Horlander [: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!

Comment 17

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

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 */

Comment 18

[Dão Gottwald [:dao]]

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

Comment 19

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a87f7974e8fe

Comment 20

[Dão Gottwald [:dao]]

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

Comment 21

[Dão Gottwald [:dao]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/81664379b072

Comment 22

[moses]

Nice job guys! Verified in 30b3

Comment 23

[Bogdan Maris, Desktop QA]

I can also confirm that this is verified.