Closed Bug 907336 Opened 6 years ago Closed 6 years ago

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

Categories

(Firefox :: Theme, defect)

defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 978752

People

(Reporter: Gijs, Unassigned)

References

Details

(Whiteboard: [Australis:M?][Australis:P4])

Attachments

(5 files, 1 obsolete file)

No description provided.
Attachment #793004 - Flags: review?(mconley)
Well, that was embarrassing...

/me kicks bzexport
Summary: This gradient looks really messed up, period. But it looks even more messed up in customize mode. Let's nuke this. → Australis: Remove vertical gradients from TabsToolbar in win7 classic mode
Whiteboard: [Australis:M?][Australis:P4]
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)
Attachment #793004 - Attachment description: This gradient looks really messed up, period. But it looks even more messed up in customize mode. Let's nuke this. → Remove vertical gradient from TabsToolbar in win7 classic mode
Please get ui-review on this as I've never heard complaints about this before.
Status: NEW → ASSIGNED
Component: Toolbars and Customization → Theme
Attached image without-patch.PNG
Attached image with-patch.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?
Attachment #793038 - Flags: ui-review?(shorlander)
Attached image 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.
Just for record-keeping, it looks like this gradient was added in bug 624292 by Dao.
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).
(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 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.
Attachment #793004 - Flags: review?(mconley)
Attachment #793004 - Flags: review+
Attachment #793004 - Flags: feedback?(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 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.
Attachment #793004 - Flags: feedback?(dao) → feedback-
(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?
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)...
(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.
(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).
(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...
(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.
(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.
(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.
Depends on: 908665
Summary: Australis: Remove vertical gradients from TabsToolbar in win7 classic mode → Australis: Figure out what to do with vertical gradients in TabsToolbar on Windows classic
Attachment #793004 - Attachment is obsolete: true
Assignee: gijskruitbosch+bugs → nobody
Status: ASSIGNED → NEW
I'm pretty sure the fix we're talking aobut in 978752 will take care of this, reopen if I'm wrong!
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 978752
You need to log in before you can comment on or make changes to this bug.