Text colour of background tabs is unreadable when in fullscreen mode

VERIFIED FIXED in Firefox 56

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: jonathanGB, Assigned: nhnt11)

Tracking

unspecified
Firefox 56
Points:
---

Firefox Tracking Flags

(firefox56 fixed, firefox57 verified)

Details

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

Attachments

(2 attachments)

Latest nightly didn't apply the dark theme correctly.

Comment 1

2 years 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 years 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 years ago
Assignee: nobody → nhnt11
Status: NEW → ASSIGNED
(Assignee)

Comment 3

2 years 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)
Iteration: --- → 56.1 - Jun 26
Flags: qe-verify+
Priority: -- → P1
QA Contact: brindusa.tot

Updated

2 years ago
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)

Updated

2 years ago
Duplicate of this bug: 1376402

Comment 8

2 years 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-
Iteration: 56.1 - Jun 26 → 56.2 - Jul 10
Comment hidden (mozreview-request)
(Assignee)

Comment 10

2 years 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 years 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?
Attachment #8880965 - Flags: review?(dao+bmo)
Comment hidden (mozreview-request)
(Assignee)

Comment 13

2 years 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 years 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 years 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 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/cf4987e9de05
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 56

Updated

2 years 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
Flags: qe-verify+

Comment 20

2 years 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.