1702258 - Fix browser_selectpopup_colors.js which fails on Win10 with browser.proton.contextmenus.enabled

Status: RESOLVED FIXED

(Firefox :: Menus, defect)

Description

[:Gijs (back August 25; he/him)]

+++ This bug was initially created as a clone of Bug #1701963 +++

Splitting this off from bug 1701963 as I think we need some more changes to the select popup... :-(

https://treeherder.mozilla.org/jobs?repo=try&revision=ed2bdd264727e17c7b790a68167b40051666275c

Comment 1

[:Gijs (back August 25; he/him)]

So this test fails right now, and I'm not 100% sure how to proceed. You can reproduce locally by running ./mach mochitest browser/base/content/test/forms/browser_selectpopup_colors.js --setpref 'browser.proton.enabled=true' --setpref 'browser.proton.contextmenus.enabled=true' on your win10 machine.

The gist of how things work right now is:

1. websites can specify custom foreground/background colors for select dropdowns, and for individual options
2. we collect styling for the select dropdown in the SelectChild actor. We collect values for all the properties we support and send them to the parent. We also collect those values for the "default" style and send those in as UA styles.
3. the parent compares these with the UA style (which we also send up from the child) and does some other sanity checks like dealing with transparent background colours needing to "fall back" to the default background.

There are, AFAICT, no checks for contrast - if the site specifies dumb combinations, it gets them.

For proton on win10, we're changing the default background colour to be not-quite-white and the default foreground colour to be not-quite-black. Additionally, in the dark theme, we use a dark background and light foreground colour.

In https://phabricator.services.mozilla.com/D109751 I tried to fix some of the issues with contrast / flickering (cf. bug 1692593) by overwriting the "UA style" with the proton style. I didn't realize this effectively breaks the "is this the default style" checks in the SelectParent.

This is getting flagged up in the test because it checks things like setting a transparent background colour and no foreground colour. I would have assumed this would lead to proton styling being applied, but we actually end up applying the proton background colour and the "site provided" (but not really - implicitly deduced in the child from the UA / default computed style!) foreground colour. That isn't going to work well. I mean, I can just fix the test to pass but it'll look bad in dark mode.

I'm not sure how to proceed here and reconcile the seemingly-conflicting goals of:

1. respecting site colours
2. not ending up with a proton foreground vs site background colour or vice versa (and thus unreadable selectboxes through no fault of the site), or (like in JIRA's case) first having a black-on-almost-white dropdown and then, when the site updates the colours while the popup is up, switching to default proton colours, causing flashing
3. ideally not unnecessarily overriding proton colours when the site hasn't actually set any colours.

Emilio, do you see a practical way forward here?

Comment 2

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1702258 - Fix interaction of proton with custom styles. r=Gijs

If the site specifies the background-color, we also specify the color to the
HTML UA style. This fixes the msn issue in a better way.

Unstyled selects would still get dark mode.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1ff313897354
Fix interaction of proton with custom styles. r=Gijs

Comment 4

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/1ff313897354