CSS prefers-color-scheme media feature doesn't update

VERIFIED FIXED in Firefox 67

Status

()

defect
P2
normal
VERIFIED FIXED
2 months ago
2 months ago

People

(Reporter: sven.knoetgen, Assigned: boris)

Tracking

67 Branch
mozilla67
Points:
---

Firefox Tracking Flags

(firefox65 unaffected, firefox66 unaffected, firefox67+ verified)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

2 months ago
Posted file index.html

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.14; rv:67.0) Gecko/20100101 Firefox/67.0

Steps to reproduce:

Operating System: macOS 10.14.1 (Mojave)
Firefox Version: Nightly 67.0a1 (2019-02-19) (64-bit)

Steps to reproduce:

  • Create a basic html file containing an inline style tag utilizing the CSS media query prefers-color-scheme to switch between the background-color of the document (like the attached "index.html" file)
  • Serve the file via a webserver (I used Apache/2.4.34 provided from macOS)
  • Navigate to the file
  • Switch between dark- and light-mode of the operating system

Actual results:

The background color changes initially to the defined value but switching between light and dark mode has no effect on the background color. Opening the document in a new Tab adapts the current system state but also doesn't update to any mode switch.

However, when the file is being opened from the local filesystem instead (using file://) the CSS definition works as expected.

Expected results:

The styling should update to the defined values in the stylesheet on switching between dark and light mode

Component: Untriaged → Theme
Blocks: 1494034
Component: Theme → CSS Parsing and Computation
Product: Firefox → Core

I can take a look, this is also reproducible tweaking ui.systemUsesDarkTheme, so should be pretty easy to write a test for this.

Flags: needinfo?(emilio)

[Tracking Requested - why for this release]: Would be bad to ship prefers-color-scheme without this bug fixed.

So, we don't react to the ui.* pref changes using NotifyThemeChanged(). Is that expected Jimm?

Anyhow, even with that fixed (I can spawn a different bug for that), I think we wouldn't detect the actual dark mode changing on Mac, and we probably need something similar to what prefersReducedMotion did.

I don't have a Mac or Windows machines at all... First, can anybody double-check that the dark theme properly reacts to changes in Windows? I suspect it goes via ThemeChanged(), which should take care of that.

I'd be surprised otherwise given we have code in the tree listening to that:

https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/toolkit/modules/LightweightThemeConsumer.jsm#139

Anyhow. Probably Hiro or Markus can provide mentorship for this bug. It probably shouldn't be too hard... I could work on it at the beginning of March when I'm back at the office. Though if somebody wants to steal it please be my guest.

Component: CSS Parsing and Computation → Widget: Cocoa
Flags: needinfo?(emilio) → needinfo?(jmathies)

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

I don't have a Mac or Windows machines at all... First, can anybody double-check that the dark theme properly reacts to changes in Windows? I suspect it goes via ThemeChanged(), which should take care of that.

I'd be surprised otherwise given we have code in the tree listening to that:

https://searchfox.org/mozilla-central/rev/93905b660fc99a5d52b683690dd26471daca08c8/toolkit/modules/LightweightThemeConsumer.jsm#139

Yes, that works on Windows, but note that it uses -moz-system-dark-theme instead of prefers-color-scheme: dark. Not sure how much of their implementation is shared. (The use case is the same, and once prefers-color-scheme: dark works as expected we should get rid of -moz-system-dark-theme.)

Status: UNCONFIRMED → NEW
Ever confirmed: true

(In reply to Dão Gottwald [::dao] from comment #3)

Yes, that works on Windows, but note that it uses -moz-system-dark-theme instead of prefers-color-scheme: dark. Not sure how much of their implementation is shared. (The use case is the same, and once prefers-color-scheme: dark works as expected we should get rid of -moz-system-dark-theme.)

They use the same underlying mechanism, so you should be able to do that right now. I'd be surprised if (-moz-system-dark-theme) changes worked on Mac.

Can the up-to-date value query in the parent process properly? If so, we probably need to do what I did for prefers-reduced-motion. https://hg.mozilla.org/mozilla-central/rev/cea8a92452d5

(Assignee)

Comment 6

2 months ago

(In reply to Emilio Cobos Álvarez (:emilio) from comment #4)

They use the same underlying mechanism, so you should be able to do that right now. I'd be surprised if (-moz-system-dark-theme) changes worked on Mac.

Ya, just tried to enable Apache on my mac to verify this (i.e. using http://localhost/index.html instead of file:///.../index.html). It seems both (prefers-color-scheme: dark) and (-moz-system-dark-theme) don't work on mac with dark mode.

(In reply to Emilio Cobos Álvarez (:emilio) from comment #2)

[Tracking Requested - why for this release]: Would be bad to ship
prefers-color-scheme without this bug fixed.

So, we don't react to the ui.* pref changes using NotifyThemeChanged(). Is
that expected Jimm?

Pretty sure that call is associated with a native change vs. our prefs. Looking over some of the ui.* pref change handling it looks like we update or notify on a per pref basis as needed.

Flags: needinfo?(jmathies)
Priority: -- → P2

Triaged this to P2 (even though it's in Widget: Cocoa) since we want to fix this soon to ship prefers-color-scheme and layout folks will be working on it.

(Assignee)

Updated

2 months ago
Assignee: nobody → boris.chiou
(Assignee)

Comment 9

2 months ago

Based on the implementation and issues on prefers-reduced-motion, it
seems we have the same issue on the dark mode.

In child processes on MacOSX we don't spin native event loop at all.
Without native event loops, the global preference returned from
SystemWantsDarkTheme() doesn't return up-to-date value when the system
setting changed for some reasons.
To workaround this we call SystemWantsDarkTheme() only on the parent
process which spins native event loop or when it's the initial query on
the child process. And we give the up-to-date value to the child process
via an IPC call just like other cached values do.

(Assignee)

Comment 10

2 months ago

We use the same idea (Bug 1486971) to write the test case. In this test,
we set the boolean value of SystemUsesDarkTheme, and check
prefers-color-scheme media queries.

(Assignee)

Comment 11

2 months ago

I notice that our try servers still use MacOS 10.10, so the test case I wrote is not useful for now because there is no equivalent notification of AppleInterfaceThemeChangedNotification on MacOS 10.10, I think. Maybe we just need a QA verification.

Attachment #9047210 - Attachment is obsolete: true

Given that we can't run the test on try servers, I'll attempt to submit a manual QA request.

I've written a simple manual test case that works as expected in Safari Technology Preview – the only browser that currently supports prefers-color-scheme (Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_2) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/12.2 Safari/605.1.15).

https://codepen.io/svoisen/pen/VRegLe

Of course we will need to first need to land the patch in comment 9.

(Assignee)

Comment 13

2 months ago

Thanks, Sean. I am going to land this patch. :)

Comment 14

2 months ago
Pushed by boris.chiou@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/2b32f21ec0dc
Query MacOS system dark mode only on the main browser process or it's the initial query on the child process. r=hiro

Comment 15

2 months ago
bugherder
Status: NEW → RESOLVED
Last Resolved: 2 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67
(Assignee)

Updated

2 months ago
Flags: qe-verify+

Updated

2 months ago
Whiteboard: [qa-triaged]
QA Whiteboard: [qa-triaged]
Whiteboard: [qa-triaged]

This is not a regression.

Keywords: regression

I successfully verified that the color scheme updates accordingly using the webserver method from Comment 0 and by using the test page that Sean created in Comment 12.

Tests were performed on Firefox Nightly 67.0a1 (2019-03-06) under Windows 10 (x64), macOS 10.12 and Ubuntu 18.04 (x64).

Status: RESOLVED → VERIFIED
Flags: qe-verify+

Great! Thanks to everyone got involved in this bug. :)

You need to log in before you can comment on or make changes to this bug.