Closed Bug 1264227 Opened 8 years ago Closed 8 years ago

[GTK3] Active tab border highlighted should continue along the entire navbar

Categories

(Firefox :: Theme, defect, P2)

48 Branch
x86_64
Linux
defect
Points:
2

Tracking

()

VERIFIED FIXED
Firefox 48
Iteration:
48.3 - Apr 25
Tracking Status
firefox46 --- verified
firefox47 --- verified
firefox48 --- verified
firefox49 --- verified

People

(Reporter: mikedeboer, Assigned: mikedeboer)

References

(Depends on 1 open bug, )

Details

(Keywords: regression)

Attachments

(2 files, 1 obsolete file)

Please take a look at https://bug1244500.bmoattachments.org/attachment.cgi?id=8739764, where you'll notice that the navbar highlight is slightly darker than the highlight on the active tab.
These will need to be unified to be the same as the active tab.
Flags: qe-verify+
Flags: firefox-backlog+
This is the best I can come up with that works well with dark and light GTK themes AND LWT.
Attachment #8740952 - Flags: review?(gijskruitbosch+bugs)
Comment on attachment 8740952 [details] [diff] [review]
Patch v1: lighten the box shadow inside the navbar

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

So, I think this patch does the right thing, ish, but the structure is confusing.

You're adding a box-shadow that you label fgTabHighlight to the nav-bar. Shouldn't that be called "navbarInsetShadow" or something? It doesn't seem to actually affect the foreground tab in any way.

I'm also not 100% sure why this is changing the lwt case. That case isn't depending on the dark gtk theme, right? Can you clarify?
Attachment #8740952 - Flags: review?(gijskruitbosch+bugs) → review-
(In reply to :Gijs Kruitbosch from comment #2)
> You're adding a box-shadow that you label fgTabHighlight to the nav-bar.
> Shouldn't that be called "navbarInsetShadow" or something? It doesn't seem
> to actually affect the foreground tab in any way.

Shows that I'm terrible with naming things - you're right, that sounds much better. Except maybe that I usually associate 'shadow' with 'dark', but that might just be me.

> I'm also not 100% sure why this is changing the lwt case. That case isn't
> depending on the dark gtk theme, right? Can you clarify?

Well, that's because the LWT case is now obsolete: toolbarHighlightLWT == rgba(255,255,255,.4) == hsla(0,0%,100%,.4).
(In reply to Mike de Boer [:mikedeboer] from comment #3)
> (In reply to :Gijs Kruitbosch from comment #2)
> > You're adding a box-shadow that you label fgTabHighlight to the nav-bar.
> > Shouldn't that be called "navbarInsetShadow" or something? It doesn't seem
> > to actually affect the foreground tab in any way.
> 
> Shows that I'm terrible with naming things - you're right, that sounds much
> better. Except maybe that I usually associate 'shadow' with 'dark', but that
> might just be me.

navbarInsetHighlight ? I'm not amazing with names either, clearly. :-)

> > I'm also not 100% sure why this is changing the lwt case. That case isn't
> > depending on the dark gtk theme, right? Can you clarify?
> 
> Well, that's because the LWT case is now obsolete: toolbarHighlightLWT ==
> rgba(255,255,255,.4) == hsla(0,0%,100%,.4).

Oh, right. One day I'll learn more about how to map color spaces in my head. :-\
(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Mike de Boer [:mikedeboer] from comment #3)
> navbarInsetHighlight ? I'm not amazing with names either, clearly. :-)

This gets my vote!
Haha, OK! 'navbarInsetHighlight' it is :-)
Attachment #8740952 - Attachment is obsolete: true
Attachment #8741027 - Flags: review?(gijskruitbosch+bugs)
Attachment #8741027 - Flags: review?(gijskruitbosch+bugs) → review+
https://hg.mozilla.org/integration/fx-team/rev/4a496faf1c5cac37ab6dac9db549cb65867fa7f6
Bug 1264227: lighten the box shadow inside the navbar on Linux to match the active tab highlight color. r=Gijs
https://hg.mozilla.org/mozilla-central/rev/4a496faf1c5c
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48
Comment on attachment 8741027 [details] [diff] [review]
Patch v2: lighten the box shadow inside the navbar

Approval Request Comment
[Feature/regressing bug #]: bug 1184651, follow-up of bug 1244500.
[User impact if declined]: On dark Linux GTK themes, the border of the active tab is much lighter than the top border of the navbar. This patch fixes this case.
[Describe test coverage new/current, TreeHerder]: landed on m-c.
[Risks and why]: minor.
[String/UUID change made/needed]: n/a.
Attachment #8741027 - Flags: approval-mozilla-beta?
Attachment #8741027 - Flags: approval-mozilla-aurora?
Comment on attachment 8741027 [details] [diff] [review]
Patch v2: lighten the box shadow inside the navbar

Approval Request Comment
[Feature/regressing bug #]:
[User impact if declined]:
[Describe test coverage new/current, TreeHerder]:
[Risks and why]: 
[String/UUID change made/needed]:

Aiming this at m-r too since 46 migration from beta to release just happened.
Attachment #8741027 - Flags: approval-mozilla-release?
Comment on attachment 8741027 [details] [diff] [review]
Patch v2: lighten the box shadow inside the navbar

Polish for gtk3, please uplift for aurora, beta, and m-r for tomorrow's RC build.
Attachment #8741027 - Flags: approval-mozilla-release?
Attachment #8741027 - Flags: approval-mozilla-release+
Attachment #8741027 - Flags: approval-mozilla-beta?
Attachment #8741027 - Flags: approval-mozilla-beta+
Attachment #8741027 - Flags: approval-mozilla-aurora?
Attachment #8741027 - Flags: approval-mozilla-aurora+
Attached image highlighted border.png
I managed to reproduce this issue on Firefox 46 beta11 .
The active tab border highlighted continues  along the entire navbar, but I observed that before and after the selected tab, the highlighted border does not have a smoothly pass (see the attachment).
Tests were performed on Firefox 46.0 RC, Firefox 47.0a2 (2016-04-19), Firefox 48.0a1 (2016-04-19) and on Ubuntu 14.04 x64.
Should I log a separate bug for this issue?
Flags: needinfo?(mdeboer)
(In reply to Mihai Boldan, QA [:mboldan] from comment #15)
> Should I log a separate bug for this issue?

Yes, please!
Flags: needinfo?(mdeboer)
Since Bug 1270085 was logged separately for the issue mentioned in Comment 15, I am marking this bug Verified Fixed.
The tests were performed on Firefox 46.0.1, Firefox 47.0b2, Firefox 48.0a2 (2016-05-04), Firefox 49.0a1 (2016-05-03) and on Ubuntu 14.04 x64.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.