-
Notifications
You must be signed in to change notification settings - Fork 683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[css-color-adjust-1] Spec currently breaks use of currentColor for SVG icons in WHCM #6310
Comments
Since it wasn't entirely clear from the above: To avoid breaking existing accessible SVGs, It is less important if the The current spec handles the system color keywords as desired, but not There are two ways this could be achieved, that I see:
Given all the past discussion, I would lean towards the second option, adding a |
Thanks @AmeliaBR for bringing this one up - I do agree that the SVG icon use case is a more common scenario, and it would be helpful for developers to continue to use The only concern that I have is that if an ancestor of an SVG element has (As a side note: authors can currently set |
The CSS Working Group just discussed
The full IRC log of that discussion<dael> Topic: [css-color-adjust-1] Spec currently breaks use of currentColor for SVG icons in WHCM<dael> github: https://github.com//issues/6310 <dael> alisonmaher: This is around handling currentColor. AmeliaBR noted that SVG icons will not inherit appropriate forced-color through currentColor. <dael> alisonmaher: 2 solutions. 1 undo the resolution to use forced colors at used value time. 2) intoduce a color only value that only adjusted color. Make that default for SVGs. b/c color only effects svg through currentColor works well for icon <TabAtkins> q+ <dael> alisonmaher: Only possible unexpected is an svg ancestor set forced-color to none the svg would still set to currentCOlor. This seems rare so in favor of color-only. Looking for other opinions <Rossen_> ack TabAtkins <dael> TabAtkins: Definitely need to fix this. Only consideration to fix issue you raised could the value act as none if inheritied value is none but act as color if it's auto. <dael> TabAtkins: Then it still works as expected <dael> alisonmaher: Good idea. Will need to look into how to impl but could be good way to handle <dael> TabAtkins: Seems reasonable to add, I'm in value <dael> s/value/favor <dael> Rossen_: Prop to fix the issue through a spec clarification and not adding color-only value? <dael> TabAtkins: NO, still add value. The value acts as alisonmaher and AmeliaBR spec in auto case. If inherited is none it would act as none. It would automatically opt-out <dael> Rossen_: I see <dael> Rossen_: Other opinions or suggestions? <dael> Rossen_: alisonmaher do you want a resolution now? Or do you prefer to go back and figure out if implementable? <dael> alisonmaher: Can resolve and come back if impl is tricky. Seems adding color-only value we can resolve on <dael> Rossen_: Objections to add color-only value as described in the issue and clarified by TabAtkins <dael> RESOLVED: add color-only value as described in the issue and clarified by TabAtkins |
Reflagging for the agenda, because I don't think we quite got this situation correct. Consider an SVG where the author has set 'color' to an explicit value on the So to target just the case we need to address (icons wanting to inherit the surrounding context's text color) I think what we really want to do is limit the forced-color application to when 'color' is inherited from the SVG's context, and not to the use of the 'color' property in general. Something like:
|
That's a great point, if an SVG has set a @fantasai I may be misinterpreting the wording, but I was curious how
I'm also wondering if this were to be done at computed value time, would this re-raise the issue of handling transitions in forced colors mode: #5419? |
Was thinking about this a bit more as well, and wanted to flag something for the group. Adding a value to Take this common practice, for example: https://codepen.io/somelaniesaid/pen/Yzqxogg. "Light mode" is the default style, where the SVG parts are set to Given that SVGs can be used in so many different ways, it might not be possible to find a solution that appropriately balances potential breakages between use cases without adding just a tiny bit more work for authors. Adding a new value might still be the best way to go at this point if it minimizes breakage.* I just want to make sure that in considering the paths forward, we're aware that SVG icons will be broken on some sites given more recent * As active members of the CSSWG are probably aware, I pivoted to some other projects before some of the more recent resolutions and haven't been part of the discussions for awhile. So I will to defer to Alison, who is still active in this work and may have data/insights that I don't. :) |
@melanierichards The idea in this issue was to make this the UA's default value for |
Oh goodness, seems I misread/missed a very key element of this proposal. 🤦♀️ Thanks for pointing that out, @fantasai! Seems this is headed in the right direction then, please ignore my comment. |
The CSS Working Group just discussed
The full IRC log of that discussion<fantasai> Topic: Color Adjust<fantasai> github: https://github.com//issues/6310 <fantasai> https://github.com//issues/6310#issuecomment-858112357 <fremy> fantasai: the current proposal is not gonna get us in good shape in all cases <fremy> fantasai: currently our resolution would not work if the svg has set its color explicitly <fremy> fantasai: so my new proposal is that if the color is orginating from outside the svg then we recolor, but if not then we keep it <fremy> fantasai: proposal is to add a new keyword for that magic behavior which depends on the origin of the value of color <alisonmaher> q+ <iank_> I'm getting very warbled audio - is this is the for others? <fremy> fantasai: (if you are not inheriting from outside, then we don't reset the color) <fantasai> alisonmaher: I agree with the proposal <astearns> ack alisonmaher <fantasai> alisonmaher: only problem I wanted to ask about was whether we can do at computed value time <fantasai> alisonmaher: if we take the used value of the color, might expose the color where otherwise wouldn't <fantasai> TabAtkins: :visited won't be exposed that way <fantasai> TabAtkins: that's done by selector-hacking <fantasai> TabAtkins: and the rest of colors are already automatically exposed via getComputedStyle <fantasai> TabAtkins: because gCS returns used value of color <Rossen_> q <fantasai> TabAtkins: so not sure there's info leakage problem <fantasai> Rossen_: While we're on the topic, wrt TAG review <fantasai> Rossen_: We convinced ourselves that having a grouping of color values that we essentially return just defaults for, and lie about the computed or used values <fantasai> Rossen_: colors used for fingerprinting <fantasai> Rossen_: could be a good path forward <fantasai> Rossen_: These are magic values, you won't get the actual values though gCS <fantasai> Rossen_: benefit of user for privacy <astearns> ack fantasai <fremy> fantasai: this is a different issue though <fantasai> fantasai: That's a separate issue, here https://github.com//issues/5710 <fremy> fantasai: in that issue there are further comments there that says why it's probably not possible <fremy> fantasai: but this is not linked to our current issue here <fantasai> astearns: If we end up doing anything special for particular sources of colors <fantasai> astearns: if we add this new mechanism, we'd have to do whatever magic here, too <fantasai> fantasai: You'd also have to taint Canvas, and a lot of other things, yeah. <fantasai> astearns: alisonmaher is it enough to note that, whatever protections would end up happening here also? <fantasai> alisonmaher: We're storing [?] in chrome, so would be possible to do at used-value time <fremy> fantasai: the issue is that would have to track this down the tree <fremy> fantasai: and this type of tracking is usually done via inheritance <fremy> fantasai: I don't think implementations have a special inherit for colors <fantasai> alisonmaher: We inherit that info separately in Chromium <fantasai> astearns: So what do we resolve to deal with this <fremy> fantasai: we need to add a new value <fantasai> fantasai: We need to a new value to forced-color-adjust <fantasai> fantasai: as described in https://github.com//issues/6310#issuecomment-858112357 <fantasai> fantasai: We can call it something else, but need something that behaves like that <fantasai> astearns: New value, not something to add to default? <fantasai> TabAtkins: Yes, absolutely <fantasai> TabAtkins: and needs to be set in default UA stylesheet for SVG root element <fantasai> astearns: exposed to author styles? <fantasai> TabAtkins: yes; don't want it to be special unspecifiable magic <fantasai> astearns: So proposed resolution is to add this keyword <fantasai> astearns: and to add that to the defautl UA stylesheet <fantasai> RESOLVED: Add new keyword to forced-color-adjust as described above, apply it in UA default stylesheet for SVG root element <fremy> fantasai: other than this issue <fantasai> https://github.com/w3c/csswg-drafts/labels/css-color-adjust-1 <fremy> fantasai: we have no other remaining issue for the spec <fremy> fantasai: so our plan is to make the edit <fremy> fantasai: publish a new draft <fremy> fantasai: and ask for last comments before trying to propose to make a recommendation <fremy> fantasai: so, heads up <fremy> astearns: and get wide review? <fremy> fantasai: we already have sent an email (... missed) <fantasai> https://github.com//issues/5768 <fantasai> in December <fremy> astearns: sounds like a good plan |
When forced color mode was introduced in Chrome 89, the default user agent styles for SVGs were set to `forced-color-adjust: none` in line with the CSS Color Adjustment specification at the time [1]. Unfortunately, this means that using currentColor for stroke and fill in SVGs no longer works as expected in forced color mode. As per the comment in Chromium bug #1164162 [2]: > This is the result of one of the recent spec changes. The spec was updated to force colors at used value time rather than computed value time, which means elements that have "forced-color-adjust: none" set (like svg elements) will inherit their non-forced color values, resulting in a different currentcolor used for stroke and fill in this case than you would get if forcing was done at computed value time. > > See spec issue resolution for more details: w3c/csswg-drafts#4915 It looks like this has since been flagged as an issue with the CSS working group [3] and the spec has been updated to resolve it [4] but it’s going to take a while before that change is reflected in browsers. In the meantime, we can explicitly set `forced-color-adjust: auto` on the OGL logo in order for it to correctly inherit the color from the parent [5]. Once Chromium has been updated so that the default UA styles for SVGs use the new `forced-color-adjust: preserve-parent-color` keyword, and traffic to older versions has dropped off, we can then consider removing this. [1]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210520/#forced-color-adjust-prop [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1164162#c4 [3]: w3c/csswg-drafts#6310 [4]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210616/#forced-color-adjust-prop [5]: w3c/csswg-drafts#6310 (comment)
When forced color mode was introduced in Chrome 89, the default user agent styles for SVGs were set to `forced-color-adjust: none` in line with the CSS Color Adjustment specification at the time [1]. Unfortunately, this means that using currentColor for stroke and fill in SVGs no longer works as expected in forced color mode. As per the comment in Chromium bug #1164162 [2]: > This is the result of one of the recent spec changes. The spec was updated to force colors at used value time rather than computed value time, which means elements that have "forced-color-adjust: none" set (like svg elements) will inherit their non-forced color values, resulting in a different currentcolor used for stroke and fill in this case than you would get if forcing was done at computed value time. > > See spec issue resolution for more details: w3c/csswg-drafts#4915 It looks like this has since been flagged as an issue with the CSS working group [3] and the spec has been updated to resolve it [4] but it’s going to take a while before that change is reflected in browsers. In the meantime, we can explicitly set `forced-color-adjust: auto` on the OGL logo in order for it to correctly inherit the color from the parent [5]. Once Chromium has been updated so that the default UA styles for SVGs use the new `forced-color-adjust: preserve-parent-color` keyword, and traffic to older versions has dropped off, we can then consider removing this. [1]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210520/#forced-color-adjust-prop [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1164162#c4 [3]: w3c/csswg-drafts#6310 [4]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210616/#forced-color-adjust-prop [5]: w3c/csswg-drafts#6310 (comment)
When forced color mode was introduced in Chrome 89, the default user agent styles for SVGs were set to `forced-color-adjust: none` in line with the CSS Color Adjustment specification at the time [1]. Unfortunately, this means that using currentColor for stroke and fill in SVGs no longer works as expected in forced color mode. As per the comment in Chromium bug #1164162 [2]: > This is the result of one of the recent spec changes. The spec was updated to force colors at used value time rather than computed value time, which means elements that have "forced-color-adjust: none" set (like svg elements) will inherit their non-forced color values, resulting in a different currentcolor used for stroke and fill in this case than you would get if forcing was done at computed value time. > > See spec issue resolution for more details: w3c/csswg-drafts#4915 It looks like this has since been flagged as an issue with the CSS working group [3] and the spec has been updated to resolve it [4] but it’s going to take a while before that change is reflected in browsers. In the meantime, we can explicitly set `forced-color-adjust: auto` on the chevron SVG in order for it to correctly inherit the color from the parent [5]. This mimics the fix for the OGL logo in the footer made in 850c0b7. Once Chromium has been updated so that the default UA styles for SVGs use the new `forced-color-adjust: preserve-parent-color` keyword, and traffic to older versions has dropped off, we can then consider removing this. [1]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210520/#forced-color-adjust-prop [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1164162#c4 [3]: w3c/csswg-drafts#6310 [4]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210616/#forced-color-adjust-prop [5]: w3c/csswg-drafts#6310 (comment)
When forced color mode was introduced in Chrome 89, the default user agent styles for SVGs were set to `forced-color-adjust: none` in line with the CSS Color Adjustment specification at the time [1]. Unfortunately, this means that using currentColor for stroke and fill in SVGs no longer works as expected in forced color mode. As per the comment in Chromium bug #1164162 [2]: > This is the result of one of the recent spec changes. The spec was updated to force colors at used value time rather than computed value time, which means elements that have "forced-color-adjust: none" set (like svg elements) will inherit their non-forced color values, resulting in a different currentcolor used for stroke and fill in this case than you would get if forcing was done at computed value time. > > See spec issue resolution for more details: w3c/csswg-drafts#4915 It looks like this has since been flagged as an issue with the CSS working group [3] and the spec has been updated to resolve it [4] but it’s going to take a while before that change is reflected in browsers. In the meantime, we can explicitly set `forced-color-adjust: auto` on the chevron SVG in order for it to correctly inherit the color from the parent [5]. This mimics the fix for the OGL logo in the footer made in 850c0b7. Once Chromium has been updated so that the default UA styles for SVGs use the new `forced-color-adjust: preserve-parent-color` keyword, and traffic to older versions has dropped off, we can then consider removing this. [1]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210520/#forced-color-adjust-prop [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1164162#c4 [3]: w3c/csswg-drafts#6310 [4]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210616/#forced-color-adjust-prop [5]: w3c/csswg-drafts#6310 (comment)
When forced color mode was introduced in Chrome 89, the default user agent styles for SVGs were set to `forced-color-adjust: none` in line with the CSS Color Adjustment specification at the time [1]. Unfortunately, this means that using currentColor for stroke and fill in SVGs no longer works as expected in forced color mode. As per the comment in Chromium bug #1164162 [2]: > This is the result of one of the recent spec changes. The spec was updated to force colors at used value time rather than computed value time, which means elements that have "forced-color-adjust: none" set (like svg elements) will inherit their non-forced color values, resulting in a different currentcolor used for stroke and fill in this case than you would get if forcing was done at computed value time. > > See spec issue resolution for more details: w3c/csswg-drafts#4915 It looks like this has since been flagged as an issue with the CSS working group [3] and the spec has been updated to resolve it [4] but it’s going to take a while before that change is reflected in browsers. In the meantime, we can explicitly set `forced-color-adjust: auto` on the chevron SVG in order for it to correctly inherit the color from the parent [5]. This mimics the fix for the OGL logo in the footer made in 850c0b7. Once Chromium has been updated so that the default UA styles for SVGs use the new `forced-color-adjust: preserve-parent-color` keyword, and traffic to older versions has dropped off, we can then consider removing this. [1]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210520/#forced-color-adjust-prop [2]: https://bugs.chromium.org/p/chromium/issues/detail?id=1164162#c4 [3]: w3c/csswg-drafts#6310 [4]: https://www.w3.org/TR/2021/WD-css-color-adjust-1-20210616/#forced-color-adjust-prop [5]: w3c/csswg-drafts#6310 (comment)
Background:
For inline SVG icons that need to match the surrounding text color, the recommended technique is to use
fill: currentColor
orstroke: currentColor
. In addition to being DRY-er than setting an explicit color, this makes sure that the color adjusts to match any forced color changed (e.g., Windows High Contrast Mode).However, in other SVG, fill colour is part of the graphic and shouldn't automatically be adjusted by high contrast mode.
In #3855, we therefore agreed that
forced-color-adjust: none
should apply to all SVG, with the understanding that thecurrentColor
technique for inline icons would still work because the inherited color from the surrounding HTML markup would already be adjusted.https://drafts.csswg.org/css-color-adjust-1/#propdef-forced-color-adjust
However, because of other complications, in #4915 (comment) it was resolved the forced-color changes apply at used value time, which means they don't affect inherited values.
https://drafts.csswg.org/css-color-adjust-1/#forced-colors-properties
Chromium has shipped this change, and therefore web pages that previously had readable SVG icons in high-contrast mode now have broken icons that don't change to match the currentColor.
https://twitter.com/stommepoes/status/1397186894866288647
https://codepen.io/_mallory/pen/qBrjNpK
https://bugs.chromium.org/p/chromium/issues/detail?id=1213073
As an aside: they hypothetical example @alisonmaher brings up in #4915 (comment) doesn't seem like something that would be common (
currentColor
SVG text on top of an explicitly-coloured SVG background). In contrast, the example CodePen above uses a technique (currentColor
for SVG with transparent background embedded within text) that has been widely advocated as the correct way to do accessible inline SVG icons!The text was updated successfully, but these errors were encountered: