1628601 - [Megabar] --urlbar-popup-url-color and --urlbar-popup-action-color should not depend on popup colors

Status: VERIFIED FIXED

(Firefox :: Theme, defect, P1)

Description

[Tyson F Nuss]

Attachment: Megabar Popup Bug.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:75.0) Gecko/20100101 Firefox/75.0

Steps to reproduce:

1. Apply a custom theme that specifies a dark popup background color and light popup_text color -- e.g.:
   https://addons.mozilla.org/en-US/firefox/addon/charcoal-dark-lite/

2. Focus Megabar and start typing to expand the autocomplete popup menu.

Actual results:

A) Autocomplete popup apparently takes the theme's toolbar_field background and toolbar_field_text values, rather than popup and popup_text (curiously, this does not affect the native Dark theme);

B) Worse, legibility is degraded for the autocomplete suggestion items' URL and Action text colors, which are still apparently auto-calculated from the theme's declared popup colors, rather than the actual applied colors -- cf.:
https://hacks.mozilla.org/2018/07/dark-theme-darkening-better-theming-for-firefox-quantum/

C) Theme key documentation does not specify any other way to deliberately theme those ac-url and ac-action text colors:
https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/manifest.json/theme

Expected results:

Megabar autocomplete popup menu should apply theme's declared popup and popup_text colors; however, in case that may be deemed a WONTFIX, then themes need some way to specify ac-url and ac-action text colors.

Comment 1

[Dão Gottwald [:dao]]

(In reply to Tyson F Nuss from comment #0)

A) Autocomplete popup apparently takes the theme's toolbar_field background and toolbar_field_text values, rather than popup and popup_text (curiously, this does not affect the native Dark theme);

toolbar_field_focus and toolbar_field_text_focus are used here. When not specified, these colors fall back to toolbar_field and toolbar_field_text with a minimum transparency enforced (see bug 1586054).

B) Worse, legibility is degraded for the autocomplete suggestion items' URL and Action text colors, which are still apparently auto-calculated from the theme's declared popup colors, rather than the actual applied colors -- cf.:

Indeed, we need to fix this: https://searchfox.org/mozilla-central/rev/4e228dc5f594340d35da7453829ad9f3c3cb8b58/browser/themes/shared/urlbar-autocomplete.inc.css#30-32,35-37

Comment 2

[Tyson F Nuss]

I've updated the Charcoal Dark Lite theme to v2.1 v2.2 with a workaround for the legibility aspect of this bug (setting popup to a light color), so note that loading v2.0 will now be necessary to reproduce that aspect if needed.

Updating this theme revealed something else:
Setting the popup background to #CCCCCC lost all cursor hover styling in non-megabar popup panels, regardless of mouse or arrow-key selection. Apparently popup_highlight and popup_highlight_text only affect the megabar autocomplete dropdown, while other popups use some non-themable native algorithm for hover styling, which produces no effective difference when a theme's popup background happens to be #CCCCCC.

Comment 3

[Tim Nguyen :ntim]

FWIW, there's [lwt-toolbar-field-brighttext] that can be used to replace [lwt-popup-brighttext]

Comment 4

[Tyson F Nuss]

Thanks, I see I can target those selectors in userchrome.css for my own use, but that would not help other users of my theme, unless there's an undocumented way to do that in a theme's manifest.json?

Comment 5

[Dão Gottwald [:dao]]

Attachment: Bug 1628601 - Stop using lwt-popup-darktext / lwt-popup-brighttext for urlbar results. r=ntim

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7d9533027552
Stop using lwt-popup-darktext / lwt-popup-brighttext for urlbar results. r=ntim

Comment 7

[Arthur Iakab [arthur_iakab]]

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

Comment 8

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:dao, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Comment 9

[Dão Gottwald [:dao]]

Comment on attachment 9142053 [details]
Bug 1628601 - Stop using lwt-popup-darktext / lwt-popup-brighttext for urlbar results. r=ntim

Beta/Release Uplift Approval Request

• User impact if declined: Address bar results' URLs and action labels might be hard to read with some webextension themes
• 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: This can be tested with this theme: https://addons.mozilla.org/en-US/firefox/addon/wine-abstract-light-by-m-donna/
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Pretty straightforward patch
• String changes made/needed:

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9142053 [details]
Bug 1628601 - Stop using lwt-popup-darktext / lwt-popup-brighttext for urlbar results. r=ntim

This isn't a new regression and the patch looks like a lot to take with no more Beta builds remaining this cycle before Monday's RC. Let's let this ride 77.

Comment 11

[Alexandru Trif, Desktop QA [:atrif]]

Reproduced the issue using Firefox 77.0a1 (20200409131623) on Windows 10x64 and theme from comment 9 and https://addons.mozilla.org/en-US/firefox/addon/charcoal-dark-lite/ theme version 2.0.
The issue is verified fixed with Firefox 77.0a1 (20200426215109) on Windows 10x64, macOS 10.12 and Ubuntu 18.04. The results are now eligible using the affected theme Charcoal Dark Lite( version 2.0 and 2.2) and Wine Abstract Light by M♥Donna.