Closed Bug 1710934 Opened 7 months ago Closed 5 months ago

Hard-to-read URL bar text when using a theme with brighttext toolbar and darktext URL bar

Categories

(Core :: Widget: Cocoa, defect, P2)

defect

Tracking

()

VERIFIED FIXED
92 Branch
Tracking Status
firefox91 + verified
firefox92 --- verified

People

(Reporter: mconley, Assigned: harry)

References

Details

(Whiteboard: [mac:darkmode][proton-address-bar])

Attachments

(3 files)

Attached image image.png

STR:

  1. On macOS, be running Nightly and ensure widget.macos.respect-system-appearance is set to true
  2. Install this theme: https://addons.mozilla.org/en-US/firefox/addon/blueshift/
  3. On any tab where there's pre-existing text in the URL bar, click on the URL bar to highlight it

ER:

The text should be readable - ideally dark text on a light background.

AR:

The text is dark on a dark background. See screenshot.

See Also: → 1700294
Severity: -- → S3
Priority: -- → P2
Whiteboard: [mac:darkmode]
Whiteboard: [mac:darkmode] → [mac:darkmode][proton-address-bar]

What's happening here is that we're now setting a darker highlight color on Mac when:

  • widget.macos.respect-system-appearance is true
  • the user has a brighttext lwtheme. This is qualified by whether the toolbar is brighttext. It looks like the blue color in that theme's toolbar qualifies.

That theme has a brighttext toolbar, so we use the darker selection colors. However, it also uses a darktext Urlbar. So we're using a dark selection color over black text. We store separate attributes for when the toolbar is brighttext, the unfocused Urlbar is brighttext (lwt-toolbar-field-brighttext) and when the focused Urlbar is brighttext (lwt-toolbar-field-focus-brighttext). We should probably make chrome text selection color depend on lwt-toolbar-field-focus-brighttext instead of toolbar brighttext.

I spoke with Emilio and he put a great patch together that adds a -moz-system-color(color_name, scheme) property accessible from CSS. That way, we could have logic in the Urlbar CSS to use -moz-system-color(text-select-background, light) in this context.

Flags: needinfo?(emilio)
Depends on: 1715748

Patches up in bug 1715748.

Flags: needinfo?(emilio)
Assignee: nobody → htwyford
Status: NEW → ASSIGNED
Blocks: 1719734

[Tracking Requested - why for this release]:

This is the last bug blocking the macOS dark mode support we'd like to land in 91. See bug 1710934.

Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/9f6b5145351a
Manage toolbar field selection colors manually based on theme colors. r=emilio,dao

Backed out for causing browser-chrome failures in toolkit/components/extensions/test/browser/browser_ext_themes_highlight.js

Backout link: https://hg.mozilla.org/integration/autoland/rev/0d1bbf9e767366b810abe835e6c5126dd7b0bc38

Push with failures

Failure log

Flags: needinfo?(htwyford)
Pushed by htwyford@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7ad611c84b16
Manage toolbar field selection colors manually based on theme colors. r=emilio,dao
Blocks: 1720490
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → 92 Branch
Blocks: 1720784
Flags: qe-verify+
Flags: needinfo?(htwyford)

Looks like I disabled this rule at some point during testing and a rebase issue drew it into the final patch.

Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/660fd0b1b386
Hotfix - Re-enable urlbar-input:not(:focus) rule. r=dao

Comment on attachment 9231659 [details]
Bug 1710934 - Hotfix - Re-enable urlbar-input:not(:focus) rule. r?dao!

Beta/Release Uplift Approval Request

  • User impact if declined: Unfocused Firefox windows will not have the correct appearance. Highlighted URLs will not have the "disabled" appearance.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): CSS-only patch. Fixes mistake from the previous patch.
  • String changes made/needed:
Attachment #9231659 - Flags: approval-mozilla-beta?

Comment on attachment 9231659 [details]
Bug 1710934 - Hotfix - Re-enable urlbar-input:not(:focus) rule. r?dao!

CSS fix, approved for 91 beta 5, thanks.

Attachment #9231659 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

I have reproduced this issue using Firefox 90.0a1 (2021-05-12) on macOS 10.15.
I can confirm this issue is fixed, I verified using Firefox 92.0a1 (2021-07-20) on macOS 10.15. Note that the pref widget.macos.respect-system-appearance was renamed to widget.macos.support-dark-appearance (see: bug 1715145, commnet 3).

I can confirm this issue is fixed, I verified using Firefox 91.0b5 on macOS 10.15.

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