Summary: | <picture> container doesn't update when prefers-color-scheme media query changes | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Craig Hockenberry <craig.hockenberry> | ||||||||
Component: | CSS | Assignee: | 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
Craig Hockenberry
2018-10-25 12:51:15 PDT
I have a fix. Needs a test now. Created attachment 353741 [details]
Patch
Attachment 353741 [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 17 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 353741 [details] Patch Attachment 353741 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/9838457 New failing tests: css-dark-mode/prefers-color-scheme-picture-element.html Created attachment 353755 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Created attachment 353756 [details]
Patch
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 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 on attachment 353756 [details] Patch Clearing flags on attachment: 353756 Committed r237878: <https://trac.webkit.org/changeset/237878> All reviewed patches have been landed. Closing bug. (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. (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. 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 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? |