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

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P2)

Description

[Stephen Horlander [:shorlander]]

Attachment: 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.

Comment 1

[:Gijs (he/him)]

What's the desired fix here?

Comment 2

[Stephen Horlander [:shorlander]]

(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

Comment 3

[Dão Gottwald [:dao]]

We'll probably need a less ideal fix for the immediate future.

Comment 4

[:Gijs (he/him)]

(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?

Comment 5

[(Currently slow to respond) Philipp Sackl [:phlsa] (Firefox UX) please use needinfo]

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

Comment 6

[Stephen Horlander [:shorlander]]

(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.

Comment 7

[Tim Nguyen :ntim]

How about making the tab stroke and navbar border changes for default theme only ?

Comment 8

[:Gijs (he/him)]

(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.

Comment 9

[Stephen Horlander [:shorlander]]

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

Comment 10

[:Gijs (he/him)]

I have a patch here that needs some polish still. Will try to put it up tomorrow.

Comment 11

[:Gijs (he/him)]

Attachment: 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

Comment 12

[Dão Gottwald [: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?

Comment 13

[Tim Nguyen :ntim]

(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.

Comment 14

[:Gijs (he/him)]

(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?

Comment 15

[Dão Gottwald [: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?

Comment 16

[:Gijs (he/him)]

(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".

Comment 17

[Dão Gottwald [: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.

Comment 18

[:Gijs (he/him)]

Attachment: 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).

Comment 19

[Dão Gottwald [: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.

Comment 20

[:Gijs (he/him)]

Attachment: Patch v0.2

Comment 21

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/dffae61581d0

Comment 22

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/dffae61581d0

Comment 23

[Cornel Ionce [:noni] [Hubs QA]]

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.

Comment 24

[Dão Gottwald [:dao]]

Gijs, can we get this uplifted?

Comment 25

[:Gijs (he/him)]

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

Comment 26

[Sylvestre Ledru [:Sylvestre]]

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

visual regression, taking it.
Should be in 42 b4.

Comment 27

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/19a60a8e02c5

Comment 28

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e601b67d1f03

Comment 29

[Cornel Ionce [:noni] [Hubs QA]]

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