Text colour of background tabs is unreadable when in fullscreen mode

VERIFIED FIXED in Firefox 56

Status

()

Firefox
Theme
P1
normal
VERIFIED FIXED
2 months ago
8 days ago

People

(Reporter: jonathanGB, Assigned: nhnt11)

Tracking

unspecified
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

(Whiteboard: [photon-visual][p1])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

Created attachment 8880926 [details]
Dark_theme_screenshot

Latest nightly didn't apply the dark theme correctly.

Comment 1

2 months ago
Can you reproduce in a clean profile ( https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles ) ?

(even if not, we should figure out why it's not working in your main profile, but checking helps!)
Flags: needinfo?(jguillotteblouin)
(Assignee)

Comment 2

2 months ago
We need to make sure we don't set the light text color for fullscreen mode (in which the dark titlebar is not applied).
Flags: needinfo?(jguillotteblouin)
Summary: Dark theme not applied correctly on macOS → Text colour of background tabs is unreadable when in fullscreen mode
Whiteboard: [photon-visual][p1]
(Assignee)

Updated

2 months ago
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
(Assignee)

Comment 3

2 months ago
According to the spec, the tabstrip background should be dark (and translucent/blurred) in full screen mode. For now I will upload a simple patch to make the text colour look ok.
Comment hidden (mozreview-request)

Updated

2 months ago
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify+
Priority: -- → P1
QA Contact: brindusa.tot

Updated

2 months ago
Duplicate of this bug: 1375968

Comment 6

2 months ago
(In reply to Nihanth Subramanya [:nhnt11] from comment #2)
> (in which the dark titlebar is not applied).

Why is it not applied?
Flags: needinfo?(nhnt11)

Updated

2 months ago
Duplicate of this bug: 1376402

Comment 8

2 months ago
mozreview-review
Comment on attachment 8880965 [details]
Bug 1375973 - Color the tabstrip instead of the titlebar on macOS.

https://reviewboard.mozilla.org/r/152328/#review157638

I see no compelling reason to land this workaround, we should instead make it so that the title bar is dark.
Attachment #8880965 - Flags: review?(dao+bmo) → review-

Updated

2 months ago
Iteration: 56.1 - Jun 26 → 56.2 - Jul 10
Comment hidden (mozreview-request)
(Assignee)

Comment 10

2 months ago
(In reply to Dão Gottwald [::dao] from comment #6)
> (In reply to Nihanth Subramanya [:nhnt11] from comment #2)
> > (in which the dark titlebar is not applied).
> 
> Why is it not applied?

Because the original dark-titlebar patch coloured the *titlebar* and not the tabstrip. In fullscreen, we want to colour the tabstrip.

I ended up moving the styling to the tabstrip for all cases in the latest patch.
Flags: needinfo?(nhnt11)

Comment 11

2 months ago
mozreview-review
Comment on attachment 8880965 [details]
Bug 1375973 - Color the tabstrip instead of the titlebar on macOS.

https://reviewboard.mozilla.org/r/152328/#review157958

::: browser/themes/osx/browser.css:1561
(Diff revision 2)
>  
> +%ifndef MOZ_PHOTON_THEME
>  #main-window:not([customizing]) #navigator-toolbox[inFullscreen] > #TabsToolbar:not(:-moz-lwtheme),
>  #main-window:not(:-moz-any([customizing],[tabsintitlebar])) #navigator-toolbox > #TabsToolbar:not(:-moz-lwtheme) {
> +%else
> +#main-window:not(:-moz-any([customizing],[tabsintitlebar],[inFullscreen])) #navigator-toolbox > #TabsToolbar:not(:-moz-lwtheme) {

1. This would be easier to read, I think:

#main-window:not([customizing]):not([tabsintitlebar]):not([inFullscreen])

Do we even need all of this, though, given that your other rule sets -moz-appearance:none?

2. Can "#navigator-toolbox >" be removed from the selector?

Updated

2 months ago
Attachment #8880965 - Flags: review?(dao+bmo)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 months ago
(In reply to Dão Gottwald [::dao] from comment #11)
> Comment on attachment 8880965 [details]
> Bug 1375973 - Color the tabstrip instead of the titlebar on macOS.
> 
> https://reviewboard.mozilla.org/r/152328/#review157958
> 
> Do we even need all of this, though, given that your other rule sets
> -moz-appearance:none?

Yeah, this is specifically so that we don't color the tabstrip when the native titlebar is enabled, because it's ugly.

Based on the specs I've seen so far, we want the titlebar to be styled, so we can't use the native one. I'll work on that separately from this bug.

Comment 14

2 months ago
mozreview-review
Comment on attachment 8880965 [details]
Bug 1375973 - Color the tabstrip instead of the titlebar on macOS.

https://reviewboard.mozilla.org/r/152328/#review158090

::: browser/themes/osx/browser.css:1561
(Diff revision 3)
>  
> +%ifndef MOZ_PHOTON_THEME
>  #main-window:not([customizing]) #navigator-toolbox[inFullscreen] > #TabsToolbar:not(:-moz-lwtheme),
>  #main-window:not(:-moz-any([customizing],[tabsintitlebar])) #navigator-toolbox > #TabsToolbar:not(:-moz-lwtheme) {
> +%else
> +#main-window:not([customizing]):not([tabsintitlebar]):not([inFullscreen]) #TabsToolbar:not(:-moz-lwtheme) {

nit: replace #main-window with :root here if you can. don't bother with the non-photon selectors.
Attachment #8880965 - Flags: review?(dao+bmo) → review+
Comment hidden (mozreview-request)

Comment 16

2 months ago
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cf4987e9de05
Color the tabstrip instead of the titlebar on macOS. r=dao

Comment 17

2 months ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cf4987e9de05
Status: ASSIGNED → RESOLVED
Last Resolved: 2 months ago
status-firefox56: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

2 months ago
Duplicate of this bug: 1377821
I verified this issue on Windows 10, Mac OS X 10.10 and Ubuntu 16.04 and I can't reproduce this issue with the FF Nightly 57.0a1(2017-08-10). I will mark this as a verified fixed.
Status: RESOLVED → VERIFIED
status-firefox57: --- → verified
Flags: qe-verify+

Comment 20

8 days ago
 The issue is no longer reproducible on Firefox 55.0.1.Tests were performed under Windows 8.  [bugday-20170816]
You need to log in before you can comment on or make changes to this bug.