1736141 - prefers-color-scheme should not default to dark for Elemental Balanced

Status: RESOLVED FIXED

(Firefox :: Theme, defect)

Description

[Brian Birtles (:birtles, Feb 26~ parental leave)]

I'm using the Elemental Balanced theme which has a light new tab page but mostly dark chrome.

Bug 1529323 causes prefers-color-scheme to default to dark which is surprising to me as I would expect it to follow what the new tab page does.

Comment 1

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Hi Emilio, needinfo'ing you as requested in bug 1529323 comment 55. Thanks!

Comment 2

[Emilio Cobos Álvarez (:emilio)]

Yeah, so I'd argue this is a bug (or inconsistency) in the theme code. We derive the preferred color scheme from the toolbar background color (which in this theme is dark).

This inconsistency was already present before my patch. The newtab page is light-ish, but all the other bits of the browser chrome like about:addons, about:preferences, etc were dark.

We could split the preference for content in two using the newtab page colors I guess, let me check how hard would it be.

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1736141 - Make Document::PreferredColorScheme safe to call off-main-thread. r=mstange

Document::GetBrowsingContext uses a WeakPtr<nsIDocShell> so we can't do
that. But we have all the other media emulation data and so on in the
pres context which we can access off the main thread for style, so move the
override code there.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1736141 - Derive a content theme from newtab page background, and use it for non-chrome docshells. r=mstange!,dao!

We use "is-in-chrome-docshell" rather than "is a chrome doc" so that about:
pages that are loaded in the content area (like about:addons etc) follow the
general content theme as well.

Cache the relevant color schemes since having that many branches on the default
system theme made me a bit uncomfortable, and this code can be called quite a
lot... Though it probably isn't such a huge deal.

Depends on D128673

Comment 5

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1736141 - Remove unneeded observer in devtools theme code. r=jdescottes

The toolbar pref change triggers eventually a look-and-feel-changed
notification, so no need to observe that directly.

Depends on D128674

Comment 6

[Brian Birtles (:birtles, Feb 26~ parental leave)]

Thanks! Distinguishing between dark mode for chrome vs content makes a lot more sense to me.

Comment 7

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

Set release status flags based on info from the regressing bug 1529323

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/affb20d60222
Make Document::PreferredColorScheme safe to call off-main-thread. r=mstange

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/59c6e74da9db
Remove unneeded observer in devtools theme code. r=jdescottes

Comment 10

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/affb20d60222
https://hg.mozilla.org/mozilla-central/rev/59c6e74da9db

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/63dee7b57626
Derive a content theme from newtab page background, and use it for non-chrome docshells. r=mstange,dao

Comment 12

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/63dee7b57626