Bug 212952 - Remove "no-preference" value for prefers-color-scheme media query
Summary: Remove "no-preference" value for prefers-color-scheme media query
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Nguyen (:ntim)
URL:
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2020-06-08 20:47 PDT by Simon Fraser (smfr)
Modified: 2021-05-03 10:37 PDT (History)
18 users (show)

See Also:


Attachments
Patch (3.54 KB, patch)
2021-05-02 10:21 PDT, Tim Nguyen (:ntim)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 Tim Nguyen (:ntim) 2021-05-02 10:21:04 PDT
Created attachment 427530 [details]
Patch
Comment 2 Antti Koivisto 2021-05-02 23:45:12 PDT
Comment on attachment 427530 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427530&action=review

> Source/WebCore/css/MediaQueryEvaluator.cpp:-811
>      switch (keyword) {
> -    case CSSValueNoPreference:
> -        return false;

The default for this switch is false so this doesn't seem to change anything at all.
Comment 3 Antti Koivisto 2021-05-03 00:21:43 PDT
Comment on attachment 427530 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=427530&action=review

> LayoutTests/css-dark-mode/older-systems/prefers-color-scheme.html:25
> -@media (prefers-color-scheme: no-preference) {
> +@media not (prefers-color-scheme) {

We may need to keep supporting 'no-preference' value even when parsing is fixed.
Comment 4 Antti Koivisto 2021-05-03 00:34:44 PDT
Comment on attachment 427530 [details]
Patch

I don't know if it is ok to remove 'no-preference' support but if it is, it can be done after the parsing fix. It should also be done properly by removing all the code, not by stealth test changes.
Comment 5 Tim Nguyen (:ntim) 2021-05-03 01:15:12 PDT
Comment on attachment 427530 [details]
Patch

Regarding tests: the test already tests for `foo` which is the invalid keyword case (like `no-preference`). WPT also has coverage for `no-preference`: https://wpt.fyi/results/css/mediaqueries/prefers-color-scheme.html?label=master&label=experimental&aligned&q=prefers-color-scheme

Regarding removing all the code: CSSValueNoPreference can't be removed since other MQ use it (prefers-contrast/prefers-reduced-motion).
Comment 6 Antti Koivisto 2021-05-03 09:21:21 PDT
Tim Hatcher says getting rid of this is ok.
Comment 7 EWS 2021-05-03 10:36:09 PDT
Committed r276908 (237253@main): <https://commits.webkit.org/237253@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 427530 [details].
Comment 8 Radar WebKit Bug Importer 2021-05-03 10:37:19 PDT
<rdar://problem/77461590>