-
Notifications
You must be signed in to change notification settings - Fork 717
[css-highlight-api] Inline styles for Highlight should apply on top of styles provided by ::highlight #4588
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
Comments
This difference was deliberate when I drafted the document, but it's not something I'm completely sure about either, and did want to discuss. Here's the logic I had when I wrote this:
So my general idea was that if it is specific to overlays, it probably should work like the default for ::selection (and we still need to answer some of these open questions), but probably it should be replaced by something that works on all pseudo-elements, including, but not limited to ::highlight(). |
More specifically, I think the way we should solve this is by:
This would give us:
|
Drafted the above proposal in Pull Request form: #4607 |
Yes, I definitely see inline styles for custom highlights working similar to how they do for DOM elements. In that sense, I agree we should try to explore making this work generically for all pseudo elements. Currently though, the CSSPseudoElement interface only supports ::before, ::after and ::marker, which are all pseudos that do not inherit styles from other pseudos and also do not apply across multiple elements. To make CSSPseudoElement work with the highlight pseudos (and possibly ::first-line), I think we have to define a new event path. And if we're doing this, we should make sure we define it in a way that works with highlight events as well. (I left a comment about this in the PR.) |
@frivoal After thinking some more about this (particularly in regard the scenarios we discussed today), I'm starting to wonder if we really need to support inline styling of highlights as part of css-highlight-api-1? The .style property provides a nice convenience, but styling with ::highlight seems generally good enough. Additionally, in event scenarios, authors likely want to change the style for one range, in which case setting .style on an entire group isn't helpful. Instead, the author will probably add the range to a different highlight and style that. Pulling .style out would allow us to simplify the cascade interactions for this spec and we wouldn't need to depend on the outcomes of CSSPseudoElement. Additionally, once CSSPseudoElement does support .style and its scope expands to non-tree abiding pseudos, we would be able to inline style highlight pseudos in the same way as other pseudos. |
That makes sense to me. The #4607 Pull Request already greatly reduces the coupling, as after it, basically all that's left regarding style in the css-highlight-api spec is:
But indeed, the style attribute is a mere convenience, and it's not critical to the custom highlight api, so we could go further and drop it entirely from the spec, and just wait on css-pseudo defining the OM-based style attribute. What would you rather do? |
I'd prefer to remove .style entirely for now because I think there's more work needed before CSSPseudoElement can be used with non-tree abiding pseudo elements, even just for styling. To be able to apply an 'inline' style to a highlight with |
Uh oh!
There was an error while loading. Please reload this page.
This section (https://drafts.csswg.org/css-highlight-api-1/#c-and-h) of the highlight API spec mentions that "the User Agent must use the styles provided in the style attribute of the corresponding highlight when no applicable property has been specified via ::highlight()". This seems to imply that if a ::highlight has been specified, then the ::highlight styles should "win" over those coming from the .style attribute. What was initially proposed in the highlight API explainer (https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/highlight/explainer.md#application-of-css-properties) was that "the Highlight object's 'inline' style (i.e. properties set directly on the .style member) will be applied on top of the cascaded values for the given highlight identifier", which is the reverse of what we currently have spec'ed.
Not sure if this was a deliberate change but I think it is important that styles set via the .style attribute apply on top of ::highlight styles. This is a useful convenience for authors to be able to override the highlight styles from script. I can see it being especially useful when combined with highlight events (https://github.com/MicrosoftEdge/MSEdgeExplainers/blob/master/highlight/events-explainer.md). For example, if the author wants to change the background color of a highlight when it is clicked, it is convenient to attach a click event listener to the highlight and then set the .style attribute in the callback.
cc: @frivoal @BoCupp-Microsoft @gked
The text was updated successfully, but these errors were encountered: