Add support for CSS prefers-color-scheme media feature
Categories
(Core :: CSS Parsing and Computation, enhancement, P3)
Tracking
()
People
(Reporter: merihakar, Assigned: MatsPalmgren_bugz, Mentored)
References
(Blocks 2 open bugs)
Details
(Keywords: dev-doc-complete)
Attachments
(1 file, 3 obsolete files)
17.65 KB,
patch
|
emilio
:
review+
|
Details | Diff | Splinter Review |
Updated•6 years ago
|
Updated•6 years ago
|
Comment 2•6 years ago
|
||
Comment 3•6 years ago
|
||
Comment 4•6 years ago
|
||
Comment 5•6 years ago
|
||
Comment 6•6 years ago
|
||
Comment 7•6 years ago
|
||
Comment 8•6 years ago
|
||
Comment 9•6 years ago
|
||
Thinking aloud slightly, what should the media query match while printing? Light, because paper? Always false, because it's only meaningful on screens?
Comment 10•6 years ago
|
||
That's a good question. I think we shouldn't intermix the concept of printing and the user theme preference, but I could see the argument for the opposite.
Maybe worth filing an issue in https://github.com/w3c/csswg-drafts?
Also, Jonathan, are you actively working on this? Chromium is implemented, so it'd be great not to lag behind much given our spec questions were answered. Let me know if you want me (or someone else, quasicomputational? :)) to take over or what not (I'll make sure that the patch appears as co-authored by you of course).
Comment 11•6 years ago
|
||
Went ahead and filed https://github.com/w3c/csswg-drafts/issues/3522.
I don't mind taking this on, but I do think there's still some spec work to be done. On the other hand, both Chromium and Safari are gearing up to ship, so who knows if that'll get done before the current version's ossified...
Comment 12•6 years ago
•
|
||
I don't have cycles to complete this right at the moment, so if people want to take it on that is fine with me :).
but I do think there's still some spec work to be done.
Yeah I was kinda waiting for this to resolve before working on this further.
Thanks!
Updated•6 years ago
|
Assignee | ||
Comment 14•6 years ago
|
||
I agree with this comment:
https://github.com/w3c/csswg-drafts/issues/3278#issuecomment-452834498
In particular:
@media (prefers-color-scheme)
Simply means that the webbrowser at hand supports communicating it's users color-scheme preference.
so, I guess that means:
None => return prefers_dark_theme
should be:
None => return true
Other than that, is there anything else that needs fixing here?
Assignee | ||
Comment 16•6 years ago
|
||
OK, I made a few updates to reflect the latest spec issues, and I'll add some tests too...
Assignee | ||
Comment 17•6 years ago
|
||
A few substantial changes based on feedback in the github issues:
- if presContext->IsPrintingOrPrintPreview() is true, then return "light"
- if we fail to retrieve the LookAndFeel::eIntID_SystemUsesDarkTheme value, then return "no-preference" (this is currently the case for GTK: bug 1525775)
- we already have an integer pref, ui.systemUsesDarkTheme, where 0 means "light" and 1 means "dark"; I'm extending that so that 2 means "no-preference". This makes the name a slight misnomer, but I think it's more user friendly to continue to use the same name so that existing usage of this pref works as before rather than introducing a new (non-binary) name. It's a hidden pref anyway, so naming isn't that important.
- made "@media (prefers-color-scheme)" return true, since we now support this feature
- added reftests
Probably worth waiting a few more days until we have CSSWG resolutions on the github issues before we finalize this. I've added Agenda+ on both issues so hopefully we'll have answers soon. (Feel free to punt on the review until then if you wish.)
https://treeherder.mozilla.org/#/jobs?repo=try&revision=1841c5b4d825e8f5f16c8ac74795f1c35f6654c4
Comment 18•6 years ago
|
||
Assignee | ||
Comment 19•6 years ago
|
||
Assignee | ||
Comment 20•6 years ago
|
||
So it appears we now pass all the tests in
https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/mediaqueries/prefers-color-scheme.html
except the last one:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3247feecfcb9c376928afa0e9ec66a5235552d6d&selectedJob=227451411
which appears to test that "(prefers-color-scheme)" is always
the exact opposite of "(prefers-color-scheme: no-preference)".
This looks like a bogus test to me, at least with the assumption
that "(prefers-color-scheme)" is always true in an UA that
supports this feature (like we do with this patch).
Comment 21•6 years ago
|
||
Updated•6 years ago
|
Assignee | ||
Comment 22•6 years ago
|
||
Updated per clarifications from CSSWG. Intradiff:
fn eval_prefers_color_scheme(device: &Device, query_value: Option<PrefersColorScheme>) -> bool {
- let query_value = match query_value {
- Some(v) => v,
- None => return true,
- };
-
let prefers_color_scheme =
unsafe { bindings::Gecko_MediaFeatures_PrefersColorScheme(device.document()) };
- query_value == prefers_color_scheme
+ match query_value {
+ Some(v) => prefers_color_scheme == v,
+ None => prefers_color_scheme != PrefersColorScheme::NoPreference,
+ }
}
Comment 23•6 years ago
|
||
Comment 24•6 years ago
|
||
Well I guess you also need to remove the test expectations for the test mentioned in comment 20.
Comment 25•6 years ago
|
||
Oh, it's already in the patch, nvm :)
Assignee | ||
Comment 26•6 years ago
|
||
Comment 27•6 years ago
|
||
Comment 28•6 years ago
|
||
bugherder |
Comment 29•6 years ago
|
||
Just tested this on Nightly 67.0a1 (2019-02-22) on macOS. It detects the OS theme correctly, but it doesn't adapt to theme changes like Safari does, not even after reloading the page. You need to completely quit the browser and restart it for a new theme to be considered. window.matchMedia('(prefers-color-scheme: dark)').addListener()
handlers are also not fired (they are in Safari).
Comment 30•6 years ago
|
||
(In reply to felix.b from comment #29)
Just tested this on Nightly 67.0a1 (2019-02-22) on macOS. It detects the OS theme correctly, but it doesn't adapt to theme changes like Safari does, not even after reloading the page. You need to completely quit the browser and restart it for a new theme to be considered.
window.matchMedia('(prefers-color-scheme: dark)').addListener()
handlers are also not fired (they are in Safari).
That's bug 1528960.
Updated•6 years ago
|
Comment 31•6 years ago
|
||
Hey! This bug has been marked as fixed in v67. I'm using 67.0b8 (64-bit) on Ubuntu 18.10 (with the Unity desktop) - how do I use this & setup my preference to dark?
Comment 32•6 years ago
|
||
(In reply to Starbeamrainbowlabs from comment #31)
Hey! This bug has been marked as fixed in v67. I'm using 67.0b8 (64-bit) on Ubuntu 18.10 (with the Unity desktop) - how do I use this & setup my preference to dark?
Should be auto-detected from your GTK theme, and you can override it setting ui.systemUsesDarkTheme
to 1
.
Comment 33•6 years ago
|
||
(In reply to Emilio Cobos Álvarez (:emilio) from comment #32)
(In reply to Starbeamrainbowlabs from comment #31)
Hey! This bug has been marked as fixed in v67. I'm using 67.0b8 (64-bit) on Ubuntu 18.10 (with the Unity desktop) - how do I use this & setup my preference to dark?
Should be auto-detected from your GTK theme, and you can override it setting
ui.systemUsesDarkTheme
to1
.
Ah, I see - thanks! I don't know how to alter my GTK theme, so I'll just adjust the conf setting :-)
Comment 34•6 years ago
|
||
Update: I've set ui.systemUsesDarkTheme
to 1
, but the demo on MDN is not functioning for me - it's showing as black text on white, instead of white-on-black. Is it because it's in an iframe or something? Or is the feature in the next beta? I did have to create the about:config setting as it didn't already exist yet.
Updated•6 years ago
|
Comment 35•6 years ago
|
||
I've added a note about this to the Fx 67 rel notes:
https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/67#CSS
And it looks like some kind soul documented this already!
https://developer.mozilla.org/en-US/docs/Web/CSS/@media
https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-color-scheme
Thanks to whoever that was ;-)
I'd appreciate a tech review of this content.
Updated•6 years ago
|
Comment 36•6 years ago
|
||
Fixed it! After carefully re-reading this, you need to create the new about:config directive to be an integer
, not a string
. If you do accidentally create a string
, then you'll need to close Firefox and edit your prefs.js
file to correct it, as about:config
does not appear to be capable of this at the moment.
Comment 37•6 years ago
|
||
Dark Mode is available on this Bugzilla as of yesterday and now featured in the Hacks blog post for Firefox 67. Dark Mode itself has been planned for a while but prefers-color-scheme
has become a trigger for us. Thanks everyone for making this happen!
Description
•