Bug 190499 – Add support for prefers-color-scheme media query

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Timothy Hatcher]

The prefers-color-scheme media query should expose dark mode on macOS Mojave.

[Radar WebKit Bug Importer]

<rdar://problem/45212025>

[Timothy Hatcher]

Created attachment 352111 [details]
Patch

[Timothy Hatcher]

Created attachment 352192 [details]
Patch

[Dean Jackson]

Comment on attachment 352111 [details]
Patch

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

> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:100
> +ENABLE_DARK_MODE_CSS = $(ENABLE_DARK_MODE_CSS_$(WK_PLATFORM_NAME));
> +ENABLE_DARK_MODE_CSS_macosx = ENABLE_DARK_MODE_CSS;

Why not do the bit from FeatureDefines.h here too?

I think it would be something like this:

ENABLE_DARK_MODE_CSS_macosx = $(ENABLE_DARK_MODE_CSS$(WK_MACOS_1014));
ENABLE_DARK_MODE_CSS_MACOS_SINCE_1014 = ENABLE_DARK_MODE_CSS;

> Source/WTF/wtf/FeatureDefines.h:207
> +#if !defined(ENABLE_DARK_MODE_CSS)
> +#define ENABLE_DARK_MODE_CSS __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
> +#endif

I think you can avoid any changes in FeatureDefines.h by using the macros above.

> Source/WebCore/css/MediaQueryEvaluator.cpp:48
> +#include "RuntimeEnabledFeatures.h"

Amazing that this is the first media query to be runtime enabled!

> Source/WebCore/css/MediaQueryEvaluator.cpp:733
> +    ASSERT(RuntimeEnabledFeatures::sharedFeatures().darkModeCSSEnabled());

I wonder if we'd be better returning false if it isn't enabled? I guess that means it looks like the media query "worked"

> Source/WebCore/css/MediaQueryExpression.cpp:58
> +    || (mediaFeature == MediaFeatureNames::prefersColorScheme && RuntimeEnabledFeatures::sharedFeatures().darkModeCSSEnabled())

OK. Now I understand the ASSERT. Does this allow the WI to show that the query wasn't understood, as opposed to returning false?

> Source/WebKit/Shared/WebPreferences.yaml:1288
> +  humanReadableName: "Dark Mode CSS Support"
> +  humanReadableDescription: "Enable Dark Mode CSS Support"

Did you pick this name over prefers-color-scheme because you'll use it for the <meta> as well?

> LayoutTests/ChangeLog:10
> +        * css-dark-mode/perfers-color-scheme-expected.txt: Added.
> +        * css-dark-mode/perfers-color-scheme.html: Added.

perfers :)

[Timothy Hatcher]

Comment on attachment 352111 [details]
Patch

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

>> Source/WebKit/Shared/WebPreferences.yaml:1288
>> +  humanReadableDescription: "Enable Dark Mode CSS Support"
> 
> Did you pick this name over prefers-color-scheme because you'll use it for the <meta> as well?

Yes

[Timothy Hatcher]

Comment on attachment 352111 [details]
Patch

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

>> Source/JavaScriptCore/Configurations/FeatureDefines.xcconfig:100
>> +ENABLE_DARK_MODE_CSS_macosx = ENABLE_DARK_MODE_CSS;
> 
> Why not do the bit from FeatureDefines.h here too?
> 
> I think it would be something like this:
> 
> ENABLE_DARK_MODE_CSS_macosx = $(ENABLE_DARK_MODE_CSS$(WK_MACOS_1014));
> ENABLE_DARK_MODE_CSS_MACOS_SINCE_1014 = ENABLE_DARK_MODE_CSS;

Yeah, I should. Thanks.

>> Source/WebCore/css/MediaQueryExpression.cpp:58
>> +    || (mediaFeature == MediaFeatureNames::prefersColorScheme && RuntimeEnabledFeatures::sharedFeatures().darkModeCSSEnabled())
> 
> OK. Now I understand the ASSERT. Does this allow the WI to show that the query wasn't understood, as opposed to returning false?

Yep!

>> LayoutTests/ChangeLog:10
>> +        * css-dark-mode/perfers-color-scheme.html: Added.
> 
> perfers :)

Doh!

[Timothy Hatcher]

Created attachment 352356 [details]
Patch

[Timothy Hatcher]

Created attachment 352360 [details]
Patch

[EWS Watchlist]

Comment on attachment 352360 [details]
Patch

Attachment 352360 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/9594370

New failing tests:
css-dark-mode/prefers-color-scheme.html

[EWS Watchlist]

Created attachment 352375 [details]
Archive of layout-test-results from ews107 for mac-sierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-sierra-wk2  Platform: Mac OS X 10.12.6

[Timothy Hatcher]

Created attachment 352378 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 352378 [details]
Patch

Clearing flags on attachment: 352378

Committed r237156: <https://trac.webkit.org/changeset/237156>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.