[Windows 10] Invisible tabs with "High Contrast White" system theme

VERIFIED FIXED in Firefox 42

Status

()

P2
major
VERIFIED FIXED
3 years ago
3 years ago

People

(Reporter: shorlander, Assigned: Gijs)

Tracking

(Depends on: 1 bug, Blocks: 1 bug, {regression})

Trunk
Firefox 44
All
Windows 10
regression
Points:
---
Dependency tree / graph
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox42 verified, firefox43 verified, firefox44 verified)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

3 years ago
Created attachment 8642508 [details]
Invisible Tabs

If you use the "High Contrast White" system theme the tabs become "invisible" because the white labels and images blend in with the white window color.

Updated

3 years ago
Blocks: 1173728, 1187268
Keywords: regression
(Assignee)

Comment 1

3 years ago
What's the desired fix here?
Flags: needinfo?(philipp)
(Reporter)

Comment 2

3 years ago
(In reply to :Gijs Kruitbosch from comment #1)
> What's the desired fix here?

As we discussed on IRC the ideal fix would be using SVG tabs (bug 1068110) and then using the proper color keywords in the UI when it's in High Contrast mode.

Mocked up what it could look like with the four built in themes: http://people.mozilla.org/~shorlander/mockups/Windows-10-High-Contrast/Windows-10-High-Contrast.html
We'll probably need a less ideal fix for the immediate future.
Blocks: 1192839
Severity: normal → major
(Assignee)

Comment 4

3 years ago
(In reply to Dão Gottwald [:dao] from comment #3)
> We'll probably need a less ideal fix for the immediate future.

I agree. Can we restrict our color changes etc. to the default theme, and does that restore some level of usability for HCM, even if it looks awful?
Stephen already outlined the desired fix.
For the immediate future, it's hard for me to tell what is technically possible in the short term...
Flags: needinfo?(philipp)
(Reporter)

Comment 6

3 years ago
(In reply to :Gijs Kruitbosch from comment #4)
> (In reply to Dão Gottwald [:dao] from comment #3)
> > We'll probably need a less ideal fix for the immediate future.
> 
> I agree. Can we restrict our color changes etc. to the default theme, and
> does that restore some level of usability for HCM, even if it looks awful?

Not sure what that would look like really.
How about making the tab stroke and navbar border changes for default theme only ?
(Assignee)

Comment 8

3 years ago
(In reply to Tim Nguyen [:ntim] from comment #7)
> How about making the tab stroke and navbar border changes for default theme
> only ?

Yes, this was suggested and discussed already in comment #4 and 6, but we don't know what it'd look like and how well it'd work. If you can create a patch and try build, that would be helpful.
(Reporter)

Comment 9

3 years ago
If we switched the tab label color keyword to ButtonText I think it would at least leave everything readable in non-ideal configurations.
(Assignee)

Updated

3 years ago
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 10

3 years ago
I have a patch here that needs some polish still. Will try to put it up tomorrow.
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 11

3 years ago
Created attachment 8659291 [details]
MozReview Request: Bug 1190462 - Firefox shouldn't be completely unusable in high contrast white on windows 10, r?dao

Bug 1190462 - Firefox shouldn't be completely unusable in high contrast white on windows 10, r?dao
Attachment #8659291 - Flags: review?(dao)
Sorry if I'm confused... Why is background-color: ActiveCaption needed for Win 10 but not 8? Or do we set that for Win 8 in some place that I'm missing?
(In reply to Dão Gottwald [:dao] from comment #12)
> Sorry if I'm confused... Why is background-color: ActiveCaption needed for
> Win 10 but not 8? Or do we set that for Win 8 in some place that I'm missing?
We only draw the non-native background-color (for the custom caption buttons) on Windows 10.
(Assignee)

Comment 14

3 years ago
(In reply to Dão Gottwald [:dao] from comment #12)
> Sorry if I'm confused... Why is background-color: ActiveCaption needed for
> Win 10 but not 8? Or do we set that for Win 8 in some place that I'm missing?

Tim implied as much already, but to make this really explicit:

on win8 we have a transparent window background color: 

https://dxr.mozilla.org/mozilla-central/source/browser/themes/windows/browser-aero.css#262-272

and you can see the Windows-rendered caption (the area of which we extend behind the tabstrip).

On win10 we don't set a transparent window background, and instead set a hardcoded hsl background, which means we occlude the windows titlebar (except the top 1px border that we cut out in widget code so you can see the top window border).

Does that help?
Flags: needinfo?(dao)
The hardcoded gray background is in a default-theme media query (as it should be), so that doesn't seem directly relevant here. So my question is, can we have the non-default case use the same code path as for win8 rather than adding special rules for win10-non-default?
Flags: needinfo?(dao)
(Assignee)

Comment 16

3 years ago
(In reply to Dão Gottwald [:dao] from comment #15)
> The hardcoded gray background is in a default-theme media query (as it
> should be), so that doesn't seem directly relevant here.

But the default window background color still isn't transparent.

> So my question is,
> can we have the non-default case use the same code path as for win8 rather
> than adding special rules for win10-non-default?

Are you saying you just want us to set the background color to transparent in that media query instead of to ActiveCaption? Because I can do that, but there is no way to unify those media queries, so I don't understand what you mean by "same code path".
Flags: needinfo?(dao)
Comment on attachment 8659291 [details]
MozReview Request: Bug 1190462 - Firefox shouldn't be completely unusable in high contrast white on windows 10, r?dao

(In reply to :Gijs Kruitbosch from comment #16)
> (In reply to Dão Gottwald [:dao] from comment #15)
> > The hardcoded gray background is in a default-theme media query (as it
> > should be), so that doesn't seem directly relevant here.
> 
> But the default window background color still isn't transparent.
> 
> > So my question is,
> > can we have the non-default case use the same code path as for win8 rather
> > than adding special rules for win10-non-default?
> 
> Are you saying you just want us to set the background color to transparent
> in that media query instead of to ActiveCaption?

Does transparent have the same effect? If so I'd prefer that.

>+  @media not all and (-moz-windows-default-theme) {
>+    #main-window:not(:-moz-window-inactive) {
>+      background-color: ActiveCaption;
>+    }
>+  }

What color is used for inactive windows?

>+  @media not all and (-moz-windows-default-theme) {
>+    /* Need a border between tabs and the navbar on high contrast themes on win10. */
>+    #nav-bar {
>+      border-top: 1px solid @toolbarShadowColor@ !important;
>+      box-shadow: 0 1px 0 @toolbarHighlight@ inset;
>+    }
>+  }

I think at this point I'd prefer removing the Windows version restriction from http://hg.mozilla.org/mozilla-central/annotate/3e8dde8f8c17/browser/themes/windows/browser.css#l352 and explicitly removing the border and box-shadow for the Win10 default-theme.
Flags: needinfo?(dao)
Attachment #8659291 - Flags: review?(dao)
(Assignee)

Comment 18

3 years ago
Created attachment 8664860 [details] [diff] [review]
Patch

Please let me know if this is what you meant, or if you want the box-shadow/border resetting to move just after where we currently set it (without media queries, after this patch).
Attachment #8659291 - Attachment is obsolete: true
Attachment #8664860 - Flags: review?(dao)
Comment on attachment 8664860 [details] [diff] [review]
Patch

>+          #nav-bar {
>+            border-top: none;

nit: border-top-style

border-top is set with !important, so you'll probably need to use that here too.
Attachment #8664860 - Flags: review?(dao) → review-
(Assignee)

Comment 20

3 years ago
Created attachment 8664865 [details] [diff] [review]
Patch v0.2
Attachment #8664860 - Attachment is obsolete: true
Attachment #8664865 - Flags: review?(dao)

Updated

3 years ago
Attachment #8664865 - Flags: review?(dao) → review+
https://hg.mozilla.org/mozilla-central/rev/dffae61581d0
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox44: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
Flags: qe-verify+
Mozilla/5.0 (Windows NT 10.0; rv:44.0) Gecko/20100101 Firefox/44.0

The tabs are now visible when the "High Contrast White" system theme is enabled.
Tested on Windows 10 32-bit, using latest Nightly, build ID: 20150930030231.
Status: RESOLVED → VERIFIED
status-firefox44: fixed → verified
Gijs, can we get this uplifted?
Flags: needinfo?(gijskruitbosch+bugs)
(Assignee)

Comment 25

3 years ago
Comment on attachment 8664865 [details] [diff] [review]
Patch v0.2

Approval Request Comment
[Feature/regressing bug #]: win10 high contrast
[User impact if declined]: you can't use Firefox in win10 high contrast mode
[Describe test coverage new/current, TreeHerder]: no, styling only
[Risks and why]: very low, styling only change that means some of our special styling only applies on the default theme, instead of everywhere.
[String/UUID change made/needed]: no
Flags: needinfo?(gijskruitbosch+bugs)
Attachment #8664865 - Flags: approval-mozilla-beta?
Attachment #8664865 - Flags: approval-mozilla-aurora?
status-firefox43: --- → affected
Comment on attachment 8664865 [details] [diff] [review]
Patch v0.2

visual regression, taking it.
Should be in 42 b4.
Attachment #8664865 - Flags: approval-mozilla-beta?
Attachment #8664865 - Flags: approval-mozilla-beta+
Attachment #8664865 - Flags: approval-mozilla-aurora?
Attachment #8664865 - Flags: approval-mozilla-aurora+
I've also verified this fix on Windows 10 32-bit using:
- Firefox 42.0b4, build ID: 20151005144425
- Latest 43.0a2 Aurora, build ID: 20151005004046
status-firefox42: fixed → verified
status-firefox43: fixed → verified
(Assignee)

Updated

3 years ago
Depends on: 1219747
You need to log in before you can comment on or make changes to this bug.