Bug 190913

Summary: <picture> container doesn't update when prefers-color-scheme media query changes
Product: WebKit Reporter: Craig Hockenberry <craig.hockenberry>
Component: CSSAssignee: Timothy Hatcher <timothy>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, ews-watchlist, megan_gardner, rniwa, ryanhaddad, sabouhallawa, simon.fraser, thorton, timothy, tristan.waddington, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari Technology Preview   
Hardware: Mac   
OS: macOS 10.14   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews104 for mac-sierra-wk2
none
Patch none

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 EWS Watchlist 2018-11-02 15:54:06 PDT Comment hidden (obsolete)
Comment 5 EWS Watchlist 2018-11-02 18:04:29 PDT Comment hidden (obsolete)
Comment 6 EWS Watchlist 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 EWS Watchlist 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
Comment 15 Tristan Waddington 2021-04-09 11:43:03 PDT
I'm still seeing this issue in Safari Version 14.0.3 (15610.4.3.1.7, 15610).

Tested by opening a webpage with the picture element and switching the theme two different ways:

1. Via System Preferences.
2. Via the "Force X Appearance" icon in Safari Developer Tools.

Can you confirm that this hasn't regressed and is still fixed in newer versions of Safari?