879588 - Tab title is hard to read in windows high-contrast mode with a light LWT (yellow text)

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Justin Dolske [:Dolske]]

(Filed from Australis tabs ui-review #1)

screenshot_105_HighContrast1_twoPinnedWithOverflow_normal_lightLWT

https://www.flickr.com/photos/12814025@N06/8951222035/sizes/o/in/set-72157633930146585/

Screenshot shows Windows in high-contrast mode, where the selected tab has yellow text on a light-gray background.

Possibly an existing issue not new to Australis?

Comment 1

[Matthew N. [:MattN] (PM me if requests are blocking you)]

Note that this specific problem occurs because of the light coloured (white) lightweight theme so this is probably unlikely to happen in practice as a user wouldn't choose such a theme if they want high contrast. I suppose it's possible that a user with a light LWT turns on high contrast mode afterwards.

(fine) No LWT - screenshot_103_HighContrast1_twoPinnedWithOverflow_normal_noLWT:
https://www.flickr.com/photos/12814025@N06/8952414542/sizes/o/in/set-72157633930146585/
(fine) Dark LWT: screenshot_104_HighContrast1_twoPinnedWithOverflow_normal_darkLWT:
https://www.flickr.com/photos/12814025@N06/8952414680/sizes/o/in/set-72157633930146585/

Comment 2

[Dão Gottwald [::dao]]

(In reply to Matthew N. [:MattN] from comment #1)
> Note that this specific problem occurs because of the light coloured (white)
> lightweight theme

Why are we using a native text color (yellow in this case) on a lightweight theme background?

Comment 3

[Matthew N. [:MattN] (PM me if requests are blocking you)]

(In reply to Dão Gottwald [:dao] from comment #2)
> (In reply to Matthew N. [:MattN] from comment #1)
> > Note that this specific problem occurs because of the light coloured (white)
> > lightweight theme
> 
> Why are we using a native text color (yellow in this case) on a lightweight
> theme background?

That seems like the issue that needs to be fixed. That isn't intentional AFAIK.

Comment 4

[Mike de Boer [:mikedeboer]]

taking this to Australis M7

Comment 5

[(parental leave 10/24-mid-February) Jared Wein [:jaws] (please needinfo? me)]

This seems pretty edge-casey, so I think we shouldn't mark this as a blocker.

Comment 6

[Dão Gottwald [::dao]]

It's this rule's fault:

http://hg.mozilla.org/projects/ux/file/634e4f218c22/browser/themes/windows/browser.css#l1580

Comment 7

[:Gijs (he/him)]

(In reply to Dão Gottwald [:dao] from comment #6)
> It's this rule's fault:
> 
> http://hg.mozilla.org/projects/ux/file/634e4f218c22/browser/themes/windows/
> browser.css#l1580

This doesn't make sense to me. The problem shows on selected tabs, and that rule is for non-selected tabs, right? Or am I misreading this?

Comment 8

[Dão Gottwald [::dao]]

Attachment: patch

(In reply to :Gijs Kruitbosch from comment #7)
> (In reply to Dão Gottwald [:dao] from comment #6)
> > It's this rule's fault:
> > 
> > http://hg.mozilla.org/projects/ux/file/634e4f218c22/browser/themes/windows/
> > browser.css#l1580
> 
> This doesn't make sense to me. The problem shows on selected tabs, and that
> rule is for non-selected tabs, right? Or am I misreading this?

The problem is on selected tabs because that selector excludes them.

Comment 9

[Dão Gottwald [::dao]]

Comment on attachment 794663 [details] [diff] [review]
patch

>--- a/browser/themes/osx/browser.css
>+++ b/browser/themes/osx/browser.css
>@@ -2523,21 +2523,16 @@ toolbarbutton.chevron > .toolbarbutton-m
>   border: solid transparent;
>   border-width: 0 11px;
> }
> 
> .tabbrowser-tab:focus > .tab-stack > .tab-content > .tab-label {
>   box-shadow: @focusRingShadow@;
> }
> 
>-.tabbrowser-tab:-moz-lwtheme {
>-  color: inherit;
>-  text-shadow: inherit;
>-}

Err, text-shadow:inherit is still needed on OS X :/

Comment 10

[Dão Gottwald [::dao]]

Attachment: patch v2

Comment 11

[:Gijs (he/him)]

Comment on attachment 794668 [details] [diff] [review]
patch v2

Review of attachment 794668 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/themes/windows/browser.css
@@ -1466,5 @@
> -
> -.tabbrowser-tab[selected=true]:-moz-lwtheme {
> -  /* Copied from :root:-moz-lwtheme-darktext in global.css */
> -  text-shadow: 0 -0.5px 1.5px white;
> -}

I have to confess that I don't understand why this text-shadow rule had to go, too, in the context of this patch. If it's cleanup and/or the existing rule is unnecessary, can you explain how/why it is unnecessary?

Comment 12

[Dão Gottwald [::dao]]

The lwt text-shadow comes from global.css and is already inherited. The custom text-shadow you're looking at tried to accommodate to the selected tab being excluded in the color:inherit rule on Windows (i.e. this bug).

Comment 13

[:Gijs (he/him)]

Comment on attachment 794668 [details] [diff] [review]
patch v2

Review of attachment 794668 [details] [diff] [review]:
-----------------------------------------------------------------

(In reply to Dão Gottwald [:dao] from comment #12)
> The lwt text-shadow comes from global.css and is already inherited. The
> custom text-shadow you're looking at tried to accommodate to the selected
> tab being excluded in the color:inherit rule on Windows (i.e. this bug).

Fab, thanks for explaining. LGTM, then!

Comment 14

[Dão Gottwald [::dao]]

https://hg.mozilla.org/projects/ux/rev/306f68dadc04

Comment 15

[:Gijs (he/him)]

https://hg.mozilla.org/mozilla-central/rev/306f68dadc04