Bug 190499

Summary:

Add support for prefers-color-scheme media query

Product:

WebKit

Reporter:

Timothy Hatcher <timothy>

Component:

CSS

Assignee:

Timothy Hatcher <timothy>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bdakin, commit-queue, dino, ews-watchlist, glen, rniwa, simon.fraser, thorton, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Mac

OS:

macOS 10.14

Bug Depends on:

Bug Blocks:

190526

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	none
Patch	none
Archive of layout-test-results from ews107 for mac-sierra-wk2	none
Patch	none

Timothy Hatcher

Reported 2018-10-11 17:04:49 PDT

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

Attachments
Patch (63.50 KB, patch) 2018-10-11 17:31 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (102.46 KB, patch) 2018-10-12 13:03 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (75.12 KB, patch) 2018-10-15 12:15 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Patch (74.79 KB, patch) 2018-10-15 12:35 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-sierra-wk2 (2.82 MB, application/zip) 2018-10-15 14:04 PDT, EWS Watchlist	no flags	Details
Patch (76.13 KB, patch) 2018-10-15 14:20 PDT, Timothy Hatcher	no flags	Details Formatted Diff Diff
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Radar WebKit Bug Importer

Comment 1 2018-10-11 17:13:12 PDT

<rdar://problem/45212025>

Timothy Hatcher

Comment 2 2018-10-11 17:31:06 PDT Comment hidden (obsolete)

Created attachment 352111 [details] Patch

Timothy Hatcher

Comment 3 2018-10-12 13:03:45 PDT Comment hidden (obsolete)

Created attachment 352192 [details] Patch

Dean Jackson

Comment 4 2018-10-12 16:19:29 PDT Comment hidden (obsolete)

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 5 2018-10-12 16:29:21 PDT Comment hidden (obsolete)

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 6 2018-10-12 16:31:47 PDT Comment hidden (obsolete)

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

Comment 7 2018-10-15 12:15:10 PDT Comment hidden (obsolete)

Created attachment 352356 [details] Patch

Timothy Hatcher

Comment 8 2018-10-15 12:35:03 PDT Comment hidden (obsolete)

Created attachment 352360 [details] Patch

EWS Watchlist

Comment 9 2018-10-15 14:04:04 PDT Comment hidden (obsolete)

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

Comment 10 2018-10-15 14:04:06 PDT Comment hidden (obsolete)

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

Comment 11 2018-10-15 14:20:37 PDT

Created attachment 352378 [details] Patch

WebKit Commit Bot

Comment 12 2018-10-15 16:43:11 PDT

Comment on attachment 352378 [details] Patch Clearing flags on attachment: 352378 Committed r237156: <https://trac.webkit.org/changeset/237156>

WebKit Commit Bot

Comment 13 2018-10-15 16:43:13 PDT

All reviewed patches have been landed. Closing bug.

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