search

w3c / csswg-drafts

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

[CSS Pseudo] Revisit CSS Custom Properties in highlight pseudos #9909

Open schenney-chromium opened 9 months ago

schenney-chromium commented 9 months ago

In Issue #6641 the issue of CSS Custom Properties used in highlight pseudos, particularly selection, was discussed. The motivating use case was the heavy uses of custom properties to define the colors for selection highlights, and these custom properties were typically defined on :root.

The resolution was to copy custom properties from :root to :root::selection (and other highlights).

In trying to enable Highlight Inheritance for ::selection in chromium, it has come to my attention that custom properties are the recommended way to achieve the behavior of highlight inheritance in the absence of an implementation of highlight inheritance. And this pre-existing workaround breaks when highlight inheritance is actually implemented.

This is a simple example of how custom properties are used:

<style>
  :root {
    --selection-color: lightgrey;
  }
  ::selection {
    background-color: var(--selection-color);
  }
  .transparent {
    --selection-color: transparent;
  }
</style>
<p>Some <strong>not transparent</strong> highlight</p>
<p class="transparent">Some <strong>still transparent</strong> highlight</p>

With the non-spec-compliant originating inheritance implemented by all browsers today, all of the transparent <p> is transparent when selected, including the descendant <strong> element. The corresponding example with highlight inheritance requires this:

<style>
  :root {
    --selection-color: lightgrey;
  }
  ::selection {
    background-color: var(--selection-color);
  }
  .transparent::selection {
    background-color:transparent;
  }
</style>
<p>Some <strong>not transparent</strong> highlight</p>
<p class="transparent">Some <strong>still transparent</strong> highlight</p>

The original example with highlight inheritance renders both paragraphs as grey when selected.

The use of custom properties for selection is out there in the world right now. In particular, developers looking to style selection may well end up on Stack Overflow where they find things like this:

Setting a custom property on inline style to control selection from javascript Controlling selection colors through JS "CSS custom properties / CSS Variables will be a better approach"

As I understand things, it highlights a use case that will continue to exist once highlight inheritance is shipped: You can't set pseudo elements in inline style, so the only way to control the highlights with inline style is through custom properties.

I hate to say this, but I propose option (1) we resolve on inheriting custom properties from the originating element for highlight pseudos.

The alternative, I think , is option (2) to special case ::selection and continue to use originating inheritance as implemented by browsers today, and only use highlight inheritance for the other highlight pseudos. In my opinion that's worse that option 1 given there is some utility in inheriting custom properties for these pseudo elements too.

bramus commented 9 months ago

What about a combination where you look at the originating element and then the highlight pseudo of the parent until you end up at the root? That would be least confusing to authors.

For example, this markup:

<html>
  <body>
    <div>
      <p>…</p>
    </div>
  </body>
<html>

When selecting text in the paragrph, this would be the lookup chain:

schenney-chromium commented 9 months ago

Inheriting through both originating ancestors and highlight ancestors is possible to implement, I think.

On the other hand, I'm reconsidering this proposal anyway because of the memory costs of inheriting so much stuff, plus the potential confusion of where properties are coming from (DevTools support would need to change in Chrome, for instance).

And making the changes either of us suggest does not fix all website breakage. There's no way of getting around sites that have a foo::selection that they only expect to apply to foo, and not its decendents.

I'll ponder this some more and maybe try to figure out how much stuff is broken due to which aspect of the change: custom properties and/or unexpected inheritance.

bramus commented 9 months ago

A model with less load could be to inherit:

The root highlights would still inherit custom properties from :root in this model.

For example, this markup:

<html>
  <body>
    <div>
      <p>…</p>
    </div>
  </body>
<html>

When selecting text in the paragrph, this would be the lookup chain:

This would still allow authors to set a custom property on an intermediary element. For example, if they set --color on <div>, the <p> would end up inheriting that, and thus the p::selection would be able to use --color.

schenney-chromium commented 9 months ago

A model with less load could be to inherit:

  • custom properties from the originating element
  • properties from the highlight tree

The root highlights would still inherit custom properties from :root in this model.

Yeah, that's what I was considering implementing. The nasty part is that one would need to merge the custom properties from the originating element and the highlight parent, with the originating winning any conflicts. It's doable for sure but could do a lot of damage to style resolution performance in the presence of highlight pseudos, and I'm loath to move forward unless there is measurable benefit in the wild.

bramus commented 9 months ago

I'm loath to move forward unless there is measurable benefit in the wild

Measurable benefit as in benefit to developers? Or benefits such as performance-related ones?

Thinking a bit further on all this, I think maybe a general rule for all pseudo-elements could be that they inherit custom properties (only custom ones) from their originating element at the minimum. Specific pseudo elements can then further specify what they do or don’t do. That would keep it practical and predictable to authors.

LeaVerou commented 9 months ago

Coming to this late, I was wondering if the problem statement could be clarified a bit. The first post makes it sound like highlight pseudos don't currently have access to custom property values defined on their originating element or its ancestors, but I'm sure that can't be it, because that would render them almost unusable?

bramus commented 9 months ago

That is currently the specified behavior: highlight pseudos inherit from the highlight chain, with the root highlight inheriting from root.

This is, as you say, unusable and is also what made us postpone shipping this currently specced (and implemented behind a flag) behavior in Chrome.

LeaVerou commented 9 months ago

I made a demo to understand the current situation: https://codepen.io/leaverou/pen/poBzRVP

It looks like custom properties specified on :root are applied, but nothing below that, even if it contains the entire element. That’s …very unfortunate and severely restricts use cases for this.

As an example, I was planning to base Prism v2 (one of the most popular syntax highlighters on the Web) on the CSS custom highlight API, and now I’m not even sure it's possible at all, since different languages need different colors so these cannot just be defined on the root.

bramus commented 9 months ago

That’s why I proposed this:

I think maybe a general rule for all pseudo-elements could be that they inherit custom properties (only custom ones) from their originating element at the minimum. Specific pseudo elements can then further specify what they do or don’t do.

For highlights, in addition to inheriting custom props from the originating element, they could also walk up the highlight tree in addition to that.

dandclark commented 9 months ago

My understanding from @schenney-chromium's initial comment in this issue was that this is strictly a backwards-compatibility problem with the current ::selection behavior, since the new highlight inheritance is a breaking change when applied to best practices around the current behavior.

That shouldn't be a blocker with CSS custom highlights since they've only ever used the new inheritance behavior. So I'm a little unclear on this point:

It looks like custom properties specified on :root are applied, but nothing below that, even if it contains the entire element. That’s …very unfortunate and severely restricts use cases for this.

As an example, I was planning to base Prism v2 (one of the most popular syntax highlighters on the Web) on the CSS custom highlight API, and now I’m not even sure it's possible at all, since different languages need different colors so these cannot just be defined on the root.

This use case can still be realized by defining the colors on the ::highlight() pseudos for elements below :root. This adoption of @LeaVerou's demo shows property definitions being scoped under particular elements: https://codepen.io/daniec/pen/MWRgENy. It's a little clunky but this kind of scoping is definitely possible.

fantasai commented 9 months ago

A model with less load could be to inherit:

  • custom properties from the originating element
  • [standard] properties from the highlight tree

The root highlights would still inherit custom properties from :root in this model.

I agree this idea makes sense. But the inheritance path in the example that follows doesn't. E.g. color and background-color need to inherit purely through the highlight pseudo-tree, without looking to the originating element at all, for things like section::selection { color: blue; background: yellow; } to work reasonably. You can't have a random em { color: red; } in the middle of the section break selection colors within the em.

css-meeting-bot commented 8 months ago

The CSS Working Group just discussed [CSS Pseudo] Revisit CSS Custom Properties in highlight pseudos, and agreed to the following:

The full IRC log of that discussion <TabAtkins> schenney: I filed this base don two things
<TabAtkins> schenney: first, one of the ocmments on the bug report when we tried to enable the highlight inheritacne chain
<TabAtkins> schenney: someone in a tool was setting ac ustom property on ::selection on every element, expected it to use the custom prop from the element
<TabAtkins> schenney: seconds, when I looked at SO about the ::selection pseudo, there was at least one answer that ende dup saying "just use custom props for your selection pseudo"
<TabAtkins> schenney: so this appears to already be a big beahvior, custom properties driving selection behavior
<TabAtkins> schenney: previous browser behavior, this *was* the correct way to do it - customs would inherit thru the originating element chain
<TabAtkins> schenney: So I propose we change the spec to also inherit custom props from originating elements
<TabAtkins> schenney: This may be grat for devs, but it poses problems for impl and spec
<TabAtkins> schenney: First, at minimum it's a memory hog, you have to copy custom props onto all your selection pseudos.
<TabAtkins> schenney: every time the property set changes you have to reallocate a new custom property set for the element
<TabAtkins> schenney: second, what do you do when a custom prop is defined in both the highlight inheritance chain *and* the originating element?
<TabAtkins> schenney: "correct" answer dpeends on context, hard to spec a reasonable behavior
<TabAtkins> schenney: So in hindsight I think we shouldn't make the change. But a lot of webdevs have come in and said we *shoudl* make it.
<TabAtkins> schenney: So, is there anything sensible to do about when there's clashing declarations?
<Rossen_> ack fantasai
<TabAtkins> fantasai: I think one thing suggested in the thread which is fairly simple is to have "normal" properties inherit thru highlihgt chain, but take custom properties from the originating element
<TabAtkins> fantasai: and *only* the originating element
<TabAtkins> fantasai: so you can't set custom props on an article::selection and have that visible to a span::selection
<schenney> q+
<TabAtkins> fantasai: but you can set it on `article` and inherit to the span, and then it'll be visible in span::selection
<TabAtkins> fantasai: I don't like an interleaved where it's inherited from two places at once, but I think in this limited case it's okay. The entire set of normal and the entire set of custom each have a single place to inherit from
<TabAtkins> schenney: I think that's equivalent to saying custom prop defined on highlights are ignored?
<TabAtkins> fantasai: No. We *could* define it that way and it probably woulodn't break much. But we could also still allow highlights set on you to win, and just inherit from the element.
<TabAtkins> fantasai: but maybe that causes more problems
<Rossen_> ack schenney
<TabAtkins> schenney: Right, you'd have to say they apply to the element it's set on but not inherited... that causes issues. Simpler to just not allow it
<TabAtkins> fantasai: yeah
<TabAtkins> schenney: I think this is probably fine.
<TabAtkins> schenney: So do we do this for just ::selection or all the highlight pseudos?
<TabAtkins> fantasai: all, i think
<TabAtkins> schenney: yeah, for consistency
<TabAtkins> fantasai: And the same arguments apply for the other highlights too
<TabAtkins> schenney: I think there's not enough usage to really draw conclusions from on the other types yet
<TabAtkins> fantasai: Lea mentioned wanting to do syntax highlighting with highlight pseudos
<TabAtkins> fantasai: being able to use variables with them would be useful
<TabAtkins> schenney: syntax highlighting does seem to be the primary usecase for highlights, especially for perf, agreement
<TabAtkins> schenney: so proposed resolution: custom properties are disallowed in highlight pseudo-elements. they inherit from the originating element.
<dandclark> q+
<fantasai> fantasai: would that work for Lea's use case?
<fantasai> TabAtkins: yes, because she sets the theme colors on the originating elmeents
<TabAtkins> schenney: i think the only difference practically is if you change the custom prop on the originating element chain, in a way that's out-of-sync with how you change the highlight "pseudo-classes" on the chain
<TabAtkins> schenney: that's the only way to get into trouble
<TabAtkins> dandclark: i was thrown off by Lea's example, it seems you can make it work with either proposal
<TabAtkins> dandclark: I think our main concern was not breaking old code relying on the current browser behavior
<TabAtkins> schenney: I think that's right
<TabAtkins> schenney: My primary motivation is that people currently do a lot of custom properties on the elements and expect it to be visible to the selection. allowing that really increases the likelihood we can actually ship
<TabAtkins> dandclark: we are changing long-standing beahvior by changing the selection behavior at all, what's the bar for us to know if it's shippable?
<TabAtkins> dandclark: when we switched to inherit custom props from root we got some reports of real breakage, like form github
<TabAtkins> dandclark: It sounds like what i'm hearing is that it's not too bad to just inherit the custom props, perfwise
<TabAtkins> schenney: from a perf perspective, as long as we don't allow custom props to actually be set on the highlight itself, then from a code compelxity perspective it works fine
<TabAtkins> schenney: in trying to launch highlihgts in chromium, we've run into this issue, and people relying on the fact that they could change the selection for just one type of element and explicitly not ahve it inherited
<TabAtkins> schenney: But that second one has been fixed in the msot important sites affected by it, so i think with this change we'll have a good chance of shipping the fixes
<Rossen_> ack dandclark
<TabAtkins> fantasai: So the proposal is custom props don't apply to highlight pseudos. The var() function takes from the originating element
<TabAtkins> vmpstr: clarify on var() - that "takes from originating" is just for the highlight pseudos, not in general, right?
<TabAtkins> TabAtkins: yes
<TabAtkins> RESOLVED: custom properties don't apply to the highlight pseudos. On highlight pseudos, the var() function takes from the originating element.
LeaVerou commented 3 months ago

FWIW I ran into this today and damn, that’s some bizarre behavior if I've ever seen one.

This doesn’t work:

::highlight(mark-green) {
    --color: var(--color-green);
    background-color: color-mix(in oklab, canvas, var(--color));
}

This also doesn't work:

::highlight(mark-green) {
    --color: var(--color-green);
}

::highlight(mark-green) {
    background-color: color-mix(in oklab, canvas, var(--color));
}

This does work (--color-green is defined on :root):

::highlight(mark-green) {
    background-color: color-mix(in oklab, canvas, var(--color-green));
}

Can we please fix this? This is really bad lol.

schenney-chromium commented 3 months ago

Yeah, simple things seem like they should work ...

Start with the premise that to ever launch this we need to support the existing method, as best we can ascertain, of styling selection. That made use of custom properties to achieve a sensible highlight inheritance model, which is indeed the model we are trying to launch now. Authors would define custom properties on the elements, which would be inherited through the elements, and then use the properties in the selection. I have a write up at https://blogs.igalia.com/schenney/css-custom-properties-in-highlight-pseudos/

This existing method relies on highlights inheriting custom properties from their originating element.

To understand why we decided to ignore custom properties defined on the highlight itself, consider this.

<style>
  p { --color: blue; }
  p::highlight { --color: green; color: var(--color); }
  em { --color: red; }
  em::highlight { color: var(--color); }
</style>
<p>Some text <em>which color?</em></p>

Which --color should the em highlight use? One expects properties to inherit through the highlight chain, arguing for green. But we have made a very explicit web-compat decision to inherit custom properties from originating elements, arguing for red.

Managing the potential confusion was a goal, and also making implementations not-too-inefficient. The resolution is that the least confusing, most web-compat design is to inherit from originating and not allow custom properties in highlights.

This fits with a broader general principle for property values in highlight pseudos: When a value depends on information not available in the highlight (font size, container size, custom properties) the information is taken from the originating element.

LeaVerou commented 2 months ago

If we’re talking purely ergonomics, the answer seems simple: the em highlight uses red, unless some of it spans outside the <em> in which case that part only uses green, essentially treating ::highlight the same as a child encompassing the element content in terms of inheritance.

If that’s not possible to implement we can talk about how to reduce requirements from there to make it implementable, but I don't think disallowing custom properties from ::highlight altogether is a good compromise. I suspect folks had not realized the full implications of the Mar 7 resolution when they resolved that way. Ironically, I had sent regrets for that call saying:

I would really like to be there for 9909, but I also don't want to delay it further and end up with web compat forcing our hand. :/

Is it too late to revisit? If we can implement what I’m proposing in the first paragraph there should be no webcompat issue since custom properties are currently dropped entirely (and also since I don't think the CSS Highlight API is used much still).

schenney-chromium commented 2 months ago

I'll need to look into how an implementation of "use custom properties from the highlight itself but inherit others from the originating element" would work. If I recall correctly, the problem was at the point we need to copy properties from the originating element at which point we don't know which custom properties on the highlight was inherited or from the highlight itself.

The use case seems to be overriding the color on just one highlight easily when a site's general design is to use custom properties. But I don't know why you would do that in production when you could just replace the var on the one highlight with a specific color.

Anyways, I will look into achieving the proposed change.

bramus commented 2 months ago

Problem indeed was with all the properties needing to get copied over.

The updated resolution was made so that we would prevent from ending up with authors declaring all their custom properties on *::selection as well in order to make this work – a practice they now already do for generated content, i.e. *, *::after, *::before.