Closed Bug 1375973 Opened 3 years ago Closed 3 years ago

Text colour of background tabs is unreadable when in fullscreen mode

Categories

(Firefox :: Theme, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 56
Iteration:
56.2 - Jul 10
Tracking Status
firefox56 --- fixed
firefox57 --- verified

People

(Reporter: jonathanGB, Assigned: nhnt11)

References

Details

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

Attachments

(2 files)

Attached image Dark_theme_screenshot
Latest nightly didn't apply the dark theme correctly.
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)
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: nobody → nhnt11
Status: NEW → ASSIGNED
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.
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify+
Priority: -- → P1
QA Contact: brindusa.tot
Duplicate of this bug: 1375968
(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)
Duplicate of this bug: 1376402
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-
Iteration: 56.1 - Jun 26 → 56.2 - Jul 10
(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 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?
Attachment #8880965 - Flags: review?(dao+bmo)
(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 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+
Pushed by nhnt11@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/cf4987e9de05
Color the tabstrip instead of the titlebar on macOS. r=dao
https://hg.mozilla.org/mozilla-central/rev/cf4987e9de05
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56
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
Flags: qe-verify+
 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.