Bug 190913 - <picture> container doesn't update when prefers-color-scheme media query changes
Summary: <picture> container doesn't update when prefers-color-scheme media query changes
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: Macintosh macOS 10.14
: P2 Normal
Assignee: Timothy Hatcher
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2018-10-25 12:51 PDT by Craig Hockenberry
Modified: 2018-11-06 14:07 PST (History)
11 users (show)

See Also:


Attachments
Patch (21.74 KB, patch)
2018-11-02 15:51 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-sierra-wk2 (3.30 MB, application/zip)
2018-11-02 18:04 PDT, Build Bot
no flags Details
Patch (23.62 KB, patch)
2018-11-02 18:45 PDT, Timothy Hatcher
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Craig Hockenberry 2018-10-25 12:51:15 PDT
A <picture> container that selects sources based on a "prefers-color-scheme" media query doesn't update when the user switches between Light and Dark Mode on macOS Mojave.

For example, the following works correctly when the page loads:

<picture>
	<source srcset="images/dark.jpg" media="(prefers-color-scheme: dark)">
	<source srcset="images/light.jpg" media="(prefers-color-scheme: light)">
	<img src="images/not_available.jpg" alt="prefers-color-scheme is not available in this browser">
</picture>

But if the visitor switches their System Preference, the image doesn't change.

A working example can be found here: https://files.iconfactory.net/downloads/miscellaneous/DarkModeCSS/
Comment 1 Radar WebKit Bug Importer 2018-10-26 22:29:29 PDT
<rdar://problem/45608456>
Comment 2 Timothy Hatcher 2018-11-01 13:21:22 PDT
I have a fix. Needs a test now.
Comment 3 Timothy Hatcher 2018-11-02 15:51:16 PDT
Created attachment 353741 [details]
Patch
Comment 4 Build Bot 2018-11-02 15:54:06 PDT Comment hidden (obsolete)
Comment 5 Build Bot 2018-11-02 18:04:29 PDT Comment hidden (obsolete)
Comment 6 Build Bot 2018-11-02 18:04:31 PDT Comment hidden (obsolete)
Comment 7 Timothy Hatcher 2018-11-02 18:45:43 PDT
Created attachment 353756 [details]
Patch
Comment 8 Build Bot 2018-11-02 18:49:36 PDT
Attachment 353756 [details] did not pass style-queue:


ERROR: Source/WebCore/css/MediaQueryEvaluator.cpp:107:  Line contains only semicolon. If this should be an empty statement, use { } instead.  [whitespace/semicolon] [5]
Total errors found: 1 in 19 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Dean Jackson 2018-11-06 11:28:02 PST
Comment on attachment 353756 [details]
Patch

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

> LayoutTests/css-dark-mode/prefers-color-scheme-picture-element.html:26
> +        internals.settings.setUseDarkAppearance(true);

We default to using the light scheme no matter what the system says?

> LayoutTests/platform/mac-highsierra/css-dark-mode/prefers-color-scheme-picture-element-expected.txt:5
> +FAIL Picture image has the light source selected assert_regexp_match: expected object "/\/light.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"
> +PASS Dark color scheme enabled 
> +FAIL Picture image has the dark source selected assert_regexp_match: expected object "/\/dark.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"

Why don't we skip all of css-dark-mode on highsierra and sierra?
Comment 10 WebKit Commit Bot 2018-11-06 11:53:21 PST
Comment on attachment 353756 [details]
Patch

Clearing flags on attachment: 353756

Committed r237878: <https://trac.webkit.org/changeset/237878>
Comment 11 WebKit Commit Bot 2018-11-06 11:53:23 PST
All reviewed patches have been landed.  Closing bug.
Comment 12 Timothy Hatcher 2018-11-06 12:14:45 PST
(In reply to Dean Jackson from comment #9)
> Comment on attachment 353756 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=353756&action=review
> 
> > LayoutTests/css-dark-mode/prefers-color-scheme-picture-element.html:26
> > +        internals.settings.setUseDarkAppearance(true);
> 
> We default to using the light scheme no matter what the system says?

Yes.

> > LayoutTests/platform/mac-highsierra/css-dark-mode/prefers-color-scheme-picture-element-expected.txt:5
> > +FAIL Picture image has the light source selected assert_regexp_match: expected object "/\/light.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"
> > +PASS Dark color scheme enabled 
> > +FAIL Picture image has the dark source selected assert_regexp_match: expected object "/\/dark.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"
> 
> Why don't we skip all of css-dark-mode on highsierra and sierra?

We currently don't have a TestExpectations file for those platforms. I guess if I add one, it would use it? It would make sense to do that.
Comment 13 Ryan Haddad 2018-11-06 13:55:25 PST
(In reply to Timothy Hatcher from comment #12)
> (In reply to Dean Jackson from comment #9)
> > Why don't we skip all of css-dark-mode on highsierra and sierra?
> 
> We currently don't have a TestExpectations file for those platforms. I guess
> if I add one, it would use it? It would make sense to do that.

I would suggest skipping the css-dark-mode directory in LayoutTests/TestExpectations, then explicitly marking it as Pass for Mojave+ in LayoutTests/platform/mac-wk2/TestExpectations.
Comment 14 Ryan Haddad 2018-11-06 14:07:48 PST
After this change, css-dark-mode/prefers-color-scheme-picture-element.html is failing on High Sierra and Sierra:

--- /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/css-dark-mode/prefers-color-scheme-picture-element-expected.txt
+++ /Volumes/Data/slave/highsierra-release-tests-wk2/build/layout-test-results/css-dark-mode/prefers-color-scheme-picture-element-actual.txt
@@ -1,6 +1,6 @@
  
 
-FAIL Picture image has the light source selected assert_regexp_match: expected object "/\/light.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"
+FAIL Picture image has the light source selected assert_regexp_match: expected object "/\/light.png$/" but got "file:///Volumes/Data/slave/highsierra-release-tests-wk2/build/LayoutTests/css-dark-mode/error.png"
 PASS Dark color scheme enabled 
-FAIL Picture image has the dark source selected assert_regexp_match: expected object "/\/dark.png$/" but got "file:///Volumes/Data/EWS/WebKit/LayoutTests/css-dark-mode/error.png"
+FAIL Picture image has the dark source selected assert_regexp_match: expected object "/\/dark.png$/" but got "file:///Volumes/Data/slave/highsierra-release-tests-wk2/build/LayoutTests/css-dark-mode/error.png"

https://build.webkit.org/results/Apple%20High%20Sierra%20Release%20WK2%20(Tests)/r237879%20(7670)/results.html

..and css-dark-mode/supported-color-schemes.html is failing on Mojave:
--- /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/css-dark-mode/supported-color-schemes-expected.txt
+++ /Volumes/Data/slave/mojave-release-tests-wk2/build/layout-test-results/css-dark-mode/supported-color-schemes-actual.txt
@@ -6,7 +6,7 @@
 PASS Supported color schemes changed to light and dark 
 PASS Element colors are correct in light color scheme with light and dark supported color scheme 
 PASS Dark color scheme enabled 
-PASS Element colors are correct in dark color scheme with light and dark supported color scheme 
+FAIL Element colors are correct in dark color scheme with light and dark supported color scheme assert_equals: expected "rgb(255, 255, 255)" but got "rgb(0, 0, 0)"
 PASS Light color scheme enabled 
 PASS Supported color schemes changed to dark 
 PASS Element colors are correct in light color scheme with only dark supported color scheme 

https://build.webkit.org/results/Apple%20Mojave%20Release%20WK2%20(Tests)/r237879%20(652)/results.html