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

[css-highlight-api] Priority by number or some other method #4591

Closed
frivoal opened this issue Dec 13, 2019 · 3 comments · Fixed by #6136
Closed

[css-highlight-api] Priority by number or some other method #4591

frivoal opened this issue Dec 13, 2019 · 3 comments · Fixed by #6136

Comments

@frivoal
Copy link
Collaborator

frivoal commented Dec 13, 2019

Since multiple highlights can exist, we need to know which is on top of which (and then https://drafts.csswg.org/css-pseudo-4/#highlight-painting tells us how painting works). The current spec proposes doing that by assigning a number (and when the number is the same, by comparing the time of insertion into CSS.highlights).

This is simple to understand and to specify, but it's not clear to me that this is actually great to use. Should we drop priority by numbers system, and replace it with some other ordering mechanism? Experience with BASIC line number or z-index does not give much confidence that ordering by number is a good idea.

Is placing in an ordered data-structure instead of a map better? Should authors be able to express a desired to be placed above/below other named highlights, and let the UA figure it out? Something else?

@hober
Copy link
Member

hober commented Jan 22, 2020

Ideally, a priority system would:

  1. Be unnecessary by default (we have to spec how to order things when relative priority is not explicit)
  2. Be simple to author
  3. Would allow uncoordinated use of the Highlight API by multiple in-page libraries to not clobber each other
  4. Would not repeat the mistakes of z-index
  5. Would be straightforward to implement.

It may be that a z-index style number is the least bad of the possibilities, given the above. Sadface.

@sanketj
Copy link
Member

sanketj commented Mar 3, 2021

Below are some options that I recall discussing in the past:

(1)
Priority as an attribute on Highlight. This is what we have in the spec today. Usage would be:

<style>
  p::highlight(foo) {
    background-color:yellow;
  }
  p::highlight(bar) {
    background-color:orange;
  }
</style>
<body>Some text
<script>
  let textNode = document.body.firstChild;

  let r1 = new Range();
  r1.setStart(textNode, 0);
  r1.setEnd(textNode, 6);
  let h1 = new Highlight("foo", r1);

  let r2 = new Range();
  r2.setStart(textNode, 3);
  r2.setEnd(textNode, 9);
  let h2 = new Highlight("bar", r2);

  CSS.highlights.add(h1);
  CSS.highlights.add(h2);

  // If we paint now, h2 will paint above h1 since there is no explicit priority and h2 was registered later.
  
  h1.priority = 1;
  
  // Now h1 will paint above h2.
  
</script>

If two highlights have the same priority, the order in which they are registered in the CSS.highlights is used as a tiebreaker.

(2)
Priority as a CSS property. Though this feels more CSS-y and there is prior art for this with z-index, this may present a problem for highlight overlays. The css-pseudo-4 spec mentions that there is a single highlight overlay for the entire document per highlighting type. With priority as a CSS property, authors could write this:

#p1::highlight(foo) { priority: 1 }
#p1::highlight(bar) { priority: 2 }
#p2::highlight(foo) { priority: 2 }
#p2::highlight(bar) { priority: 1 }

If foo and bar overlap, foo will paint below bar in #p1, but above in #p2. This feels like a violation of the "single overlay for the entire document" concept.

(3)
Eliminate priority altogether, and just use the order of highlights in CSS.highlights. We would instead add methods like the ones below to HighlightRegistry, to allow reordering of highlights:

append([new highlight])
prepend([new highlight])
insertBefore([new highlight], [reference highlight])
insertAfter([new highlight], [reference highlight])
remove([existing highlight])

HighlightRegistry will no longer be setlike - it will need a custom implementation.

Amongst the three, I still prefer option 1 - what we have today - since it avoids the problems of option 2, and we can continue to take advantage of setlike for HighlightRegistry. Would be great to hear the thoughts of others.

@sanketj sanketj added the Agenda+ label Mar 3, 2021
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-highlight-api] Priority by number or some other method, and agreed to the following:

  • RESOLVED: leave custom highlight properties in the api as in spec
The full IRC log of that discussion <dael> Topic: [css-highlight-api] Priority by number or some other method
<dael> github: https://github.com//issues/4591
<emilio> vmpstr: yeah, flackr commented on the issue, should've figured that one out, d'oh :)
<dael> sanketj: Within these custom highlight overlays there's defualt order based on order registered. Highlights added later are painted on top
<dael> sanketj: Problem we're trying to solve is what if author want earlier registered on top
<dael> sanketj: Couple of options
<dael> sanketj: 1 is what we have in spec which is define priority property on highlight OM type. Number you can set >0 to give priority
<dael> sanketj: ex: If you have foo and bar highlights and bar registered later, bar paints on top. If you want foo on top set priority=1 and we would get that effect
<dael> sanketj: Another approach is not use priority but use highlight registery as source of true. Introduce additional APIs liek insertBefore which lets you change the order. Don't need additional concept, but means we can't re-use API surface. Seems like a lot of overhead
<dael> sanketj: Not super happy about that approach. I prefer what's in spec. Want other thoughts
<fantasai> +1 to not using CSS property
<dael> sanketj: On more aproach listed is similar to first but use a css property for priority. Don't think it works b/c it sets individual pseudos not for the overlay. Shouldn't be able to stack per pseudo element differently
<dael> florian: Agree. I don't think anybody loves similar to z-index but everything else seems worse
<dael> florian: Separate issue, if we go with number is it int or float and are negatives allowed
<dael> sanketj: Right. If we have time we can go in to that. Want to get resolution of should we allow a number at all
<dael> astearns: Side convo between fantasai and TabAtkins on IRC
<dael> fantasai: I just asked TabAtkins what he thinks. I'mnot super familiar.
<dael> fantasai: Question, between options 1 and 3 if there was some kind of standard interface for a lengthless type set what would your preference be?
<dael> sanketj: If we had something we could reuse?
<dael> fantasai: Yeah. One of hesitations is we would need custom impl. If that wasn't the case, what is preference?
<dael> sanketj: Highlight registry seems more about registration. Seems weird to manipulate that b/c then it's about control not jsut registering. I think I'd still prefer priority by number
<dael> florian: Agree. Having explicit manipulation would be clumnky. Maybe a little more powerful. but if you're developing all highlights you pick a number. If using libraries you'll be satisfied with neither direct numbers or direct manipulation
<dael> florian: I think the simpler thing is better
<dael> florian: Maybe you want some sort of constraints but we don't want to bake that into browser. That's on 3rd party library
<dael> fantasai: I think that order used as registered is tiebreaker is useful. Within library you can register back to back and they'll file together. With other method that's trickier
<dael> astearns: If you have 2 and concerned about way they overlap you register with numbers that have relationship you like. As others are registered that doesn't change order you register in
<dael> florian: You can register both at 733 in the right order and even if someone uses that number they won't be able to interjuct between
<dael> florian: I think there's been a bit of back and forth and no one loves numbers, but everything else is worse in some way
<dael> fantasai: Seems reasonable
<dael> astearns: Positive int or allowing float so you can fit inbetween
<dael> plinss: Going back for a second. If answer is this will not satisfy anybody why don't we do the manager?
<dael> florian: You'll need a manager if you haev multiple lib with multiple highlights. If you jsut write 3 yourself you give a number and you're fine
<dael> sanketj: Agree
<dael> fantasai: If the lib accept a param you can pass in a number unless you want to interleave. If you pass in to register at 6 that's straight forward.
<dael> plinss: Isn't that a common case? Isn't it common libraries won't bother to care about others that always use highlights
<dael> plinss: In order for multi lib to work they have to be aware of each other. I don't think that's common case
<dael> sanketj: Common to have spell check ext to have highlights, paging to have highlights. Uncoordinated use cases. For that there's no good solution. They could manipulate priority or the highlight registry. I don't think anyone is trying to solve that problem that a manager would do.
<dael> sanketj: Trying to provide a middle ground so uncoordinated access is not an issue and let authors manage the highlights they want
<dael> florian: Even if lib don't coordinate you can do it after they set. Only if they're trying to fight each other. You can fix it for them if they don't care
<dael> GameMaker: Cane we change priority after?
<dael> florian: It's an attribute you can set
<dael> GameMaker: Okay
<dael> plinss: Not the hill I want to die on, but concerned platoform has half solutions that require 3rd party library to work consistently. Don't want to add another
<dael> florian: I don't think you need a manager
<dael> astearns: Since plinss won't die on the hill, sounds like we have consensus to add to API as a priority
<dael> florian: Can we resolve on that? Sep issue for number
<dael> sanketj: There are. Not on agenda.
<dael> florian: They're separable
<dael> astearns: Prop: Keep what is in the spec for priority
<dael> astearns: Obj to leave custom highlight properties in the api as in spec
<dael> RESOLVED: leave custom highlight properties in the api as in spec
<dael> astearns: Lots of other items on agenda. Hash out number type in issues?
<dael> sanketj: Yes, we can do that

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

Successfully merging a pull request may close this issue.

4 participants