1692224 - Select elements use same foreground/background color in devtools with dark theme and NNT enabled, or on all content with widget.content.allow-gtk-dark-theme=true

Status: RESOLVED FIXED

(Core :: Widget, defect)

Description

[Danny Colin [:sdk]]

Attachment: Screenshot_2021-02-11 firefox-devtools-nnt png (PNG Image, 1920 × 1056 pixels) — Scaled (92%).png

STR

0. Open devtools (ctrl+shift+i)
1. Press F1 to open the settings panel
2. Enable dark theme

EB

Select elements should use a black text on a white background

AB

Select elements use a white text on a white background

Comment 1

[Stephen A Pohl [:spohl]]

It was an explicit decision for non-native form controls to not detect and/or respond to dark mode. Presumably, the text for the select elements is CSS styled, correct? I'd suggest either styling the select itself, or not changing the font color to white when the non-native theme is enabled.

Comment 2

[Danny Colin [:sdk]]

(In reply to Stephen A Pohl [:spohl] from comment #1)

Presumably, the text for the select elements is CSS styled, correct? I'd suggest either styling the select itself, or not changing the font color to white when the non-native theme is enabled.

I don't know. You'll have to ask that to the Firefox Devtools team. Should I change the product/component to Devtools::General?

Comment 3

[Stephen A Pohl [:spohl]]

Yes, let's reassign this to get some eyes on this. Thank you!

Comment 4

[Emilio Cobos Álvarez [:emilio]]

So this seems a more general issue with any code that runs in the parent process with a dark GTK theme.

The ultimate issue is, on one hand, that widget.content.allow-gtk-dark-theme effectively only works at the content process boundary, and not for content documents that run in the parent process. But even with that fixed, it'd be an issue if you flip that pref on for regular content.

Two options:

• The first, which is easier, is: Disable non-native theme on the parent process altogether, disable allow_gtk_dark_theme if non-native-theme is enabled.

• The second, which should be easy too, would be to make the system colors that we use on forms.css account for non-native-theme (which I think would be relatively trivial too)...

I think I want to try the second one, but we'll see. Anyhow this is not devtools-specific.

Comment 5

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1692224 - Use standins for system colors pairs that paint over non-native theme widgets to guarantee contrast. r=spohl,mstange

Nothing like having the code already written for you :-)

Comment 6

[Emilio Cobos Álvarez [:emilio]]

Attachment: Bug 1692224 - Remove ui.use_native_colors. r=mstange,spohl

It's not set to false anywhere and the browser would be quite broken
with it set to false.

Depends on D104905

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3b1604a7313a
Use standins for system colors pairs that paint over non-native theme widgets to guarantee contrast. r=spohl
https://hg.mozilla.org/integration/autoland/rev/530980f4a015
Remove ui.use_native_colors. r=spohl

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b9f69753275d
make test anonymous-button-content-box.html and test propagate-text-decoration.html pass on linux r=test-only. CLOSED TREE

Comment 9

[Andreea Pavel [:apavel]]

Hi Emilio, your changes cased web platform reftest failures and we set the tests to pass. If that's not the way you want to fix it, please revert the changes. Thank you.
Failure log: https://treeherder.mozilla.org/logviewer?job_id=329733210&repo=autoland&lineNumber=19121

[task 2021-02-12T04:20:17.409Z] 04:20:17 INFO - TEST-START | /html/rendering/widgets/button-layout/anonymous-button-content-box.html
[task 2021-02-12T04:20:17.478Z] 04:20:17 INFO - PID 3366 | 1613103617476 Marionette INFO Testing http://web-platform.test:8000/html/rendering/widgets/button-layout/anonymous-button-content-box.html == http://web-platform.test:8000/html/rendering/widgets/button-layout/anonymous-button-content-box-ref.html
[task 2021-02-12T04:20:18.426Z] 04:20:18 INFO - PID 3366 | 1613103618425 Marionette INFO No differences allowed
[task 2021-02-12T04:20:18.471Z] 04:20:18 INFO - TEST-UNEXPECTED-PASS | /html/rendering/widgets/button-layout/anonymous-button-content-box.html | Testing http://web-platform.test:8000/html/rendering/widgets/button-layout/anonymous-button-content-box.html == http://web-platform.test:8000/html/rendering/widgets/button-layout/anonymous-button-content-box-ref.html

[task 2021-02-12T04:20:22.935Z] 04:20:22 INFO - TEST-START | /html/rendering/widgets/button-layout/propagate-text-decoration.html
[task 2021-02-12T04:20:22.941Z] 04:20:22 INFO - PID 3891 | 1613103622939 Marionette INFO Testing http://web-platform.test:8000/html/rendering/widgets/button-layout/propagate-text-decoration.html == http://web-platform.test:8000/html/rendering/widgets/button-layout/propagate-text-decoration-ref.html
[task 2021-02-12T04:20:23.257Z] 04:20:23 INFO - PID 3891 | 1613103623256 Marionette INFO No differences allowed
[task 2021-02-12T04:20:23.318Z] 04:20:23 INFO - TEST-UNEXPECTED-PASS | /html/rendering/widgets/button-layout/propagate-text-decoration.html | Testing http://web-platform.test:8000/html/rendering/widgets/button-layout/propagate-text-decoration.html == http://web-platform.test:8000/html/rendering/widgets/button-layout/propagate-text-decoration-ref.html

Comment 10

[Emilio Cobos Álvarez [:emilio]]

No, that's awesome, thank you!

Comment 11

[Emilio Cobos Álvarez [:emilio]]

Ah, probably will need to add a and non_native_theme: to the expectation so they don't fail on beta.

Comment 12

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/3b1604a7313a
https://hg.mozilla.org/mozilla-central/rev/530980f4a015
https://hg.mozilla.org/mozilla-central/rev/b9f69753275d