1577740 - "This time, search with" and prefs/options cog/gear icon use too faint a shade of grey as foreground colour on default theme

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P1)

Description

[:Gijs (he/him)]

Attachment: Screenshot 2019-08-30 at 11.42.22.png

See screenshot. Even as someone with good eyesight, I had to squint to read this. It's much too light to pass wcag contrast checks (1.82 : 1). Looks like the colour is specified as rgba(0,0,0,0.247).

Comment 1

[Marco Bonardo [:mak]]

[Tracking Requested - why for this release]: I suspect we don't want to release hardly readable text in primary ui.

Comment 2

[Liz Henry (:lizzard) (relman/hg->git project)]

Drew, can you help find an owner for this issue ? Thanks!

Comment 3

[Drew Willcoxon :adw]

Dao, could you take a look, please?

Comment 4

[Dão Gottwald [:dao]]

This should be graytext, i.e. the standard color for "disabled" items; the value comes from the OS and should be consistent with other places in Firefox. I think it makes sense to use that color here since this is the only piece of the autocomplete popup that isn't interactive, but verdi may want to reconsider.

Comment 5

[Verdi [:verdi] Best to slack me]

(In reply to Dão Gottwald [::dao] from comment #4)

This should be graytext, i.e. the standard color for "disabled" items; the value comes from the OS and should be consistent with other places in Firefox. I think it makes sense to use that color here since this is the only piece of the autocomplete popup that isn't interactive, but verdi may want to reconsider.

FWIW, I wasn't aware that this came from the OS. Looking at it now, I see that the copy in the current one-offs on Mac fails contrast accessibility by a wide margin. My intention was to use the "Secondary Color" (defined in https://design.firefox.com/photon/visuals/typography.html ), Grey 50 #737373.

Additionally, the settings icon should use Grey 90 a80 rgba(12, 12, 13, 0.8) - https://design.firefox.com/photon/components/buttons.html

Comment 6

[Dão Gottwald [:dao]]

The problem with a custom secondary color is that it won't properly work for high contrast themes. We also don't currently detect OS themes that make popups dark, for which the design wants a different color value. This is why we generally use graytext for this kind of thing. I don't think fixing the implied problems with a custom secondary color is within the scope for this bug, so our options are:

• use the custom color only on Mac where we don't have to deal with high contrast and dark OS themes
• stop de-emphasizing this label
• wontfix this bug

Which option do you prefer?

Comment 7

[Verdi [:verdi] Best to slack me]

(In reply to Dão Gottwald [::dao] from comment #6)

The problem with a custom secondary color is that it won't properly work for high contrast themes. We also don't currently detect OS themes that make popups dark, for which the design wants a different color value. This is why we generally use graytext for this kind of thing. I don't think fixing the implied problems with a custom secondary color is within the scope for this bug, so our options are:

• use the custom color only on Mac where we don't have to deal with high contrast and dark OS themes
• stop de-emphasizing this label
• wontfix this bug

Which option do you prefer?

Got it. Thanks for the explanation. In that case, let's stop de-emphasizing the label.

Comment 8

[Dão Gottwald [:dao]]

(In reply to Verdi [:verdi] NI or PM me from comment #7)

(In reply to Dão Gottwald [::dao] from comment #6)

The problem with a custom secondary color is that it won't properly work for high contrast themes. We also don't currently detect OS themes that make popups dark, for which the design wants a different color value. This is why we generally use graytext for this kind of thing. I don't think fixing the implied problems with a custom secondary color is within the scope for this bug, so our options are:

• use the custom color only on Mac where we don't have to deal with high contrast and dark OS themes
• stop de-emphasizing this label
• wontfix this bug

Which option do you prefer?

Got it. Thanks for the explanation. In that case, let's stop de-emphasizing the label.

Just to be sure, should we stop doing it on all platforms? graytext translates to different colors on different platforms which is why this bug pretty much only manifests on Mac.

Comment 9

[Dão Gottwald [:dao]]

Also, the engine label in the search bar popup uses the same color. What should happen with that?

Comment 10

[Dão Gottwald [:dao]]

(In reply to Dão Gottwald [::dao] from comment #9)

Also, the engine label in the search bar popup uses the same color. What should happen with that?

FWIW, one option would be to make that text black as well, and bring back the gray background that we used to have there.

Comment 11

[Verdi [:verdi] Best to slack me]

Attachment: trackingprotectionmenu.png

(In reply to Dão Gottwald [::dao] from comment #8)

Just to be sure, should we stop doing it on all platforms? graytext translates to different colors on different platforms which is why this bug pretty much only manifests on Mac.

So I talked to shorlander about this and he said we've fixed this issue in other places (e.g. the tracking protection menu). Instead of stop de-emphasizing the label we should use the solution that was used in the tracking protection menu instead.

(In reply to Dão Gottwald [::dao] from comment #9)

Also, the engine label in the search bar popup uses the same color. What should happen with that?

We can also use the tracking protection menu solution there.

Comment 12

[Dão Gottwald [:dao]]

(In reply to Verdi [:verdi] NI or PM me from comment #11)

So I talked to shorlander about this and he said we've fixed this issue in other places (e.g. the tracking protection menu).

No we haven't, it suffers exactly from the flaws I outlined earlier.

Comment 13

[Verdi [:verdi] Best to slack me]

Ok, talked with Dão this morning and then with shorlander in slack. We decided that we should use a custom color only Mac and continue using graytext everywhere else. We also want to make the change here specific to the address bar and search bar.

Dão - shorlander and I discussed doing the global change to graytext on mac but that would remove that lighter disabled state which we'd like to keep. In fact, we should figure out how to do that on other OSs but, like you said, that's out of scope for this bug.

Comment 14

[Dão Gottwald [:dao]]

Attachment: Bug 1577740 - Set custom disabled text color for urlbar and searchbar results since native graytext is too faint on Mac. r=harry

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/007d27ffbe94
Set custom disabled text color for urlbar and searchbar results since native graytext is too faint on Mac. r=harry

Comment 16

[Stefan Hindli [:stefan_hindli]]

Backed out changeset 007d27ffbe94 (Bug 1577740) for causing bc permafailures in toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=007d27ffbe94dc6bdc95520660a61c705dd461c6&selectedJob=268183710

https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268183710&repo=autoland&lineNumber=4624

Comment 17

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e4642f42ad29
Backed out changeset 007d27ffbe94 for causing bc permafailures in toolkit/components/extensions/test/browser/browser_ext_themes_autocomplete_popup.js CLOSED TREE

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7f009ca5a251
Set custom disabled text color for urlbar and searchbar results since native graytext is too faint on Mac. r=harry

Comment 19

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/7f009ca5a251

Comment 20

[Dão Gottwald [:dao]]

Comment on attachment 9094881 [details]
Bug 1577740 - Set custom disabled text color for urlbar and searchbar results since native graytext is too faint on Mac. r=harry

Beta/Release Uplift Approval Request

• User impact if declined: see comment 0
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Using the default theme, type in the urlbar and searchbar and check if de-emphasized labels are readable. There should be no change for the Dark and Light themes.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): We're just changing the value of the --panel-disabled-color CSS variable for the urlbar and searchbar result panels for the default theme on Mac.
• String changes made/needed:

Comment 21

[Liz Henry (:lizzard) (relman/hg->git project)]

Comment on attachment 9094881 [details]
Bug 1577740 - Set custom disabled text color for urlbar and searchbar results since native graytext is too faint on Mac. r=harry

Fix for accessibility issue, looks fine for uplift for beta 11.
We can verify in beta.

Comment 22

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3aaaeea6dd03

Comment 23

[Daniel Cicas [:dcicas], Release QA]

Hello,

I can conform this issue is fixed on Fx 70.0b11 and Fx 71.0a1 (20191001041624) on mac OS 10.14.5.