search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.45k stars 657 forks source link

[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 closed 2 years ago

emilio commented 2 years ago

In https://github.com/w3c/csswg-drafts/issues/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 https://github.com/w3c/csswg-drafts/issues/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 commented 2 years ago

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

lilles commented 2 years ago

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?

alisonmaher commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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/w3c/csswg-drafts/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
svgeesus commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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,

tabatkins commented 2 years ago

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 commented 2 years ago

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/w3c/csswg-drafts/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/w3c/csswg-drafts/pull/6533#issuecomment-1023455165
tabatkins commented 2 years ago

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 commented 2 years ago

@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: https://github.com/w3c/csswg-drafts/issues/4915#issuecomment-701674628 and https://github.com/w3c/csswg-drafts/issues/4915#issuecomment-707969753.

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: https://github.com/w3c/csswg-drafts/issues/5419#issuecomment-724269096.

svgeesus commented 2 years ago

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)

tabatkins commented 2 years ago

Single bit for provenance; all we need to know is if they were originally system colors so we know not to force them.

smfr commented 2 years ago

Can we re-title this issue? The indirection makes it hard to remember what it's about, every time.

svgeesus commented 2 years ago

I see this is second on the agenda tomorrow. @emilio @alisonmaher @tabatkins @smfr are we ready to resolve on this on the call?

alisonmaher commented 2 years ago

I think I would be ok resolving to revert both https://github.com/w3c/csswg-drafts/issues/3847# and https://github.com/w3c/csswg-drafts/issues/4915# as long as the following are met:

  1. We add something to the spec similar to what @tabatkins mentioned in https://github.com/w3c/csswg-drafts/issues/6773#issuecomment-1042098304
  2. We re-open https://github.com/w3c/csswg-drafts/issues/5419#issuecomment-724269096).
  3. We also revert the resolution in https://github.com/w3c/csswg-drafts/issues/6310#issuecomment-862517612
svgeesus commented 2 years ago

Sounds good to me, let's suggest that plan on the call.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed Making system colors fully resolve.

The full IRC log of that discussion <fantasai> Topic: Making system colors fully resolve
<fantasai> github: https://github.com/w3c/csswg-drafts/issues/6773
<chris> https://github.com/w3c/csswg-drafts/issues/6773#issuecomment-1062008116
<fantasai> alisonmaher: Proposal is for both issue 3747 and 4915, which resolved to make system colors resolve to themselves and resolve at used value time
<fantasai> alisonmaher: Proposal is to resolve the colors, but flag them as being system colors, so that we don't force them when they're used in forced colors mode
<fantasai> alisonmaher: This would also allow to revert 6310, which added a special value for SVGs
<fantasai> alisonmaher: I'm not sure if there are specific color scheme resolutions that need to be checked
<fantasai> emilio: I need to check wrt retain that they were system colors bit, but the rest seems fine
<fantasai> chris: I asked whether you need to ask which system color, and they said not, it's just a single bit so that you know not to force the color
<fantasai> emilio: Ah, ok, that makes sense
<astearns> ack fantasai
<dholbert> scribenick: dholbert
<dholbert> fantasai: there was a comment a while back which is why we went in this direction...
<fantasai> https://github.com/w3c/csswg-drafts/issues/4915#issuecomment-707969753
<dholbert> fantasai: this comment... I wonder if we're handling that somehow or if we have a problem
<fantasai> alisonmaher: I think we wouldn't be handle that case, we'd be inheriting the forced colors rather than author-specified values
<dholbert> scribenick: fantasai
<fantasai> alisonmaher: we originally shipped in Edge with forcing colors at computed value time, but ended up switching to used value time
<fantasai> alisonmaher: and we only got the opposite problem in SVG, in terms of bug
<fantasai> alisonmaher: I think it could be a good change, but maybe some cases where authors don't expect the behavior
<fantasai> TabAtkins: We solve either this one or the other one where an author-specified color naturally inherits into something with a forced background
<emilio> q+
<fantasai> TabAtkins: so I think our reverting resolution solves the more important of the issues
<fantasai> chris: I agree
<astearns> ack emilio
<fantasai> emilio: I agree with what Tab just said
<fantasai> emilio: Also, at this point, what is the difference between forcing at used value time and computed value time
<fantasai> alisonmaher: Difference is if authors sets 'forced-color-adjust: none', if at used-value time
<fantasai> alisonmaher: inherits color as if never forced
<fantasai> alisonmaher: [gives example]
<fantasai> alisonmaher: then would inherit the color that the author expected, but if at computed value time it would inherit the forced color that author doesn't expect
<fantasai> emilio: I wonder which behavior is better, but understand the difference
<dholbert> fantasai: what are we currently proposing to do with regards to this case?
<dholbert> chris: what alisonmaher proposed on that last issue
<dholbert> fantasai: particular to emilio's question: are we computing and then inheriting, or vice versa?
<fantasai> alisonmaher: Currently we inherit and then force the colors, and proposal is to force at computed value time so that you would inherit the forced colors instead
<dholbert> alisonmaher: since it's at used-value time, we're inheriting and then forcing the colors [...]
<fantasai> astearns: Seems like several issues
<fantasai> astearns: 3847, proposal is to revert the resolution. What was it?
<fantasai> chris: I see a lot of ppl being in agreement and fantasai being confused
<fantasai> astearns: fantasai, you're concerned that this set of resolutions would result in page getting a mix of forced colors and author colors that will result in bad color design
<fantasai> fantasai: yes
<fantasai> astearns: Should we explore that bad color design problem before making this set of resolutions, or should we make the resolutions and see how applying the resolutions turns out
<fantasai> chris: I suggest we try out and see if there's an issue
<fantasai> chris: I think it's the right way forward
<fantasai> chris: as Tab said, it solves the more important problem
<emilio> I agree
<dholbert> fantasai: not sure, I'm not really up to date with what's going on, and there's a lot going on in this issue
<dholbert> fantasai: how about we take them as tentative resolutions for 24 hours, and TabAtkins will explain them to me :)
<dholbert> astearns: how about we postpone making the resolutions until next week
<dholbert> fantasai: sounds good, but let's outline what we're resolving
<dholbert> astearns: That's alisonmaher 's comment. she outlines a 5-step process
<dholbert> astearns: take a look at her comment this week, and then next week we can resolve on all 5 items
<dholbert> astearns: any concerns about waiting another week?
<dholbert> chris: I'd prefer to get spec text drafted, but we can wait another week. I can still hash out some proposed spec text.
<dholbert> astearns: proposed text would probably make it easier to review in the meantime
<dholbert> astearns: emilio, please look at the proposed resolutions too
<dholbert> emilio: sounds good
<dholbert> astearns: everyone else who's concerned/interested, please also look, and we'll come back to this next week and resolve
<dholbert> astearns: thanks alisonmaher for the proposal
svgeesus commented 2 years ago

So, in 4.7.5. Resolving other colors, instead of

Each <system-color> keyword computes to itself. Its used value is the corresponding color in its color space.

we would have

Each <system-color> keyword computes to the corresponding color in its color space. However, implementations which also implement [forced color mode]() must maintain an internal flag for such colors, to preserve the fact that they were specified as system colors and must not be forced.

(I would link to the specs but drafts are down again)

svgeesus commented 2 years ago

We would also need a change to 4.8.6. Serializing other colors because currently, <system-color> keywords serialize as the lowercased name. They would now serialize as rgb(nn, nn, nn) same as named colors, right?

fantasai commented 2 years ago

OK, I reread the entire conversation.

Arguments for resolving system colors at computed value time:

Arguments for resolving system colors at used value time:

What it boils down to for authors is:

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed [css-color] [css-color-adjust] Make system colors fully resolve (but flag they were system colors) thus reversing the resolution of #3847.

The full IRC log of that discussion <dbaron> Topic: [css-color] [css-color-adjust] Make system colors fully resolve (but flag they were system colors) thus reversing the resolution of #3847
<dbaron> github: https://github.com/w3c/csswg-drafts/issues/6773
<dbaron> Agenda is https://lists.w3.org/Archives/Public/www-style/2022Mar/0005.html
<dbaron> fantasai: We delayed this to give me a chance to reread the whole conversation... which I did. What I understand about this issue is that it was raised because switching color scheme shouldn't cause system colors to change when they're intherited. I think behavior of having system colors switch along with color scheme makes sense. Why would you switch color scheme if you're not changing colors?
<dbaron> fantasai: Then an argument that making ??? with 18 different combinations of colors is unwieldy. I'd take that as "too hard to implement".
<emilio> q+
<TabAtkins> q+
<dbaron> fantasai: Then there were arguments about resolving system colors at used value time: (1) makes some transitions issues go away and (2) setting forced-color-adjust:none on subtree reverts to colors that author specified. If we take this change then forced-colors would work only on non-inherited colors. That could cause contrast problems.
<dbaron> fantasai: There's arguments in both directions.
<fantasai> s/???/forced-color-adjust: none/
<Rossen_> ack emilio
<dbaron> emilio: There's another argument for ... color-scheme doesn't behave like you want it to behave. But regarding forced-color-adjust:none, that seems like a better behavior. At least, we were proposing adding a new value just to do the thing you get when you resolve system colors at computed value time. We were planning to make that default in SVG, which is the obvious place to use forced-color-adjust:none. So I'm not sure the behavior for
<dbaron> forced-color-adjust:none with used value time forcing is better.
<Rossen_> ack tantek
<Rossen_> ack TabAtkins
<fantasai> https://github.com/w3c/csswg-drafts/issues/6773#issuecomment-1069295596
<dbaron> TabAtkins: In first part of fantasai's response, fantasai argues against inheriting a color not respecting a color scheme. I think you should reconsider. I switched to emilio's viewport because if an inherited system color repsects a change in color scheme... this leads to the a11y problem of text color and background color not being set as a pair. If background is transparent... will be bad when you use your text color with other
<dbaron> background color.
<dbaron> TabAtkins: If you use the system colors explicitly, inherited text will by default wrong if it respects color scheme changes. I think that's a mistake.
<Rossen_> ack fantasai
<emilio> q+
<dbaron> fantasai: The color-scheme doesn't switch at a boundary randomly... the author has to explicitly say they're switching the color scheme. Why are they changing it if they're not making other color changes too? I'd expect author to use these things in coordinatiotn.
<tantek> +1 fantasai
<Rossen_> fantasai, what about tools? F12 is supporting forced-colors effect for debugging/testing
<dbaron> fantasai: On the flip side, forced-color-adjust: none problem: if you have a subtree where you want the colors to be the ones I specified and you set forced-color-adjust:none, and the browser only turns off forced-color-adjust for some of your colors, that's not expected behavior. You tried to turn it off and it doesn't turn it off. (You might not notice depending on your colors.)
<Rossen_> q
<dbaron> fantasai: I think the difference here is there's no good use case for an author setting color scheme and not setting other colors, but there are reasons the author should expect that setting forced-color-adjust:none actually turns it off.
<dbaron> fantasai: I think contrast issue you're concerned about is ???
<dbaron> TabAtkins: I think you're off... when author sets forced-color-adjust.. ??? ... either author is setting some colors and letting others through which violates a11y guidelines, or they're seting all the colors and it doesn't matter.
<dbaron> fantasai: They know what color is inheriting through.
<dbaron> TabAtkins: There's no guarantee they know what the colors are from the outside, if forced-color is happening, or if they're distributing a component.
<dbaron> TabAtkins: So the inherited color is potentially a mystery. They're potentially mixing it with known colors (bad), or overriding all. I think people still do the bad case, and we should give a more-likely-accessible result.
<fantasai> Plenty of authors write pages where they set the background on lots of elements, but only set the text color on the root
<TabAtkins> (This convo did happen in the issue, we're re-arguing it.)
<fantasai> This is normal and expected, and it's not a problem.
<dbaron> Rossen: Sounds like we're probably not ready to resolve, but let's hear from fantasai and emilio.
<dbaron> emilio: [we can hear him but he can't hear us]
<Rossen_> emilio, we can hear you!
<fantasai> But it *is* a problem if you set `forced-color-adjust: none` on an element.
<emilio> lol
<fantasai> because the color you've inherited is no longer the one you expected.
<emilio> brb
<dbaron> fantasai: It's normal and common for authors to set text colors on root element, and set background colors on other elements without resetting the text color. There's no problem with it, because you know what color is inherited because you control the whole page. This doesn't hold for components, but not all pages are component-based.
<Rossen_> ack emilio
<dbaron> emilio: So I was going to argue the same thing as TabAtkins... that doing it at computed value causes contrast issues by default. Specifying color-scheme may not end up in a color scheme change... may specify normal. It's not clear that authors will realize their colors are wrong.
<dbaron> (was there a "no" I missed?)
<TabAtkins> Right, even in color-scheme changes authors can't necessarily predict the color that they'll receive. So no, this is bad *even in an entirely first-party situation*.
<dbaron> Rossen: Continue discussion in github issue, bring back for resolution next week.
<tantek> +1 Rossen
<fantasai> scribenick: fantasai
fantasai commented 2 years ago

Overview

The key question here is whether system color keywords compute to an absolute color, and inherit as such, or whether they remain keywords and resolve at used-value time. My takeaway from the WG discussion was that the hangup here is, “which one results in more authoring mistakes?”

The two examples to think about are how this is impacted by color-scheme and forced-color-adjust changes specified by an author on a descendant of the root.

Color Scheme Example

Emilio's example in the OP, showing the effects of author-specified color-scheme changes:

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

In this case the body text is light on a dark background. If system colors compute to absolute before inheriting, then “I'm light” will be light text. Is this expected or unexpected, and will it cause more or fewer mistakes than if we resolved after inheritance (yielding dark text for the element with color-scheme: light)?

Forced Colors Example

The other example is about forced-color-adjust, and was brought up when we originally decided to make them resolve at used-value time:

<!doctype html>
<style>
  :root { color: black; background: white; }
  .special { forced-color-adjust: none; background: yellow; }
</style>
Author says I'm black on white.
<div class=".special">
  What color am I?
</div>

In this case, the text is black on white. If forced-colors mode is on, however, the text might be forced to be, e.g. white on black. But inside the “special” div, we've turned off forced-colors mode.

If system colors compute to absolute before inheriting, then in forced white-on-black mode “What color am I?” will be white. Is this expected or unexpected, and will it cause more or fewer readability problems than if we resolved after inheritance (yielding the original color, black)?

Issue

The proposed change is to make the system colors compute to absolute before inheriting. The existing spec is to have them resolve on the element to which they apply.

pxlcoder commented 2 years ago

I have been discussing this issue with @smfr, and WebKit believes that system colors should resolve at used-value time, after inheritance. Our reasoning is as follows:

svgeesus commented 2 years ago

I haven't seen much discussion on this point:

There's an argument that managing computed values that are linear combinations of, potentially, 18 different channels is unweildy. I take this argument as valid.

Are implementers all on board with that (rare, but certainly possible) complexity?

alisonmaher commented 2 years ago

There's an argument that managing computed values that are linear combinations of, potentially, 18 different channels is unweildy. I take this argument as valid.

Are implementers all on board with that (rare, but certainly possible) complexity?

I don't have enough context on this particular argument to comment, but based on the discussion in https://github.com/w3c/csswg-drafts/issues/5780#issuecomment-1019490769, it isn't clear to me in what case we would have 18 different combinations. Could you provide an example?

Regarding this issue as a whole, I see arguments on both side and could likely come up with use cases to support either decision, so I personally could live with either path forward (i.e. reverting the relevant issues or keeping things as-is). However, this issue is currently blocking us from shipping forced-color-adjust: preserve-parent-color in Chromium, which will help solve a use-case related to SVGs and currentColor that is a developer pain point. So it would be useful to come to a consensus on this either way.

One thing that I will say is that I do think the use cases provided by @fantasai are valid. Particularly related to Forced Colors Mode. If we revert back to computed value time, authors won't have the option to inherit through the original colors when forced-color-adjust: none is set. Whereas with the current approach, developers can choose between three forced-color-adjust values to get varying behavior.

The current use cases to support having system colors compute to themselves aren't as convincing to me, so perhaps a less controversial path forward here would be to leave Forced Colors Mode alone (i.e. continue to force colors at used value time), and only revert https://github.com/w3c/csswg-drafts/issues/3847. This would require us to add a bit to tell if a color was a system color, as @tabatkins mentioned, but I don't think this should be a problem. Thoughts?

emilio commented 2 years ago
  • Form controls already change their appearance across color-scheme boundaries, without any other author style changes. Authors may expect system colors to match form controls. In this scenario, we believe that authors should not need to redeclare color in order to match the current color-scheme.

When would that happen? Form controls already specify color and background-color (and borders and so on) in the UA stylesheet, e.g. here for WebKit. Do you have an example that behaves suboptimally if we resolve at computed-value time?

I don't have enough context on this particular argument to comment, but based on the discussion in https://github.com/w3c/csswg-drafts/issues/5780#issuecomment-1019490769, it isn't clear to me in what case we would have 18 different combinations. Could you provide an example?

When you interpolate system colors, if they resolve at used value time, in order to interpolate correctly you need to keep N channels (where N is the number of system colors), plus currentColor, plus potentially others.

I guess you can express it as a color-mix(), but I don't recall whether color-mix() resolved at computed or used-value time either per spec (in Gecko it resolves at computed-value time).

emilio commented 2 years ago
  • Form controls already change their appearance across color-scheme boundaries, without any other author style changes. Authors may expect system colors to match form controls. In this scenario, we believe that authors should not need to redeclare color in order to match the current color-scheme.

Err, upon re-reading I think I may have misread this, and now I'm not sure about the argument you're trying to make.

I guess the argument you're making is that you want every element to react to color-scheme changes if it inherits a system color for consistency with form controls? If so... I don't think that's great behavior because color would react to the change but backgrounds won't. Do you expect authors to re-declare backgrounds but not need to re-declare colors? (Sorry if that's not the argument you're making though).

In any case, I think the behavior of resolving at computed-value time is generally what authors would expect / the less error-prone behavior. The fact that we needed to add this behavior to force-color-adjust by default is kind of telling, IMO (https://github.com/w3c/csswg-drafts/issues/6310), and as @alisonmaher says the use-cases for this for forced colors can be achieved in other ways if necessary.

svgeesus commented 2 years ago

I don't recall whether color-mix() resolved at computed or used-value time either per spec (in Gecko it resolves at computed-value time).

There is a reason you don't recall :) In CSS Color 5, Resolving <color> Values doesn't have a section on resolving color-mix(). It should.

It does have Serializing color-mix(), which serializes the result as a single color in the specified colorspace used for mixing.

svgeesus commented 2 years ago

For comparison, color-contrast() resolves at computed value time

emilio commented 2 years ago

You mean at used value time right? in that example, color-contrast() is preserved in the computed value (which means that it resolves at used value time).

svgeesus commented 2 years ago

Yes the individual color values are resolved at computed value time, and at used value time the whole function is evaluated and the winning result is the used value.

alisonmaher commented 2 years ago

The fact that we needed to add this behavior to force-color-adjust by default is kind of telling, IMO (https://github.com/w3c/csswg-drafts/issues/6310), and as @alisonmaher says the use-cases for this for forced colors can be achieved in other ways if necessary.

@emilio My comment in https://github.com/w3c/csswg-drafts/issues/6310 was referring to the fact that authors can currently achieve the results they want by setting forced-color-adjust: auto for SVG icons given that things happen at used value time. However, I do think that moving forced colors to computed value time will give authors more limited options in achieving the results they want.

Here's an example that I found while searching around: https://codepen.io/AmeliaBR/pen/xxJBgp. In this example, Chromium and Gecko produce different results in Forced Colors Mode due to the difference in inheritance (see screenshots below). I think Chromium's behavior is more likely to be what an author would expect in this case. Would an author have an option to get this case working in Gecko without re-declaring the original (non-forced) styles that would have been inherited previously?

Chromium's behavior (used value time):

chromium

Gecko's behavior (computed value time):

gecko

upsuper commented 2 years ago

Here's an example that I found while searching around: https://codepen.io/AmeliaBR/pen/xxJBgp.

I see the same rendering between Chrome and Firefox on this example. I also don't see how it should differ based on used-value time vs. computed-value time. Seemingly it is only about how SVG use works.

MReschenberg commented 2 years ago

I think Alison's screenshot is with HCM enabled (in firefox: about:preferences > colors > set dropdown to "always"). I can reproduce her results on macOS.

emilio commented 2 years ago

@emilio My comment in #6310 was referring to the fact that authors can currently achieve the results they want by setting forced-color-adjust: auto for SVG icons given that things happen at used value time. However, I do think that moving forced colors to computed value time will give authors more limited options in achieving the results they want.

That is true.

Here's an example that I found while searching around: https://codepen.io/AmeliaBR/pen/xxJBgp. In this example, Chromium and Gecko produce different results in Forced Colors Mode due to the difference in inheritance (see screenshots below). I think Chromium's behavior is more likely to be what an author would expect in this case.

Is it though? If you use currentColor, you most likely want stuff to fit with the surrounding text, which is Gecko's behavior.

In any case guessing author intent is hard, and you're right that forcing colors at computed value time the original author-specified colors are lost down the tree (though that's different from the question of whether system colors should themselves resolve at computed or used value time).

alisonmaher commented 2 years ago

Is it though? If you use currentColor, you most likely want stuff to fit with the surrounding text, which is Gecko's behavior.

Yeah, I think that is a more common scenario, so having that be the default behavior for SVGs makes sense. But I do think having the option to inherit through the original values can be valuable if that's what the author does end up wanting in certain situations.

In any case guessing author intent is hard, and you're right that forcing colors at computed value time the original author-specified colors are lost down the tree (though that's different from the question of whether system colors should themselves resolve at computed or used value time).

Agreed, I think these would be two independent resolutions. And the arguments for resolving system colors at computed value time do seem valid to me.

svgeesus commented 2 years ago

Agreed, I think these would be two independent resolutions. And the arguments for resolving system colors at computed value time do seem valid to me.

The only thing stopping me pushing hard for fully resolving system colors at computed value time, is concern on the knock-on impact for forced colors mode (the arguments around which I am not sure I fully understand).

I'm not arguing for fully resolving currentColor, btw. Handling that one special case seems necessary, and tractable.

alisonmaher commented 2 years ago

The only thing stopping me pushing hard for fully resolving system colors at computed value time, is concern on the knock-on impact for forced colors mode (the arguments around which I am not sure I fully understand).

Are you referring to the bit that would be needed to tell if something was a system color (which would be needed if forced colors mode happens at used value time while system colors resolve at computed value time)? Fwiw we are currently doing something similar to this in Chromium today.

svgeesus commented 2 years ago

I was assuming we had consensus that the bit would be needed, per the adjusted title of this issue.

alisonmaher commented 2 years ago

Gotcha, I think we can continue to force colors at used value time while system colors resolve at computed value time (as long as we have that bit). Was there some other impact on forced colors mode you were referring to in your previous comment?

Perhaps you were referring to some of the impacts noted in https://github.com/w3c/csswg-drafts/issues/6773#issuecomment-1062008116? These points were assuming we would also move forced colors mode to computed value time, but if we decide to leave forced colors mode alone, then those impacts should no longer be an issue.

svgeesus commented 2 years ago

Thanks for the clarification @alisonmaher in that case I think we could propose to adopt the resolution in the issue title.

svgeesus commented 2 years ago

As proposed earlier:

So, in 4.7.5. Resolving other colors, instead of

Each <system-color> keyword computes to itself. Its used value is the corresponding color in its color space.

we would have

Each <system-color> keyword computes to the corresponding color in its color space. However, implementations which also implement [forced color mode]() must maintain an internal flag for such colors, to preserve the fact that they were specified as system colors and must not be forced.

We would also need a change to 4.8.6. Serializing other colors because currently, <system-color> keywords serialize as the lowercased name. They would now serialize as rgb(nn, nn, nn) same as named colors, right?

svgeesus commented 2 years ago

This will also require changes to WPT system-color-compute.html

alisonmaher commented 2 years ago

That looks correct to me. There may be some updates needed around color-scheme, as well. (i.e. to clarify that it affects the computed value of system colors)

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed 5. [css-color] [css-color-adjust] Make system colors fully resolve (but flag they were system colors) thus reversing the resolution of #3847, and agreed to the following:

The full IRC log of that discussion <fantasai> Topic: 5. [css-color] [css-color-adjust] Make system colors fully resolve (but flag they were system colors) thus reversing the resolution of #3847
<fantasai> github: https://github.com/w3c/csswg-drafts/issues/6773
<chris> q+
<Rossen_> ack chris
<fantasai> chris: We basically got agreement on this issue, Alison seemed to like what I proposed and I drafted into the spec
<fantasai> chris: Need to update tests on WPT
<fantasai> chris: and any knock-on effects in other issues
<fantasai> chris: Apart from that, I thin issue is ready to close
<Rossen_> q?
<TabAtkins> weird that the bot didn't remove Agenda+ last time this was discussed
<fantasai> chris: Proposal is to make <system-colors> fully resolve, but flag they were system colors so that forced-colors mode doesn't touch them
<fantasai> smfr: For forced color mode, does that implementations that don't implement forced color mode, but do implement light and dark mode don't need flags?
<fantasai> TabAtkins: flags only necessary for forcing colors
<TabAtkins> fantasai: I think the other behavior would be better, but I understand it's debateable and this is easier to implement bc you don't need ot maintain all the system colors as seaprate channels
<TabAtkins> 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.
<TabAtkins> chris: suggested wording?
<TabAtkins> fantasai: I can come up with some, yeah
<fantasai> chris: Once we resolve on this, can I ask for another resolution?
<fantasai> Rossen_: Any objections to this proposed resolution?
<fantasai> RESOLVEDS: System colors compute to absolute colors, but must not be affected by forced-colors mode.
<fantasai> chris: There's been this change and some others, would like to resolve to re-publish Color 4 and Color 5
<fantasai> +
<fantasai> +1
<fantasai> Rossen_: Any objections?
<fantasai> RESOLVED: Republish css-color-4 and css-color-5
<chris> once the improved wording from fantasai is in, of course