907336 - Australis: Figure out what to do with vertical gradients in TabsToolbar on Windows classic

Status: RESOLVED DUPLICATE of bug 978752

(Firefox :: Theme, defect)

Comment 1

[:Gijs (he/him)]

Attachment: Remove vertical gradient from TabsToolbar in win7 classic mode

Comment 2

[:Gijs (he/him)]

Well, that was embarrassing...

/me kicks bzexport

Comment 3

[:Gijs (he/him)]

Comment on attachment 793004 [details] [diff] [review]
Remove vertical gradient from TabsToolbar in win7 classic mode

(all this because bzexport has a --description and a --bug-description field, and I used the former where I should have used the latter, upon which it helpfully decided to stick it both in the bug summary and in the patch name. Sigh)

Comment 4

[Matthew N. [:MattN]]

Please get ui-review on this as I've never heard complaints about this before.

Comment 5

[:Gijs (he/him)]

Attachment: without-patch.PNG

Comment 6

[:Gijs (he/him)]

Attachment: without-patch-customizing.PNG

Comment 7

[:Gijs (he/him)]

Attachment: with-patch.PNG

Comment 8

[:Gijs (he/him)]

Attachment: with-patch-customizing.PNG

If I recall correctly, when we did the tabs review there was surprise about how this looked on Win7 classic on regular nightly (which has the same appearance as the before-patch screenshots) anyway, but I may be imagining that...

Stephen, would you be OK with removing these gradients? If not, can you suggest how else we should fix the visual discrepancy here?

Comment 9

[:Gijs (he/him)]

Attachment: restored-mode.PNG

For reference, this is what restored mode looks like. It's not affected by this patch. Note also that on WinXP, we already look like the with-patch screenshots in classic mode, it's only win7 that has the vertical gradient.

Comment 10

[Mike Conley (:mconley) (:⚙️)]

Just for record-keeping, it looks like this gradient was added in bug 624292 by Dao.

Comment 11

[Matthew N. [:MattN]]

The gradient looks as expected to me when outside of customization mode. I only remember this getting mentioned because of the titlebar not lining up in customization mode (bug 885062 fixes this) and that the new tab button needed more contrast when near the left of the tabstrip (bug 887992).

Comment 12

[:Gijs (he/him)]

(In reply to Matthew N. [:MattN] from comment #11)
> The gradient looks as expected to me when outside of customization mode. I
> only remember this getting mentioned because of the titlebar not lining up
> in customization mode (bug 885062 fixes this) and that the new tab button
> needed more contrast when near the left of the tabstrip (bug 887992).

Actually, it seems we've had bug 634975 filed for having it be a left to right gradient rather than a vertical one, so I'm not sure how "expected" this is in the first place...

Comment 13

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 793004 [details] [diff] [review]
Remove vertical gradient from TabsToolbar in win7 classic mode

The removal looks fine to me - but I think I want Dao's feedback here in the event that we're overlooking a case where this gradient is necessary.

Comment 14

[Dão Gottwald [:dao]]

Comment on attachment 793004 [details] [diff] [review]
Remove vertical gradient from TabsToolbar in win7 classic mode

How readable are background tabs on the right side of the window with this patch applied?

Comment 15

[Dão Gottwald [:dao]]

Comment on attachment 793004 [details] [diff] [review]
Remove vertical gradient from TabsToolbar in win7 classic mode

I tested this myself and it reduces the contrast between tab titles and their background considerably. Still good enough for my eyes, but probably not for others.

Comment 16

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #15)
> Comment on attachment 793004 [details] [diff] [review]
> Remove vertical gradient from TabsToolbar in win7 classic mode
> 
> I tested this myself and it reduces the contrast between tab titles and
> their background considerably. Still good enough for my eyes, but probably
> not for others.

I'm confused. I was going to look at this this morning, but the background tab titles presumably use the Caption foreground colors in that same block, which is the same color that an ordinary titlebar title would be if it were in an ordinary titlebar, with the same background. How is that contrast not good enough?

Comment 17

[:Gijs (he/him)]

NB: there is no vertical gradient on XP. Bug 879593's summary is about tab title readability there as well, but then comment #0 and comment #2 talk about the menubar and suggest making it white (which, if I'm not mistaken, it now is)...

Comment 18

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #16)
> (In reply to Dão Gottwald [:dao] from comment #15)
> > Comment on attachment 793004 [details] [diff] [review]
> > Remove vertical gradient from TabsToolbar in win7 classic mode
> > 
> > I tested this myself and it reduces the contrast between tab titles and
> > their background considerably. Still good enough for my eyes, but probably
> > not for others.
> 
> I'm confused. I was going to look at this this morning, but the background
> tab titles presumably use the Caption foreground colors in that same block,
> which is the same color that an ordinary titlebar title would be if it were
> in an ordinary titlebar, with the same background. How is that contrast not
> good enough?

Title bar captions usually don't go all the way to the right side. Tabs however do. When a wide title bar caption does become less readable at the end, that's also less critical than it is for tabs, because users need to interact with those.

Comment 19

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #18)
> Title bar captions usually don't go all the way to the right side. Tabs
> however do. When a wide title bar caption does become less readable at the
> end, that's also less critical than it is for tabs, because users need to
> interact with those.

This is a fair point. However, XP (in both modes) and Win7 Classic in restored mode already don't have this gradient, so they already have this problem. Do we want to introduce this gradient everywhere? Should we do that and then get rid of it in customization mode? (I have to say that I'm not a particularly big fan of the look of it, but making foreground colors and images invert across the halfway point of the window is (a) much harder to implement and (b) going to look bad, too, so I'm not sure what alternatives we have).

Comment 20

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #19)
> This is a fair point. However, XP (in both modes) and Win7 Classic in
> restored mode already don't have this gradient, so they already have this
> problem. Do we want to introduce this gradient everywhere?

This code doesn't exclude XP in any way, so I don't see why it wouldn't work there.

As for restored windows, this is a regression introduced by Australis, so we should probably do something about it.

> Should we do that
> and then get rid of it in customization mode? (I have to say that I'm not a
> particularly big fan of the look of it, but making foreground colors and
> images invert across the halfway point of the window is (a) much harder to
> implement and (b) going to look bad, too, so I'm not sure what alternatives
> we have).

Well, I've already advocated elsewhere for dropping the super-thick window padding while customizing...

Comment 21

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #20)
> (In reply to :Gijs Kruitbosch from comment #19)
> > This is a fair point. However, XP (in both modes) and Win7 Classic in
> > restored mode already don't have this gradient, so they already have this
> > problem. Do we want to introduce this gradient everywhere?
> 
> This code doesn't exclude XP in any way, so I don't see why it wouldn't work
> there.

On XP, the menubar is visible by default. The selectors require an inactive menubar, so no gradient is displayed. Should we change this?

> As for restored windows, this is a regression introduced by Australis, so we
> should probably do something about it.

And I just gave suggestions, but you haven't said what you think of them, apart from suggesting we remove the customization mode padding, which (a) we've discussed before, and (b) wouldn't solve the issue I was asking about.

To summarize my line of thought: if this gradient is meant to address a readability issue, and we think that issue is severe and still needs addressing, then we should address it everywhere, not only if the menubar is invisible, and only in maximized windows. If we decide it's not important or there is an alternative way to fix the contrast issue, let's pursue that.

Note that on current m-c, if you enable the menubar, both the tabstoolbar and the menubar get a greyish (ie menu) background. This is not the most attractive, but it does solve the issue. We could do the same for Australis, but it'll probably look even worse on restored windows than it looks on maximized ones.

As for customization mode, I think that in principle, the readability issue isn't as important for customization mode (as you're not trying to read tabs at that point) so we could, if necessary, solve any discrepancy in color/gradient with the 'real' titlebar colors by disabling whatever workaround we use during customization mode.

Comment 22

[Dão Gottwald [:dao]]

(In reply to :Gijs Kruitbosch from comment #21)
> (In reply to Dão Gottwald [:dao] from comment #20)
> > (In reply to :Gijs Kruitbosch from comment #19)
> > > This is a fair point. However, XP (in both modes) and Win7 Classic in
> > > restored mode already don't have this gradient, so they already have this
> > > problem. Do we want to introduce this gradient everywhere?
> > 
> > This code doesn't exclude XP in any way, so I don't see why it wouldn't work
> > there.
> 
> On XP, the menubar is visible by default. The selectors require an inactive
> menubar, so no gradient is displayed. Should we change this?

So the difference is menubar hidden vs. shown rather than XP vs. non-XP. As you've already realized, this is also fallout from Australis. The selector only covers the cases it had to cover pre-Australis.

> > As for restored windows, this is a regression introduced by Australis, so we
> > should probably do something about it.
> 
> And I just gave suggestions, but you haven't said what you think of them,
> apart from suggesting we remove the customization mode padding, which (a)
> we've discussed before, and (b) wouldn't solve the issue I was asking about.

I don't understand what you were talking about, then. I was responding to the customization mode part. If we stop dramatically changing the whole window layout, we won't have to worry the discrepancy here.

Comment 23

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [:dao] from comment #22)
> If we stop dramatically changing the whole
> window layout, we won't have to worry the discrepancy here.

... worry about the discrepancy here.

Comment 24

[Justin Dolske [:Dolske]]

I'm pretty sure the fix we're talking aobut in 978752 will take care of this, reopen if I'm wrong!