Title bar gradient on the tab bar shouldn't care about the sizemode or menu bar state

RESOLVED FIXED in Firefox 28

Status

()

Firefox
Theme
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dao, Assigned: dao)

Tracking

Trunk
Firefox 28
All
Windows XP
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [Australis:M9][Australis:P3])

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

5 years ago
Created attachment 794671 [details] [diff] [review]
gradient.diff

The selector is still in the pre-Australis state and doesn't match Australis reality with regards to tabs being rendered in the title bar.
Attachment #794671 - Flags: review?(gijskruitbosch+bugs)
(Assignee)

Comment 1

5 years ago
Created attachment 794677 [details] [diff] [review]
patch v2

... and during self-review, I realized that [sizemode="maximized"] also excluded full-screen mode, which AFAIK we need to keep excluding.
Attachment #794671 - Attachment is obsolete: true
Attachment #794671 - Flags: review?(gijskruitbosch+bugs)
Attachment #794677 - Flags: review?(gijskruitbosch+bugs)

Comment 2

5 years ago
(In reply to Dão Gottwald [:dao] from comment #1)
> Created attachment 794677 [details] [diff] [review]
> patch v2
> 
> ... and during self-review, I realized that [sizemode="maximized"] also
> excluded full-screen mode, which AFAIK we need to keep excluding.

Can you put up some screenshots and get ui-review on this first, especially regarding restored windows with menubars?
(Assignee)

Comment 3

5 years ago
I don't think this needs ui-review. The selector is obviously just broken, so we should fix that. If somebody comes up with a better design, we can consider that, but that's orthogonal to the fact that this code should no longer care about the menu bar state or whether windows are restored or maximized.

Comment 4

5 years ago
(In reply to Dão Gottwald [:dao] from comment #3)
> I don't think this needs ui-review. The selector is obviously just broken,
> so we should fix that. If somebody comes up with a better design, we can
> consider that, but that's orthogonal to the fact that this code should no
> longer care about the menu bar state or whether windows are restored or
> maximized.

The gradient's impact is going to be very different in a context where the window isn't maximized, and/or has a menubar above it without that gradient (but *with* a native titlebar gradient, leading to something like a bottom-left-to-top-right combined gradient). It also seems from the discussion when it was originally added (bug 624292 comment 12) that it was only chosen because we didn't yet have code to extend the titlebar down the right number of pixels. I.e., it seems like it was a low-risk workaround for an issue that we would have liked to have fixed otherwise (and in fact, is now fixed). Now extending the reach of that workaround without reconsidering seems imprudent.

We're not in a rush, as it's not like the current situation has had people screaming about it. We've found issues using automated testing of various combinations, and we should work to correct the issues. Let's get UI/UX input on this and figure out what we actually want, and then get it right.
(Assignee)

Comment 5

5 years ago
(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > I don't think this needs ui-review. The selector is obviously just broken,
> > so we should fix that. If somebody comes up with a better design, we can
> > consider that, but that's orthogonal to the fact that this code should no
> > longer care about the menu bar state or whether windows are restored or
> > maximized.
> 
> The gradient's impact is going to be very different in a context where the
> window isn't maximized, and/or has a menubar above it without that gradient
> (but *with* a native titlebar gradient, leading to something like a
> bottom-left-to-top-right combined gradient).

I don't really see that difference. The combined gradient exists in maximized windows as well, and I actually agree it's kind of ugly. Again, that's a matter of figuring out a better design, which can happen independently from what this bug is about.

> It also seems from the
> discussion when it was originally added (bug 624292 comment 12) that it was
> only chosen because we didn't yet have code to extend the titlebar down the
> right number of pixels. I.e., it seems like it was a low-risk workaround for
> an issue that we would have liked to have fixed otherwise (and in fact, is
> now fixed).

Yes, the context has changed. The title bar now reaches down for enough, but at the same time background tabs became transparent, creating a new problem.

> Now extending the reach of that workaround without reconsidering
> seems imprudent.

I'm all for reconsidering. That's why I decided to file a new bug for this patch rather than hijacking bug 907336.
(Assignee)

Comment 6

5 years ago
> The title bar now reaches down for enough

s/for/far/

I should sanity-check my stuff before submitting it...

Comment 7

5 years ago
Comment on attachment 794677 [details] [diff] [review]
patch v2

OK, but then let's please make sure we fix this situation before we hit aurora... :-)
Attachment #794677 - Flags: review?(gijskruitbosch+bugs) → review+
(Assignee)

Comment 8

5 years ago
https://hg.mozilla.org/projects/ux/rev/e2a6a3c10b0d
Whiteboard: [Australis:P3] → [Australis:P3][fixed-in-ux]

Updated

5 years ago
Whiteboard: [Australis:P3][fixed-in-ux] → [Australis:M9][Australis:P3][fixed-in-ux]

Comment 9

5 years ago
https://hg.mozilla.org/mozilla-central/rev/e2a6a3c10b0d
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [Australis:M9][Australis:P3][fixed-in-ux] → [Australis:M9][Australis:P3]
Target Milestone: --- → Firefox 28
You need to log in before you can comment on or make changes to this bug.