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] [css-color-adjust] Make system colors fully resolve (but flag they were system colors) thus reversing the resolution of #3847 #6773

Closed
emilio opened this issue Oct 27, 2021 · 54 comments

Comments

@emilio
Copy link
Collaborator

emilio commented Oct 27, 2021

In #3847 it was resolved that system colors would compute to themselves. The argument for that seems sensible (which is that color-scheme would be automatically honored for them while inheriting).

However I'm not so sure that's great behavior (plus there are still open issues from that change like #5780).

In particular, in order to guarantee contrast, you need to use system color pairs (the foreground and the background), such as:

div {
  background-color: Canvas;
  color: CanvasText;
}

If color-scheme changes, but the author doesn't specify a background, making system colors compute to themselves at computed-value time breaks contrast (rendered):

<!doctype html>
<style>
  :root { color-scheme: dark }
  span { color-scheme: light }
</style>
I'm dark, and <span>I'm light</span>

The span should be dark text over dark background per spec, since it inherits the initial color which is canvastext, which is undesirable.

That's clearly not how browsers are working today, which confuses me because I thought Chrome implemented this change.

I'd expect this test-case to render per spec the same as the following (rendered):

<!doctype html>
<style>
  :root { color-scheme: dark }
  span { color-scheme: light; color: CanvasText }
</style>
I'm dark, and <span>I'm light</span>

(which is clearly undesirable, and not what's going on).

Can you explain what's going on here @Futhark / @andruud / @kbabbitt / @tabatkins?

cc @smfr

@emilio
Copy link
Collaborator Author

emilio commented Oct 27, 2021

err, s/at-futhark/@lilles :)

@lilles
Copy link
Member

lilles commented Oct 28, 2021

I'll defer to @tabatkins as he raised the original issue.

Adding @alisonmaher as I think this is even more interesting for forced-color-adjust. preserve-parent-color seems relevant here?

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue 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 issue 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
@alisonmaher
Copy link
Collaborator

@emilio said:

Can you explain what's going on here?

I don't believe that Chromium has shipped the system color computing to itself behavior, which is likely why you're seeing the behavior you are.

There is a part of the Forced Colors Mode spec that I think relies on the decision in #3847:

if its computed value is a color other than a system color, its used value is instead forced to a system color

In other words, we force the colors at used value time, but do not force the color if it is a system color already. If system colors no longer compute to themselves, this won't be as clean to do (although in Chromium, we currently have a workaround for this, so we can continue to work around this if the decision is overturned).

I think this is even more interesting for forced-color-adjust. preserve-parent-color seems relevant here?

Because preserve-parent-color forces the computed color to the parent's used color value, in general, it shouldn't matter if the system color computes to itself since we are only looking at the used value. But perhaps you are specifically referring to the interaction between system colors, color-scheme and forced-color-adjust, and that does seem pretty interesting.

In Chromium, we are currently ignoring color-scheme when resolving system colors in Forced Colors Mode, even if forced-color-adjust is none/preserve-parent-color. In other words, we always resolve the system colors to the OS defined system colors in Forced Colors Mode, no matter what color-scheme is set to (which may not be the expected behavior).

However, if we do take color-scheme into account, we could run into some pretty interesting use cases. For example:

<!doctype html>
<style>
  :root { color-scheme: dark; }
  span { forced-color-adjust: preserve-parent-color; color-scheme: light; }
</style>
I'm CanvasText in Forced Colors Mode <span>I should be light?</span>

In this case, the root's colors-scheme will be overridden to light dark. The span would set its color value to the used value of its parent (i.e. the resolved value of CanvasText), which may not match the expected light color-scheme of the span. We could run into similarly interesting cases with forced-color-adjust: none.

@emilio
Copy link
Collaborator Author

emilio commented Nov 1, 2021

@emilio said:

Can you explain what's going on here?

I don't believe that Chromium has shipped the system color computing to itself behavior, which is likely why you're seeing the behavior you are.

I'm testing on Chrome Dev with experimental features enabled (and I confirmed that https://wpt.live/css/css-color/system-color-compute.html passes on this browser, yet my test-case renders differently from what I'd expect). So it seems there's something else going on.

@alisonmaher
Copy link
Collaborator

@emilio said:

Can you explain what's going on here?

I don't believe that Chromium has shipped the system color computing to itself behavior, which is likely why you're seeing the behavior you are.

I'm testing on Chrome Dev with experimental features enabled (and I confirmed that https://wpt.live/css/css-color/system-color-compute.html passes on this browser, yet my test-case renders differently from what I'd expect). So it seems there's something else going on.

Ah I see, so it only works when you explicitly set color to CanvasText, but not in the default case. In Chromium, the default for color is still black and hasn't been updated to CanvasText, so that would explain why your test-case is behaving as it is.

@emilio
Copy link
Collaborator Author

emilio commented Nov 1, 2021

Ugh, alright. So :root { color-scheme: dark } does change the computed text style to light but it behaves differently from :root { color: CanvasText }? That seems a bit broken, how do you implement the initial color switching for the root?

Anyways that explains what's going on to some extent, I guess.

@alisonmaher
Copy link
Collaborator

I'm guessing this change has the details for how the initial color switching happens at the root https://chromium-review.googlesource.com/c/chromium/src/+/2224222, although @lilles or @andruud may be able to provide more details on that.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-color] [css-color-adjust] Consider reversing the resolution of #3847.

The full IRC log of that discussion <fantasai> Topic: [css-color] [css-color-adjust] Consider reversing the resolution of #3847
<fantasai> github: https://github.com//issues/6773
<fantasai> emilio: We made system colors compute to themselves because wanted them to react to color-scheme
<fantasai> emilio: but that's not behavior any agent has, and when you do that you get lacking contrast if color scheme changes but you didn't change the bgcolor
<fantasai> emilio: not quite clear to me what Blink is doing
<fantasai> emilio: but, computing system colors to the keywords themselves also causes complexity
<fantasai> emilio: with interpolation, issues open since we resolved that
<Rossen_> q?
<TabAtkins> q+
<fantasai> emilio: So can we undo that resolution?
<fantasai> TabAtkins: A couple of points emilio made are present today regardless
<Rossen_> ack TabAtkins
<fantasai> TabAtkins: e.g. complexity of interpolating color, currentColor resolves at used-value time
<fantasai> TabAtkins: exact same case for system colors
<fantasai> TabAtkins: also has a point about needing to set system colors in pairs, and yes, you do, and spec says you should always do that
<fantasai> TabAtkins: it's very easy to get messed up and have bad colors if you don't, in general
<fantasai> TabAtkins: if we compute system colors and computed value time, then whenever color-scheme changes, in particular if you go across any embedding boundaries, shadow boundaries, they'll be messed up as well
<fantasai> TabAtkins: they'll assume in light mode and responsibly use system colors, get wrong colors inheriting in
<fantasai> TabAtkins: so I think the current specced behavior is the best
<fantasai> TabAtkins: chrome's behavior is weird and inconsistent right now, but once matches spec, I think it will be reasonable behavior
<fantasai> TabAtkins: and won't save any complexity but changing it
<fantasai> s/but/by/
<fantasai> emilio: Not about setting colors in pairs, but about changing color scheme
<fantasai> emilio: if ...
<fantasai> emilio: if you make system colors compute to themselves, forced-color-adjust: preserve-parent-color is like forcing to resolve their values, which is pretty odd
<fantasai> emilio: we're landing workarounds...
<fantasai> emilio: I disagree that this doesn't introduce complexity
<alisonmaher> q+
<fantasai> emilio: preserve-parent-color is literally working around this behavior
<fantasai> TabAtkins: ppc is about forced-color-adjust, where we want SVG to be able to opt out of being forced-colored, but still be able to respond to system colors coming in
<fantasai> TabAtkins: We have plenty of SVG in the wild that want to respond to outside color and set some of their own colors as well
<fantasai> TabAtkins: to work in forced color mode, need some way to tell whether receiving system color vs other color
<fantasai> emilio: not true, FF behaves correctly right now [...]
<fantasai> alisonmaher: ppc was a workaround due to handling forced-colors mode, not about system colors computing to themselves
<Rossen_> ack alisonmaher
<fantasai> emilio: to me, both are related. If don't preserve ??? then can't at used value time
<fantasai> alisonmaher: in Chrome we do implement this workaround. We also haven't shipped system colors computing to themselves yet
<fantasai> alisonmaher: Essentially piggybacking on top of, have an implementation where it computes to itself, so workaround with impl that hasn't shipped yet
<fantasai> Rossen_: so building on the capability but not exposed?
<fantasai> alisonmaher: yeah
<Rossen_> q?
<fantasai> emilio: We define system colors to compute to themselves
<fantasai> emilio: you need to implement forced-colors at used value time and vice versa
<fantasai> emilio: complexity comes from making both forced colors and system colors compute at used value time
<drott> fantasai: if we don't make the system colors compute to themselves
<drott> fantasai: if you switch scheme, it won't have any effect - which is not expected
<fantasai> emilio: Let's say have a dark background and background is Canvas
<fantasai> emilio: if you just change color-scheme you get illegible text
<fantasai> dbaron: if they don't compute to themselves, you run a restyle
<drott> fantasai: if you have an element that is color-scheme dark, inside one with color-scheme light
<drott> fantasai: are you going to have light or dark? or what's happening inside?
<drott> fantasai: if you move it out, you'd expect it to stay on that
<fantasai> fantasai: but it inherits different resolve colors depending on parent color scheme
<fantasai> fantasai: to get colors you expect, you can't rely on system colors inheriting, have to re-declare them when declaring color-scheme
<Rossen_> q?
<TabAtkins> Form what I can tell, emilio, you're saying that if authors don't set colors in pairs, they can get bad results. Is that right?
<fantasai> emilio: need to restyle anyway, need to set at least the backgrounds, so can't rely on inheritance otherwise get broken behavior
<fantasai> emilio: always need to set in pairs, but even if you do, but if you change color-scheme without setting any color then behavior is broken if they compute to themselves
<fantasai> emilio: because changing only foreground color and bg color doesn't change because not inherited
<fantasai> Rossen_: Given the complexity of these different combinations and where Chromium implementation is, in its inconsistent state, I propose we continue to work on this issue in GH
<chris_> "Authors may also use these keywords at any time, but should be careful to use the colors in matching background-foreground pairs to ensure appropriate contrast, as any particular contrast relationship across non-matching pairs (e.g. Canvas and ButtonText) is not guaranteed."
<fantasai> Rossen_: and once more implementers as well as others have a stable conclusion, can bring it back for resolution
<chris_> https://drafts.csswg.org/css-color-4/#css-system-colors
<fantasai> Rossen_: going in circles here trying to define expected behaviors
<fantasai> Rossen_: as well as what everyone is talking about
<fantasai> emilio: Yeah, agree, I think we're talking past each other a bit
<fantasai> Rossen_: ok, well thanks for opening issue
<chris_> Also "The following system color pairings are expected to form legible background-foreground colors:"
<fantasai> Rossen_: let's move on to next issue

@astearns astearns removed the Agenda+ label Nov 10, 2021
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Nov 20, 2021
… r=dholbert

There's two kinds of test failures remaining:

 * Failures related to how system colors inherit, which is something
   that no engine is shipping and I'm not convince it's a good thing:
   w3c/csswg-drafts#6773

 * Failures related to <iframes> observing color-scheme of ancestors
   (which Chrome does implement but Safari doesn't):
   w3c/csswg-drafts#4772
   https://bugzilla.mozilla.org/show_bug.cgi?id=1738380
   I'm unconvinced that should work across cross-origin iframes though
   (see the question at the end of that issue), and it seems delaying
   shipping it blocked on figuring that out while other engines ship
   diverging behaviors is probably not worth it.

Given privileged code is already using this and we know of no other
blockers, I think it's fine to let it ride the trains.

Differential Revision: https://phabricator.services.mozilla.com/D131635
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Nov 22, 2021
… r=dholbert

There's two kinds of test failures remaining:

 * Failures related to how system colors inherit, which is something
   that no engine is shipping and I'm not convince it's a good thing:
   w3c/csswg-drafts#6773

 * Failures related to <iframes> observing color-scheme of ancestors
   (which Chrome does implement but Safari doesn't):
   w3c/csswg-drafts#4772
   https://bugzilla.mozilla.org/show_bug.cgi?id=1738380
   I'm unconvinced that should work across cross-origin iframes though
   (see the question at the end of that issue), and it seems delaying
   shipping it blocked on figuring that out while other engines ship
   diverging behaviors is probably not worth it.

Given privileged code is already using this and we know of no other
blockers, I think it's fine to let it ride the trains.

Differential Revision: https://phabricator.services.mozilla.com/D131635
@svgeesus
Copy link
Contributor

Also in #3847 @upsuper said:

What I meant to say in #3847 (comment) is that Firefox's approach is not scalable to more keywords. It was done under the assumption that only currentcolor would behave that way. It's not touching serialization / Typed OM, but about the fact that animating with multiple parts which can change during animation could be challenging.

But I might be wrong because most complexity of animating currentcolor comes from the fact that color itself can animate as well, so there might be some smart way to handle all the system color keywords which can only discretely switch (AFAIU) without adding too much complexity to implementation, but I'm not sure.

Serialization / Typed OM is indeed something we need to consider, but I don't think that has or would introduce inherent complexity as far as we allow only linear combination of colors and keywords. But animation may have such complexity by itself.

@svgeesus
Copy link
Contributor

As a reminder of the problem identified last time this was discussed:

emilio: always need to set in pairs, but even if you do, but if you change color-scheme without setting any color then behavior is broken if they compute to themselves
emilio: because changing only foreground color and bg color doesn't change because not inherited

@svgeesus
Copy link
Contributor

Also a reminder that this doesn't just affect currentColor but also the (undeprecated) system colors, which leads to the css-color-5 issue on color-mix() where the result is, in the worst case,

  • the percentage of all actual resolved colors
  • the percentage of currentColor
  • the percentage of Canvas
  • (and so on for the other 16 system colors)

@tabatkins
Copy link
Member

The "worst case" isn't unusual; you get the exact same effect for specified lengths, where you can only merge the absolute units.

The inheritance issue is indeed problematic tho, hm.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-color] [css-color-adjust] Consider reversing the resolution of #3847.

The full IRC log of that discussion <emeyer> Topic: [css-color] [css-color-adjust] Consider reversing the resolution of #3847
<lea> I don't need to be here for that, dropping off
<emeyer> Github: https://github.com//issues/6773
<emeyer> emilio: I think this is the right thing to do, otherwise interpolation becomes very messy.
<emeyer> TabAtkins: After review, I agree with Emilio and it would be best to reverse the decision.
<Rossen_> q
<emeyer> …We should resolve that system colors should compute at computed style time.
<alisonmaher> q+
<emeyer> chris: So this is reversing for system colors but not currentColor.
<emeyer> alisonmaher: What will this mean for use value time?
<Rossen_> ack alisonmaher
<emeyer> emilio: The only reason I started looking into this is because of the preserve keyword. If you force colors at use value time, it’s bad.
<emeyer> Rossen: We have a resolution for forced colors that they resolve in used value time. That meant we could avoid the color mix function.
<emeyer> emilio: Where was it required? Just for backgrounds and alpha?
<emeyer> …Otherwise, system colors are preserved. Backgrounds are a special case.
<emeyer> …Gecko implements this with magic, not color mix. Could implement with color mix as well.
<emeyer> alisonmaher: This could re-raise an issue around forced-color-adjust.
<emeyer> emilio: I don’t see why that would be a problme.
<emeyer> s/problme/problem/
<Rossen_> ack fantasai
<emeyer> fantasai: I think one problem would be that with the default color on the canvas, if you change the color scheme at any point lower than root, it should have no effect.
<emeyer> TabAtkins: Because background colors aren’t inherited, if you change color scheme to the opposite, if the background is transpaarent, you end up with black on black or white on white.
<emeyer> …If you explicitly set colors with the scheme, you get what you expect. But it’s not reasonable to expect that flipping scheme will make things unreadable.
<fantasai> s/it should have/it would have/
<JakeA> is everyone just silent or have I dropped out?
<emeyer> (silence)
<Rossen_> q
<Rossen_> q
<emeyer> Rossen: Anyone who is still concerned about the effect of this change or the handling of forced-color-adjust, if this is something that doesn’t sound right yet, we can take another week to consider so we don’t end up flipping back and forth.
<emeyer> dlibby: I wouldn’t mind more time to think through, given Blink has shipped this behavior for a year or so.
<emeyer> TabAtkins: We did ship, but other things we’re doing don’t match the spec, so changing this wouldn’t have an enormous effect, I believe. There are still details to work out.
<fantasai> s/work out/work out with forced-colors mode, though, so happy to delay a week as well/
<emeyer> Rossen: Let’s give it a week. We’ll prioritize this next week.
<JakeA> https://github.com//pull/6533#issuecomment-1023455165

@tabatkins
Copy link
Member

So, regarding the Forced Colors edits: currently, Forced Colors Mode will leave a color alone if it's a system color, and only actually "force" them (into a predefined system color) otherwise. If we want to preserve this ability (so someone can, for example, purposely invert colors for an element), we'll need to specify that, while system colors compute to absolutes, they retain the fact that they were system colors (in an invisible-to-authors fashion, so no-op setting a color property to itself would clear this info).

Is there more issues to think about? @alisonmaher ?

@alisonmaher
Copy link
Collaborator

@tabatkins That's correct. And since I don't think this has been mentioned in the thread directly, to summarize, the proposal is to reverse the resolution in both #3847 and #4915.

(And I'm guessing there might be resolutions related to color-scheme and the default color value that would need to be reverted, as well?)

The reasons why forcing colors at used value time was desirable is well summarized by two of @fantasai's comments: #4915 (comment) and #4915 (comment).

One point of which is how inheritance works when forced-color-adjust:none is set (i.e. we would inherit the parent's forced color styles rather than the non-forced color values). This would be a behavior change from what is currently shipped in Chromium.

As a result of this behavior change, though, we would no longer need to add preserve-parent-color to address the issue raised in #6310.

And reverting back to forcing colors at computed value time would likely re-raise the following issue: #5419 (comment).

@svgeesus
Copy link
Contributor

svgeesus commented Mar 2, 2022

while system colors compute to absolutes, they retain the fact that they were system colors (in an invisible-to-authors fashion, so no-op setting a color property to itself would clear this info).

A single bit to indicate was_system_color or an indication of which system color it was? (Some of them compute to the same color)

@svgeesus
Copy link
Contributor

fantasai: The flag part is an internal impl detail; we just need to spec that these colors, even tho they resolve, need to stay as-is in forced colors mode.
chris: suggested wording?
fantasai: I can come up with some, yeah

I will wait for the suggested wording from @fantasai before publishing

@fantasai
Copy link
Collaborator

fantasai commented Apr 20, 2022

@svgeesus How about just:

However, such colors must not be altered by forced color mode.

?

@svgeesus svgeesus removed the css-color-4 Current Work label Apr 20, 2022
emilio added a commit to emilio/web-platform-tests that referenced this issue May 23, 2022
In w3c/csswg-drafts#6773 it was decided to
change how system colors computed to not inherit as themselves.
emilio added a commit to emilio/web-platform-tests that referenced this issue May 23, 2022
In w3c/csswg-drafts#6773 it was decided to
change how system colors computed to not inherit as themselves.
@emilio
Copy link
Collaborator Author

emilio commented May 23, 2022

I sent an update to the relevant WPT here: web-platform-tests/wpt#34170

emilio added a commit to web-platform-tests/wpt that referenced this issue May 29, 2022
In w3c/csswg-drafts#6773 it was decided to
change how system colors computed to not inherit as themselves.
@svgeesus
Copy link
Contributor

Closing as WPT now in sync, thanks @emilio

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 9, 2022
…a=testonly

Automatic update from web-platform-tests
[css-color] Fix color computation test

In w3c/csswg-drafts#6773 it was decided to
change how system colors computed to not inherit as themselves.

--

wpt-commits: eb390f79f23990524760ed0ac692d295fd6403e6
wpt-pr: 34170
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 10, 2022
…a=testonly

Automatic update from web-platform-tests
[css-color] Fix color computation test

In w3c/csswg-drafts#6773 it was decided to
change how system colors computed to not inherit as themselves.

--

wpt-commits: eb390f79f23990524760ed0ac692d295fd6403e6
wpt-pr: 34170
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 30, 2022
In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
aarongable pushed a commit to chromium/chromium that referenced this issue Jun 30, 2022
In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 30, 2022
In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 30, 2022
In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 5, 2022
…stonly

Automatic update from web-platform-tests
Remove CSSSystemColorComputeToSelf

In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}

--

wpt-commits: ec7152811117695160a237f83f33ad0abc53eab8
wpt-pr: 34657
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jul 7, 2022
…stonly

Automatic update from web-platform-tests
Remove CSSSystemColorComputeToSelf

In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}

--

wpt-commits: ec7152811117695160a237f83f33ad0abc53eab8
wpt-pr: 34657
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
We had resolved to update the default forced-color-adjust value for
SVGs from none to preserve-parent-color. However, we have been blocked
on shipping this behavior as a result of
w3c/csswg-drafts#6773.

We have received signals that it would be useful to have this work
for SVGs in the meantime while we wait for a CSSWG resolution.

As a temporary fix, allow preserve-parent-color to parse if we are
in the UA stylesheet so that the default behavior of SVGs is updated
to what authors expect without having to ship the preserve-parent-color
property.

Bug: 1164162
Change-Id: I16fea791a376078b5960801eae6190c44499e565
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3580054
Reviewed-by: Daniel Libby <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Alison Maher <[email protected]>
Cr-Commit-Position: refs/heads/main@{#991577}
NOKEYCHECK=True
GitOrigin-RevId: bd29a99c80f79a969e530b581e18275229c0a53c
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
In w3c/csswg-drafts#6773, CSSWG resolved to
reverse the decision to support this feature, so remove the feature
flag and all related code and tests.

[email protected]

Bug: 1081945
Change-Id: If38641559b530b44dc0eb054f485974145bd316c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3718880
Reviewed-by: Alison Maher <[email protected]>
Reviewed-by: Anders Hartvoll Ruud <[email protected]>
Commit-Queue: Daniel Libby <[email protected]>
Reviewed-by: Xianzhu Wang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1019741}
NOKEYCHECK=True
GitOrigin-RevId: a532c0150bfb7d57f8dab95f689edfdd35c4331b
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue 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 issue 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

No branches or pull requests