search

w3c / csswg-drafts

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

[css-color] Resolving color-mix() #7302

Open emilio opened 2 years ago

emilio commented 2 years ago

https://drafts.csswg.org/css-color-5/#resolving-contrast has some rules to resolve color-contrast() values, however I'm not sure they are clear or they match what implementations want or can do.

https://github.com/web-platform-tests/wpt/issues/34158 is an example of this, where it seems like WebKit resolves color-mix() at parse-time (they don't seem to support currentColor inside color-mix()).

In Gecko, I opted for the approach of keeping color-mix() in the specified value, but resolve at computed-value time (except for currentColor which is special of course, but that's not observable anyways because getComputedStyle returns resolved colors).

It's clear that some amount of these functions needs to be preserved all the way to computed-value time (system colors), or even used-value time, for currentColor.

However, it's not clear what amount needs to be preserved at parse-time vs. not. Generally, we preserve stuff at parse-time, (so element.style.color would return color-mix(red, blue) for example. But then there's the precedent of calc() where we normalize generally as much as possible.

I tend to think preserving the function at parse-time is simpler and maybe less confusing for authors, but I don't particularly mind either way.

cc @weining @svgeesus @lilles

weinig commented 2 years ago

I think you meant to cc me (@weinig).

I am a bit unclear on where CSS stands with regards to preserving input. For instance, I CSS Images 4 says this in its serialization section:

To serialize any function defined in this module, serialize it per its individual grammar, in the order its grammar is written in, omitting components when possible without changing the meaning, joining space-separated tokens with a single space, and following each serialized comma with a single space.

I would love for this to be more explicit in more places.

emilio commented 2 years ago

Gah, yeah, got tricked by the autocomplete, sorry!

svgeesus commented 2 years ago

@emilio said:

In Gecko, I opted for the approach of keeping color-mix() in the specified value, but resolve at computed-value time (except for currentColor which is special of course, but that's not observable anyways because getComputedStyle returns resolved colors).

That seems preferable to me; and currentColor gets resolved at used-value time.

@weinig I assume the fact that WebKit currently resolves color-mix() at parse time, and the fact that it currently doesn't handle currentColor in color-mix(), are related?

CSS Color 5 doesn't mention color-mix() in the Resolving Color Values section, and it should. Maybe we can nail this down this week (in this issue, and resolve on the call)?

svgeesus commented 2 years ago

CSS Color 5 doesn't mention color-mix() in the Resolving Color Values section, and it should. Maybe we can nail this down this week (in this issue, and resolve on the call)?

Proposed text for review is now at

7.2. Resolving color-mix() values

and revised text and example at

7.1 Resolving color-contrast() values

Both of these specifically call out currentColor, which does not resolve to a corresponding color in a specific color space.

Comments and corrections welcome.

Once we have agreement I will add more examples to both sections.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed css-color] Resolving color-mix() / color-contrast().

The full IRC log of that discussion <dael> Topic: css-color] Resolving color-mix() / color-contrast()
<dael> github: https://github.com/w3c/csswg-drafts/issues/7302
<chris> People should also have https://github.com/w3c/csswg-drafts/issues/6168 open as they cover basically the same thing
<dael> chris: 7302 is about resolving color-mix and color-contrast. Have spec text but didn't say how to resolve
<dael> chris: Have another issue that talks about same thing. I'll copy discussion to both
<dael> chris: Suggested wording, emilio said it's slightly wrong and I agree. There is prop wording in color 5
<dael> chris: lea did a little codepen that implt color-mix so you can see how it works and get calc
<dael> chris: Useful to see what's happening, particularly when currentColor involved.
<dael> chris: If we can agree here I'll put this in the spec
<dael> chris: Color-mix has to inferit in as the whole thing
<chris> https://codepen.io/svgeesus/pen/QWQrQqr
<dael> jensimmons: codepen URL? I don't see it in the issue
<fantasai> +1 to computing currentColor components to themselves
<fantasai> even if that requires maintaining notations through inheritance
<dael> chris: It's in the other issue; discussion is happening in both
<dael> chris: I put in concrete wording. emilio pointed out it's a bit wrong, but other then that do people like the wording?
<dael> chris: smfr or jensimmons do you know what Sam is thinking?
<dael> smfr: Not sure I do. Can you give prop wording comment?
<chris> https://drafts.csswg.org/css-color-5/#resolving-color-values
<dael> chris: Dropping a link
<dael> chris: emilio didn't like the section being called resolving color values and I asked him about that. Didn't hear back yet. That's what it's named in color 4
<dael> fantasai: Call it computing color values?
<dael> chris: Resolving makes it sound used. This is both computed and used value
<dael> fantasai: I don't know what to do. It's fine then.
<dael> chris: It's different if currentColor is involved. In WK this is at parse time and WK doesn't do currentColor b/c it didn't know what to do. I suspect WK will need to change
<dael> fantasai: If I understand intention, it's that you compute the winning color and that becomes computed. If currentColor is used you compute individual values but don't resolve until used value time
<dael> fantasai: That makes sense. Not exactly what you wrote but if that's what you're going for good
<dael> fantasai: You wrote another sentence about resolving the used value to compute...confusing.
<dael> chris: That is intent. used is the winning color for contrast and the mix value for mix
<dael> fantasai: used is always a color. current is not always for currentColor
<dael> chris: Yes.
<fantasai> s/current is/computed is/
<dael> chris: Other thing is emilio opened an issue a while ago that if we resolve another bug happy to ship. bug closed today so I take as happyt o ship from Moz. Want to know if WK is happy to ship too
<dael> chris: Both browsers you have to run with flag enabled to get riht result so all tests flag
<dael> smfr: I think with WPT it runs with flags enabled
<dael> jensimmons: I think it enables for chrome and firefox but not safari
<dael> smfr: May want to fix currentColor before shipping on by default
<dael> chris: But sounds near-term thing on what spec should say?
<dael> fantasai: Seems like we should fix up spec to say what we want shipped and once that's ready we say go ahead
<dael> Rossen_: Do you want to go back and do that and let us know once ready
<dael> chris: Have to assume that's done to cover item 3 on agenda
<dael> Rossen_: We can resolve to accept text that is not quite correct. Let's consider the request once more next week and we can go from there
<dael> chris: Okay
josepharhar commented 2 years ago

I implemented color-contrast() in chromium a little while ago, but I've been holding off on shipping it because I got feedback that I was doing the whole calculation at parse time instead of used-value time, which was making it not work properly with currentColor.

I guess I sort of assumed that color-contrast was already specced to be resolved at used-value time instead of parse time, but now that I see this issue I'm not so sure. I'm having a hard time fully following the discussion here, but is the idea that we should make color-contrast get resolved at used-value time and support currentColor? Are there any WPTs for this?

(I don't know what the difference is between used-value time and computed-value time)

cc @andruud

svgeesus commented 2 years ago

(I don't know what the difference is between used-value time and computed-value time)

Computed values inherit to children, so that affects currentColor

svgeesus commented 2 years ago

@emilio @weinig these edits look good now?

emilio commented 2 years ago

@svgeesus The specified value shouldn't resolve colors. color-mix(blue, red) should be color-mix(blue, red), not color-mix(rgb(0, 0, 255), rgb(255, 0, 0)).

svgeesus commented 2 years ago

Ah, I see. So, just remove the text there about specified values?

svgeesus commented 2 years ago

@emilio now?

emilio commented 2 years ago

I think so, thanks!

svgeesus commented 2 years ago
fantasai: Seems like we should fix up spec to say what we want shipped and once that's ready we say go ahead Rossen_: Do you want to go back and do that and let us know once ready

Its ready, here is the text on resolving color-mix() and resolving color-contrast()

mysteryDate commented 2 years ago

From https://drafts.csswg.org/css-color-5/#resolving-mix:

Otherwise (if currentColor was used in the function), the computed value is the color-mix() function with each parameter...

Is currentColor the only issue? What about system colors that depend on the color-scheme computed value, e.g.:

div { color: color-mix(in lch, canvastext, blue) }
div.dark { color-scheme: dark; }

Depending on whether an element has the class "dark" or not, the computed value will be different. Serializing the color declaration in the div rule above cannot give a reasonable serialization as a <color>.

@lilles, who I am basically quoting here, might have more to say.

mysteryDate commented 2 years ago

@svgeesus The specified value shouldn't resolve colors. color-mix(blue, red) should be color-mix(blue, red), not color-mix(rgb(0, 0, 255), rgb(255, 0, 0)).

@svgeesus Should we add some tests here to reflect that? https://github.com/web-platform-tests/wpt/blob/master/css/css-color/parsing/color-valid-color-mix-function.html

svgeesus commented 2 years ago

Should we add some tests here to reflect that?

That would be great

svgeesus commented 2 years ago

System colors are already defined:

Each <system-color> keyword computes to the corresponding color in its color space. However, such colors must not be altered by 'forced colors mode'.

mysteryDate commented 2 years ago

System colors are already defined:

Each <system-color> keyword computes to the corresponding color in its color space. However, such colors must not be altered by 'forced colors mode'.

What is this in response to?

mysteryDate commented 2 years ago

Should we add some tests here to reflect that?

That would be great

See: crrev.com/c/3945930

mysteryDate commented 2 years ago

It seems like firefox nightly never eagerly evaluates the resulting color mix, input values are always returned as-is at specified value time. I'm beginning to think that the expectations in this test are entirely incorrect?

https://github.com/web-platform-tests/wpt/blob/master/css/css-color/parsing/color-valid-color-mix-function.html

svgeesus commented 2 years ago

What is this in response to?

It was in response to

Is currentColor the only issue? What about system colors that depend on the color-scheme computed value, e.g.:

svgeesus commented 2 years ago 
div { color: color-mix(in lch, canvastext, blue) }
div.dark { color-scheme: dark; }

Depending on whether an element has the class "dark" or not, the computed value will be different. Serializing the color declaration in the div rule above cannot give a reasonable serialization as a <color>.

Wouldn't the serialization on the div use the initial value of color-scheme which is normal?

mysteryDate commented 2 years ago

Okay, I'm still wondering about this test though:

https://github.com/web-platform-tests/wpt/blob/master/css/css-color/parsing/color-valid-color-mix-function.html

Is it valid? Firefox is not eagerly computing the colors at specified value time, though Safari is.

mysteryDate commented 2 years ago

Another open question: what should we do for negative calc values? From https://w3c.github.io/csswg-drafts/css-values-4/#calc-range:

Parse-time range-checking of values is not performed within math functions, and therefore out-of-range values do not cause the declaration to become invalid. However, the value resulting from an expression must be clamped to the range allowed in the target context. Clamping is performed on computed values to the extent possible, and also on used values if computation was unable to sufficiently simplify the expression to allow range-checking. (Clamping is not performed on specified values.)

This would mean that for something like color-mix(in srgb, red calc(-10%), blue);, calc(-10%) will get accepted, then clamped to zero, so the resulting color will be blue. This is what Firefox is doing, Safari rejects the input.

svgeesus commented 2 years ago

Another open question: what should we do for negative calc values?

Hmm. I thought that was already covered:

Percentages are required to be in the range 0% to 100%. Negative percentages are specifically disallowed.

But then the list of steps for percentage normalization doesn't say what to do if they happen anyway. It needs a new step between 2 and 3 where any value < 0% gets clamped to 0%, and any value > 100% gets clamped to 100%

LeaVerou commented 2 years ago

Another open question: what should we do for negative calc values?

This is not something that needs to be defined specifically for color-mix(), it's the same handling as any other property or function that only allows positive numbers, e.g. padding or <border-radius> in inset().

lilles commented 2 years ago

Another open question: what should we do for negative calc values?

This is not something that needs to be defined specifically for color-mix(), it's the same handling as any other property or function that only allows positive numbers, e.g. padding or <border-radius> in inset().

Yes, this is generally covered by the css-values spec which says e.g. calc(-10%) should not be invalid at parse/specified time here even if negative percentages are not allowed, but should be clamped to the allowed range at computed value time.

There is one tricky part that I'm not sure if we have for other properties, and that is that color-mix() can be invalid even when both percentage values are separately within the allowed range. If you have two negative percentages (or 0%), and they are clamped at computed value time, you will end up with two 0% values which is still invalid per spec.

svgeesus commented 2 years ago

If you have two negative percentages (or 0%), and they are clamped at computed value time, you will end up with two 0% values which is still invalid per spec.

I agree that this case needs to be explicitly specified. I suggest it is treated the same as "both percentages omitted" which results in a 50:50 mix.

svgeesus commented 2 years ago

it's the same handling as any other property or function that only allows positive numbers, e.g. padding or <border-radius> in inset()

True, although some examples demonstrating this wouldn't hurt.

lilles commented 2 years ago

If you have two negative percentages (or 0%), and they are clamped at computed value time, you will end up with two 0% values which is still invalid per spec.

I agree that this case needs to be explicitly specified. I suggest it is treated the same as "both percentages omitted" which results in a 50:50 mix.

I don't have any strong opinions here.

lilles commented 2 years ago

If you have two negative percentages (or 0%), and they are clamped at computed value time, you will end up with two 0% values which is still invalid per spec.

I agree that this case needs to be explicitly specified. I suggest it is treated the same as "both percentages omitted" which results in a 50:50 mix.

I don't have any strong opinions here.

Another possibility is to make it invalid at computed value time. That would be the first case of values being invalid at computed value time for values not involving var() references.

svgeesus commented 2 years ago

I can see the case for IACVT. Deliberately omitting both percentages has clear authorial intent (mix these two colors, equally) while two negatives does not.

svgeesus commented 8 months ago

Looping back to this, was the consensus to make negative values which then get clamped to 0 and sum to 0 IACVT?