Bug 212952

Summary: Remove "no-preference" value for prefers-color-scheme media query
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: CSSAssignee: Tim Nguyen (:ntim) <ntim>
Status: RESOLVED FIXED    
Severity: Normal CC: ddkilzer, eoconnor, eric.carlson, esprehn+autocc, ews-watchlist, glenn, gyuyoung.kim, jer.noble, koivisto, macpherson, megan_gardner, menard, ntim, philipj, sergio, simon.fraser, timothy, webkit-bug-importer
Priority: P2 Keywords: InRadar, WebExposed
Version: Safari Technology Preview   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

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>