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

Status: VERIFIED FIXED

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

Description

[Mike Conley (:mconley) (:⚙️)]

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

Comment 1

[Harry Twyford]

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.

Comment 2

[Harry Twyford]

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.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Patches up in bug 1715748.

Comment 4

[Harry Twyford]

Attachment: Bug 1710934 - Manage toolbar field selection colors manually based on theme colors. r?dao!,emilio

Comment 5

[Harry Twyford]

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

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/9f6b5145351a
Manage toolbar field selection colors manually based on theme colors. r=emilio,dao

Comment 7

[Sandor Molnar]

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

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7ad611c84b16
Manage toolbar field selection colors manually based on theme colors. r=emilio,dao

Comment 9

[Natalia Csoregi [:nataliaCs]]

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

Comment 10

[Pascal Chevrel:pascalc]

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

Comment 11

[Harry Twyford]

Attachment: Bug 1710934 - Hotfix - Re-enable urlbar-input:not(:focus) rule. r?dao!

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

Comment 12

[Pulsebot]

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

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/660fd0b1b386

Comment 14

[Harry Twyford]

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:

Comment 15

[Pascal Chevrel:pascalc]

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.

Comment 16

[Pascal Chevrel:pascalc]

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

Comment 17

[Timea Zsoldos [:zstimi/tzsoldos], Desktop Release QA]

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

Comment 18

[Timea Zsoldos [:zstimi/tzsoldos], Desktop Release QA]

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