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

preserve-parent-color value for forced-color-adjust CSS property #591

Open
sartang opened this issue Nov 5, 2021 · 13 comments
Open

preserve-parent-color value for forced-color-adjust CSS property #591

sartang opened this issue Nov 5, 2021 · 13 comments

Comments

@sartang
Copy link

sartang commented Nov 5, 2021

Request for Mozilla Position on an Emerging Web Specification

Other information

The main focus of this review is around the introduction of the preserve-parent-color value for the existing forced-color-adjust CSS property.

@chrishtr
Copy link

chrishtr commented Nov 5, 2021

Related: #463, which I think still has no official position?

@alisonmaher
Copy link

General support was provided in #463, although it wasn't specific to forced-color-adjust, which hasn't shipped yet in Firefox:

Regarding the general feature, I think we're generally supportive.

cc @emilio @MReschenberg

@emilio
Copy link
Collaborator

emilio commented Nov 8, 2021

So I'm confused about this value, but mostly I think is because the specified behavior for system colors is not great, see w3c/csswg-drafts#6773.

If we decide that system colors do not evaluate at used value time, is this value still meaningful / useful?

@alisonmaher
Copy link

If system colors no longer evaluate at used value time, I think preserve-parent-color would still be useful, since the main reason this value was added was because of the used value time nature of Forced Colors Mode (and not the used value time nature of system colors).

For example, SVGs currently have forced-color-adjust: none set by default. If fill is set to currentColor on an SVG, it will not inherit the forced color value from its parent, since that gets forced to CanvasText at used value time. This does not match the expectation from authors.

preserve-parent-color is meant to ensure that currentColor computes to whatever the used value of the parent is (i.e. that it inherits the parent's forced color value). By setting this as the default for SVGs instead of forced-color-adjust: none, we meet author expectation in this case.
(See the explainer and the original spec issue for more details).

As a side note: I'm not sure how this value should act in the case when color-scheme is set to something other than the default (and if system colors do not compute to themselves), but I think that would be an issue with forced-color-adjust: none, as well.

@emilio
Copy link
Collaborator

emilio commented Nov 9, 2021

If system colors no longer evaluate at used value time, I think preserve-parent-color would still be useful, since the main reason this value was added was because of the used value time nature of Forced Colors Mode (and not the used value time nature of system colors).

I see. But the whole reason (or at least a big reason) we specified to do forced colors at used-value time was because system colors were specified to resolve at used-value time, right? (see w3c/csswg-drafts#4915 (comment) and co)

Gecko right now resolves forced colors at computed-value time, and I think we can deal with transparency in backgrounds (which is what this complexity was all about iirc) somewhat reasonably...

For example, SVGs currently have forced-color-adjust: none set by default. If fill is set to currentColor on an SVG, it will not inherit the forced color value from its parent, since that gets forced to CanvasText at used value time. This does not match the expectation from authors.

Right, but if preserve-parent-color was implemented, yet system colors resolved at computed-value time (like they do in all shipping engines right now), then you'd get the expected rendering without needing preserve-parent-color, right?

As in, I think Firefox renders the explainer as you want / expect: https://crisal.io/tmp/forced-color-adjust-preserve-parent-color.html right? I tried Chrome Canary and I couldn't make forced-color-adjust: preserve-parent-color parse, did I miss something? But given it doesn't parse I assume the behavior I'm seeing (the "C" being white instead of the system color) is the one you don't want, which I agree with.

preserve-parent-color is meant to ensure that currentColor computes to whatever the used value of the parent is (i.e. that it inherits the parent's forced color value). By setting this as the default for SVGs instead of forced-color-adjust: none, we meet author expectation in this case. (See the explainer and the original spec issue for more details).

So if I understand correctly, you want forced-color-adjust: preserve-parent-color anywhere you have forced-color-adjust: none and might have any currentcolor value in the subtree, right? That seems to me like it is pretty much "always", right? Or what am I missing?

As a side note: I'm not sure how this value should act in the case when color-scheme is set to something other than the default (and if system colors do not compute to themselves), but I think that would be an issue with forced-color-adjust: none, as well.

Right, this complicates the whole model even more as you note. I think a simpler model is:

But then this value becomes completely irrelevant / synonym with forced-color-adjust: none, and it doesn't interact with color-scheme, right?

@emilio
Copy link
Collaborator

emilio commented Nov 9, 2021

To be clear, I might be missing something about how Edge/Chromium deals with background-colors differently from Firefox. Do you have a test-case where we render differently that I could look at? Maybe that helps justifying all this complexity, but otherwise I think preserve-parent-color is pretty much a hack.

Or, at least, is there any reason forced-color-adjust: none doesn't behave like preserve-parent-color instead? I think it's just a more intuitive / better behavior. Otherwise currentcolor just misbehaves unless you forget to override it or use preserve-parent-color.

@alisonmaher
Copy link

I see. But the whole reason (or at least a big reason) we specified to do forced colors at used-value time was because system colors were specified to resolve at used-value time, right? (see w3c/csswg-drafts#4915 (comment) and co)

That played a large role in the decision. Although there were other reasons (see comments here and here).

If I understand correctly, then, the proposal in w3c/csswg-drafts#6773 is also, as a side effect, aiming to move forced colors to computed-value time, instead? I think that would re-raise some of the transition issues (#5419), and would make handling the resolution in #4178 somewhat strange (although not impossible).

Gecko right now resolves forced colors at computed-value time, and I think we can deal with transparency in backgrounds (which is what this complexity was all about iirc) somewhat reasonably...

Chromium originally implemented forced colors at computed-value time with background-color handling, so I don't think that is a major issue. (Although it is slightly cleaner to do at used-value time).

Right, but if preserve-parent-color was implemented, yet system colors resolved at computed-value time (like they do in all shipping engines right now), then you'd get the expected rendering without needing preserve-parent-color, right?

Not unless forced colors happened at computed-value time, as well. If both system color resolution and forced colors happened at computed-value time, then yes, preserve-parent-color wouldn't be needed.

As in, I think Firefox renders the explainer as you want / expect: https://crisal.io/tmp/forced-color-adjust-preserve-parent-color.html right? I tried Chrome Canary and I couldn't make forced-color-adjust: preserve-parent-color parse, did I miss something? But given it doesn't parse I assume the behavior I'm seeing (the "C" being white instead of the system color) is the one you don't want, which I agree with.

The reason Firefox renders this correctly is because forced colors is still happening at computed-value time, whereas in Chromium, it is happening at used-value time. preserve-parent-color is implemented behind a flag, so you'll need to apply --enable-blink-features=ForcedColorsPreserveParentColor to see the effects in Chrome Canary.

So if I understand correctly, you want forced-color-adjust: preserve-parent-color anywhere you have forced-color-adjust: none and might have any currentcolor value in the subtree, right? That seems to me like it is pretty much "always", right? Or what am I missing?

I don't think that's necessarily true. The idea of forced-color-adjust: none is to have it render as if forced colors mode was not enabled. So it depends on what the author wants currentColor to evaluate to in this case (if an ancestor has forced-color-adjust: auto set, should currentColor inherit the forced color of the parent or the non-forced color?) I think it would be up to the author to determine this and choose between none and preserve-parent-color accordingly. The case of SVG icons was one where authors expected currentColor to inherit the forced color, which is why we specifically updated the default behavior for SVGs. (Although I'm sure we can come up with cases where an author would expect the opposite behavior for SVGs).

I think a simpler model is:

But then this value becomes completely irrelevant / synonym with forced-color-adjust: none, and it doesn't interact with color-scheme, right?

Correct, if we were to make forced colors work at computed-value time, then preserve-parent-color would be irrelevant. (That didn't seem like the original intention of w3c/csswg-drafts#6773, though).

@dlibby-
Copy link

dlibby- commented Dec 14, 2021

Hi @emilio - did your thinking change at all after the discussions in CSSWG last month via w3c/csswg-drafts#6773? Reading through this thread it sounds like you were in favor of:

  • having system colors not compute to themselves
  • moving forced-colors back to computed value time
  • thus remove the need for preserve-parent-color

One of the drawbacks for forcing colors at computed value time is that elements with forced-color-adjust: none would inherit a mix of colors forced at computed time along with author-specified colors, whereas if colors are forced at used time, only author-specified colors would be cascaded/inherited (i.e. this comment w3c/csswg-drafts#4915 (comment)).

In my estimation preserve-parent-color provides a mechanism for authors to opt in to some form of the 'mixed inherited colors', but if we changed to forced colors at computed time, there wouldn't be a mechanism for authors to get 'author-specified only inherited colors'.

@dlibby-
Copy link

dlibby- commented Jan 14, 2022

Hi @emilio - any updates on your thinking here?

@emilio
Copy link
Collaborator

emilio commented Jan 15, 2022

Hi, sorry, I forgot to reply here. Happy new year btw :-)

In my estimation preserve-parent-color provides a mechanism for authors to opt in to some form of the 'mixed inherited colors', but if we changed to forced colors at computed time, there wouldn't be a mechanism for authors to get 'author-specified only inherited colors'.

Well, yeah, because the author values would be lost at value computation time, right? But I think that's actually a feature, since otherwise you get the kind of contrast issues that we are trying to avoid really easily.

I think I'd rather wait for a resolution to w3c/csswg-drafts#6773. I need to chat with @tabatkins about it because I seem to recall we were talking past each other a bit on the call it was discussed.

@dlibby-
Copy link

dlibby- commented Jan 26, 2022

Thanks for your response (and belated happy new year as well :)

Do you happen to have a rough ETA for discussing with Tab in context of w3c/csswg-drafts#6773? We do know that sites are running into this with our implementation in Chromium (which matches the current spec of 'force at used value time').

Perhaps we could, in the interim, consider changing the default behavior of SVG without having the new property value be web exposed, but hopefully we're not too far from getting consensus on 6773.

@alisonmaher
Copy link

@emilio given the resolution in w3c/csswg-drafts#6773, do you have an updated position on preserve-parent-color or any further concerns?

@dlibby-
Copy link

dlibby- commented Jun 7, 2022

@emilio - any additional thoughts here? We'd like to ship preserve-parent-color to address a specific need for authors, given the existing state of forced colors happening at used value time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Unscreened
Development

No branches or pull requests

5 participants