Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-color-adjust] Remove an invalid test. #31407

Merged

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Oct 27, 2021

This test was added in:

https://chromium-review.googlesource.com/c/chromium/src/+/2230909

However I don't understand why it should pass. There's nothing that
guarantees that (prefers-color-scheme: dark) matches on the SVG, and
the test is consequently failing in all browsers:

https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I guess it could be nice if color-scheme affected
prefers-color-scheme for svg-as-image, maybe, but afaict that was not
what the test was about, nor something that is specified.

This test was added in:

  https://chromium-review.googlesource.com/c/chromium/src/+/2230909

However I don't understand why it should pass. There's nothing that
guarantees that `(prefers-color-scheme: dark)` matches on the SVG, and
the test is consequently failing in all browsers:

  https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I guess it could be nice if `color-scheme` affected
`prefers-color-scheme` for svg-as-image, maybe, but afaict that was not
what the test was about, nor something that is specified.
@wpt-pr-bot
Copy link
Collaborator

There are no reviewers for this pull request. Please reach out on W3C's irc server (irc.w3.org, port 6665) on channel #testing (web client) to get help with this. Thank you!

Copy link
Member

@lilles lilles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem is that the reference should have also relied on prefers-color-scheme.

@emilio emilio merged commit e7cc1ae into web-platform-tests:master Oct 28, 2021
@emilio emilio deleted the remove-invalid-color-scheme-test branch October 28, 2021 10:25
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Oct 30, 2021
…re. r=dholbert

For that, add `.dark` version of the browser.display* prefs that control
the light version of these colors.

The default for background/foreground colors are taken from the
GenericDarkColors used in LookAndFeel.

The defaults for links are based on this discussion:

  whatwg/html#5426 (comment)

(So they effectively match Chrome).

Whether the dark colors should be exposed in about:preferences (like the
light colors are) is TBD.

With this patch, we pass all the tests in:

  /html/semantics/document-metadata/the-meta-element/color-scheme/

Use the colors to paint the default canvas background and the default
colors.

There are three "regressions", though they are really progressions: we
now render the reference as the test expects (before we rendered a light
canvas background even for the reference).

Apart of these iframe tests (which we should look into, I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1738380), there are three
remaining test failures.

Two of them are due to `color: initial` not changing based on the
color-scheme. Safari also fails these tests, and the thing they're
really testing is whether system colors are preserved at computed-value
time:

  w3c/csswg-drafts#3847

Regarding that change, I'm not so sure the trade-offs there are worth
it, as that not only complicates interpolation (we wouldn't be able to
use system colors in color-mix among others, see
w3c/csswg-drafts#5780) plus it changes
inheritance behavior in sorta unexpected ways, see:

  w3c/csswg-drafts#6773

Which I just filed because apparently no browser implements this
correctly. So for now will punt on those (keep matching Safari).

There's an svg-as-image test:

  https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html

Which isn't using the feature at all and I'm not sure why is it supposed
to pass (why prefers-color-scheme: dark is supposed to match that SVG
image). This test fails in all browsers apparently:

  https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I sent web-platform-tests/wpt#31407 to remove
it and hopefully get it reviewed by some Chromium folks.

Differential Revision: https://phabricator.services.mozilla.com/D129746
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 1, 2021
…re. r=dholbert

For that, add `.dark` version of the browser.display* prefs that control
the light version of these colors.

The default for background/foreground colors are taken from the
GenericDarkColors used in LookAndFeel.

The defaults for links are based on this discussion:

  whatwg/html#5426 (comment)

(So they effectively match Chrome).

Whether the dark colors should be exposed in about:preferences (like the
light colors are) is TBD.

With this patch, we pass all the tests in:

  /html/semantics/document-metadata/the-meta-element/color-scheme/

Use the colors to paint the default canvas background and the default
colors.

There are three "regressions", though they are really progressions: we
now render the reference as the test expects (before we rendered a light
canvas background even for the reference).

Apart of these iframe tests (which we should look into, I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1738380), there are three
remaining test failures.

Two of them are due to `color: initial` not changing based on the
color-scheme. Safari also fails these tests, and the thing they're
really testing is whether system colors are preserved at computed-value
time:

  w3c/csswg-drafts#3847

Regarding that change, I'm not so sure the trade-offs there are worth
it, as that not only complicates interpolation (we wouldn't be able to
use system colors in color-mix among others, see
w3c/csswg-drafts#5780) plus it changes
inheritance behavior in sorta unexpected ways, see:

  w3c/csswg-drafts#6773

Which I just filed because apparently no browser implements this
correctly. So for now will punt on those (keep matching Safari).

There's an svg-as-image test:

  https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html

Which isn't using the feature at all and I'm not sure why is it supposed
to pass (why prefers-color-scheme: dark is supposed to match that SVG
image). This test fails in all browsers apparently:

  https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I sent web-platform-tests/wpt#31407 to remove
it and hopefully get it reviewed by some Chromium folks.

Differential Revision: https://phabricator.services.mozilla.com/D129746
chromium-wpt-export-bot pushed a commit that referenced this pull request Nov 1, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
chromium-wpt-export-bot pushed a commit that referenced this pull request Nov 2, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
chromium-wpt-export-bot pushed a commit that referenced this pull request Nov 2, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}
chromium-wpt-export-bot pushed a commit that referenced this pull request Nov 2, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}
pull bot pushed a commit to FairyWorld/tool_chromium that referenced this pull request Nov 2, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
web-platform-tests/wpt#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 10, 2021
…a=testonly

Automatic update from web-platform-tests
Re-add SVG Image color-scheme wpt test

external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
web-platform-tests/wpt#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}

--

wpt-commits: e9ff7dcef68df0b75c49043aae7e6f119c261c6b
wpt-pr: 31467
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Nov 10, 2021
…a=testonly

Automatic update from web-platform-tests
Re-add SVG Image color-scheme wpt test

external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
web-platform-tests/wpt#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}

--

wpt-commits: e9ff7dcef68df0b75c49043aae7e6f119c261c6b
wpt-pr: 31467
Gabisampaio pushed a commit to Gabisampaio/wpt that referenced this pull request Nov 18, 2021
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
web-platform-tests#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this pull request Oct 14, 2022
external/wpt/css/css-color-adjust/rendering/dark-color-scheme/
svg-as-image.html was removed by
web-platform-tests/wpt#31407.

Re-add the test so that the preferred color scheme is also taken into
account in the reference file.

Change-Id: I0feacf73ed423c0b6ba495ea2351345b5f9590ab
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3255858
Commit-Queue: Alison Maher <almaher@microsoft.com>
Reviewed-by: Rune Lillesveen <futhark@chromium.org>
Cr-Commit-Position: refs/heads/main@{#937385}
NOKEYCHECK=True
GitOrigin-RevId: cdca94c177661054fe5341f721589e1d310b7cd7
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request May 30, 2023
For that, add `.dark` version of the browser.display* prefs that control
the light version of these colors.

The default for background/foreground colors are taken from the
GenericDarkColors used in LookAndFeel.

The defaults for links are based on this discussion:

  whatwg/html#5426 (comment)

(So they effectively match Chrome).

Whether the dark colors should be exposed in about:preferences (like the
light colors are) is TBD.

With this patch, we pass all the tests in:

  /html/semantics/document-metadata/the-meta-element/color-scheme/

Use the colors to paint the default canvas background and the default
colors.

There are three "regressions", though they are really progressions: we
now render the reference as the test expects (before we rendered a light
canvas background even for the reference).

Apart of these iframe tests (which we should look into, I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1738380), there are three
remaining test failures.

Two of them are due to `color: initial` not changing based on the
color-scheme. Safari also fails these tests, and the thing they're
really testing is whether system colors are preserved at computed-value
time:

  w3c/csswg-drafts#3847

Regarding that change, I'm not so sure the trade-offs there are worth
it, as that not only complicates interpolation (we wouldn't be able to
use system colors in color-mix among others, see
w3c/csswg-drafts#5780) plus it changes
inheritance behavior in sorta unexpected ways, see:

  w3c/csswg-drafts#6773

Which I just filed because apparently no browser implements this
correctly. So for now will punt on those (keep matching Safari).

There's an svg-as-image test:

  https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html

Which isn't using the feature at all and I'm not sure why is it supposed
to pass (why prefers-color-scheme: dark is supposed to match that SVG
image). This test fails in all browsers apparently:

  https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I sent web-platform-tests/wpt#31407 to remove
it and hopefully get it reviewed by some Chromium folks.

Differential Revision: https://phabricator.services.mozilla.com/D129746
Loirooriol pushed a commit to Loirooriol/servo that referenced this pull request May 30, 2023
For that, add `.dark` version of the browser.display* prefs that control
the light version of these colors.

The default for background/foreground colors are taken from the
GenericDarkColors used in LookAndFeel.

The defaults for links are based on this discussion:

  whatwg/html#5426 (comment)

(So they effectively match Chrome).

Whether the dark colors should be exposed in about:preferences (like the
light colors are) is TBD.

With this patch, we pass all the tests in:

  /html/semantics/document-metadata/the-meta-element/color-scheme/

Use the colors to paint the default canvas background and the default
colors.

There are three "regressions", though they are really progressions: we
now render the reference as the test expects (before we rendered a light
canvas background even for the reference).

Apart of these iframe tests (which we should look into, I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1738380), there are three
remaining test failures.

Two of them are due to `color: initial` not changing based on the
color-scheme. Safari also fails these tests, and the thing they're
really testing is whether system colors are preserved at computed-value
time:

  w3c/csswg-drafts#3847

Regarding that change, I'm not so sure the trade-offs there are worth
it, as that not only complicates interpolation (we wouldn't be able to
use system colors in color-mix among others, see
w3c/csswg-drafts#5780) plus it changes
inheritance behavior in sorta unexpected ways, see:

  w3c/csswg-drafts#6773

Which I just filed because apparently no browser implements this
correctly. So for now will punt on those (keep matching Safari).

There's an svg-as-image test:

  https://searchfox.org/mozilla-central/rev/f8576fec48d866c5f988baaf1fa8d2f8cce2a82f/testing/web-platform/tests/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html

Which isn't using the feature at all and I'm not sure why is it supposed
to pass (why prefers-color-scheme: dark is supposed to match that SVG
image). This test fails in all browsers apparently:

  https://wpt.fyi/results/css/css-color-adjust/rendering/dark-color-scheme/svg-as-image.html?label=master&label=experimental&aligned

I sent web-platform-tests/wpt#31407 to remove
it and hopefully get it reviewed by some Chromium folks.

Differential Revision: https://phabricator.services.mozilla.com/D129746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants