Closed Bug 1577740 Opened 5 years ago Closed 5 years ago

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

Categories

(Firefox :: Address Bar, defect, P1)

Unspecified
macOS
defect
Points:
1

Tracking

()

VERIFIED FIXED
Firefox 71
Iteration:
71.2 - Sept 16 - 29
Tracking Status
firefox-esr60 --- unaffected
firefox-esr68 --- unaffected
firefox69 --- unaffected
firefox70 + verified
firefox71 --- verified

People

(Reporter: Gijs, Assigned: dao)

References

(Regression)

Details

(Keywords: access, regression)

Attachments

(3 files)

Attached image Screenshot 2019-08-30 at 11.42.22.png (deleted) —

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

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

Points: --- → 1
Priority: -- → P1

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

Flags: needinfo?(adw)

Dao, could you take a look, please?

Component: Address Bar → Theme
Flags: needinfo?(adw) → needinfo?(dao+bmo)

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.

Component: Theme → Address Bar
Flags: needinfo?(dao+bmo) → needinfo?(mverdi)
Regressed by: 1561894

(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

Flags: needinfo?(mverdi)

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?

Flags: needinfo?(mverdi)

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

Flags: needinfo?(mverdi)

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

Flags: needinfo?(mverdi)

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

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

Attached image trackingprotectionmenu.png (deleted) —

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

Flags: needinfo?(mverdi)

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

Flags: needinfo?(mverdi)

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.

Flags: needinfo?(mverdi)
Iteration: --- → 71.2 - Sept 16 - 29
Pushed by dgottwald@mozilla.com: 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
Backout by shindli@mozilla.com: 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
Pushed by dgottwald@mozilla.com: 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
Flags: needinfo?(dao+bmo)
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 71
Flags: qe-verify+

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:
Attachment #9094881 - Flags: approval-mozilla-beta?
QA Whiteboard: [qa-triaged]

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.

Attachment #9094881 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Hello,

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

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Has Regression Range: --- → yes
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: