w3c / csswg-drafts

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

[css-highlight-api] Approaches for dispatching highlight pointer events #7513

Open dandclark opened 2 years ago

dandclark commented 2 years ago

7512 discusses whether a new pointer event type should be added for use with Highlights. A related task is to develop the details about how pointer events should be dispatched to highlights.

It's important to consider what should happen if there are multiple overlapping highlights. Consider for example an in-page editor implementing find-on-page, where a spell-checking extension is also running. Both might have actions they want to take when the user clicks a highlighted range, for example the find-on-page might want to change the color of the clicked word and update the selection, and the spell-checker might want to show a popup with spelling suggestions. When the spell checker decides to show its popup, it might be reasonable to allow it to block actions that another highlight type might take (like updating find-on-page highlight colors or selection).

With that in mind, there are a few potential approaches for what to do when a user interacts with (potentially overlapping) highlighted content.

Approach A: Dispatch a separate event against each Highlight under the cursor, in descending priority order.

Additionally, we could define the default action of each event to be the generation of the event for the next Highlight. This way, a Highlight's event handler can call preventDefault and prevent events from reaching subsequent Highlights.

It is a bit suspicious to potentially fire an arbitrary number of pointer events for a single user interaction, but this might be fine.

Approach B: Dispatch a single event against the top-priority highlight, whose event path includes any overlapping highlights in descending priority order, followed by the containing element and its parents.

There's a web compatibility problem with this one. Currently a PointerEvent's target can only be an element, so PointerEvent handlers will probably expect this and encounter problems if they receive an event whose target is a Highlight.

The MSEdgeExplainer proposes a variation of Approach B that works around this by setting the event's target to the element rather than the Highlight, even though the event path is ordered as if the Highlight is the target. But as @smaug---- points out at https://github.com/MicrosoftEdge/MSEdgeExplainers/issues/588, this raises its own questions about how eventPath would work.

I'm also interested in a potential combination of Approach A and Approach B, where we dispatch exactly two events per user action. Firstly, dispatch an event against the top-priority highlight such that the event path includes any overlapping highlights in descending priority order. Secondly, the "normal" pointer event is dispatched against the originating element. In this case we could consider defining preventDefault on the highlight-dispatched event to prevent the subsequent dispatch of the "normal" pointer event. This approach seems to avoid a lot of the difficulties around Approach A and Approach B separately, and allows a great deal of flexibility in how Highlights can prevent propagation to lower-priority highlights and to the page content.

cc @frivoal @luisjuansp

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed dispatching highlight events.

The full IRC log of that discussion <TabAtkins> Topic: dispatching highlight events
<TabAtkins> github: https://github.com/w3c/csswg-drafts/issues/7513
<TabAtkins> dandclark: so pointer events with highlights, this is about making highlight pseudos interactive like for a spellchecker
<TabAtkins> dandclark: first q is how do we dispatch these events, what's the event order
<TabAtkins> dandclark: first i assume the highlight object is the right thing to receive the event, rather than the individual ranges
<TabAtkins> dandclark: If i have a bunch of ranges on a single highlight, i probably want to do the same thing for each
<TabAtkins> dandclark: seems silly to have to set an event listener on each range, especially as the highlight moves
<TabAtkins> dandclark: and when considering dispatches, need to consider there can be multiple types of highlights, and possibly overlapping
<TabAtkins> dandclark: like a find-on-page that emphasizes and scrolls, and also a spellcheck that underlines misspelled words and shows spelling suggestions
<TabAtkins> dandclark: might want one of those to eat the click, so spelling error can win over find-on-page
<TabAtkins> dandclark: so i can talk about a fe wapproaches
<TabAtkins> dandclark: in the issue, version A is each highlight gets its own event
<TabAtkins> dandclark: in descending priority order
<TabAtkins> dandclark: they can be cancelable, so the default is to advance to the next highlight, and you can preventDefault()
<TabAtkins> dandclark: suspicious to fire an arbitrary number of events for a single action tho
<TabAtkins> dandclark: approach B is a single event for the top priority highlight, define event path so it goes thru other highlights, then bubbles to dom tree
<TabAtkins> dandclark: this seems like pretty normal appraoch. but there's a web compat problem
<emilio> q+
<TabAtkins> dandclark: currently pointer events can only ever be an element
<TabAtkins> dandclark: but now suddenly they can have non-element targets
<TabAtkins> dandclark: so might break assumptions
<TabAtkins> dandclark: the Edge explainer suggests lying about the event target to get around this
<TabAtkins> dandclark: thinking abou tit more, think it's not the right approach
<TabAtkins> dandclark: Approach C is a combo. we dispatch two events, one for highlights, one for dom tree. highlight one propagates thru overlapping highlights. if it's not preventDefault'd by the end, we fire a new one on the dom tree
<TabAtkins> dandclark: that dual-event appraoch is where I lean now
<flackr> q+
<TabAtkins> q+
<astearns> ack emilio
<astearns> q+
<TabAtkins> emilio: rather than messing witht he dom events and retargeting, can we add an api to highlight that tells "is point X intersecting the highlight?"? seems like that would let you do what you want with regular dom events
<TabAtkins> emilio: you handle the pointer event and ask the highlights if the point overlaps, and just deal with normal dom event
<TabAtkins> emilio: Seems simpler but still gives all the use-cases
<TabAtkins> emilio: If you ahve two objects, handling pointer event on one or the other...
<TabAtkins> emilio: the normal element pointer event should still work even if you click on the highlight
<TabAtkins> emilio: seems simpler and less risky, and doesn't introduce new event model
<TabAtkins> emilio: and will work with touch events, etc
<TabAtkins> emilio: curious if it's been considered?
<dandclark> q+
<TabAtkins> TabAtkins: so assumption is you'd iterate thru all your highlights and check?
<TabAtkins> emilio: Yes. and they can check if it's been defaulted, you can deal with the order yourself, etc.
<TabAtkins> dandclark: this seems useful for- given i have a click with an offset, show me all the highlights under this location? or go thru each highlight in the registry and check?
<emilio> boolean intersects(long clientX, long clientY)
<bkardell_> emilio: that is on highlight?
<TabAtkins> dandclark: Can def see if there's a pointer even thandler on the body, and i get an event, could ask platform for all the highlights at this point
<TabAtkins> emilio: was thinking about method on the highlight object
<TabAtkins> emilio: so you iterate over the highlights i nthe order you want
<TabAtkins> emilio: but that should provide all the flexibility you'll ever need
<TabAtkins> emilio: If we want something more convenient, could put one on the highlight registry that gives back all of them
<TabAtkins> emilio: would just be sugar over a for loop
<TabAtkins> emilio: so could use if you want a central place to handle all the highlights
<TabAtkins> emilio: but might also want to deal with each highlight separately
<astearns> ack flackr
<TabAtkins> flackr: My point was partially overlapped with emilio, concerned about a special event befor ethe dom event
<TabAtkins> flackr: could consider the highlight to be the normal action, so if the normal event wasn't defaulted you dispatch to highlight
<TabAtkins> flackr: longer term, we want to support css pseudo targets, wondering if that helps so we can just target those
<TabAtkins> dandclark: the highlight pseudo is a pseudo object, it's a little special...
<TabAtkins> flackr: i think my main concern is whether those should be given the event first
<TabAtkins> flackr: I think visually the highlight is behind the text, so from dev pov the element should get first crack, and pseudo gets opportunity after
<astearns> ack dbaron
<astearns> ack astearns
<TabAtkins> q-
<TabAtkins> dbaron: i think it's worth trying to get event.stopPropagation vs preventDefault to behave as closely as possible to normal
<TabAtkins> dbaron: I think what dan was originally defining was to make pD act like SP
<TabAtkins> dbaron: Hope we can do something better even if the dom event spec needs to expose some extra concepts
<TabAtkins> dbaron: thoughts on emilio's proposal
<TabAtkins> dbaron: first is i'm a little concerned a bout an appraoch tha tforces you to add event listeners to the doc, for cases where impls want to make optimizations for passive listeners
<TabAtkins> dbaron: if it's an event type where registering a non-passive listener, and you need to register non-passive for the highlight, you have to take the perf penalty document-wide
<TabAtkins> dbaron: that seems undesirable
<emilio> q+
<TabAtkins> dbaron: so i think that's a reason you might want to have EL registration be on more specific targets
<TabAtkins> dandclark: I like the simplicity of emilio's idea.
<TabAtkins> dandclark: if we can find a way around the perf hit
<astearns> ack dandclark
<TabAtkins> emilio: the initial proposal was about clicks
<astearns> ack emilio
<TabAtkins> emilio: don't think we can get much opt about passive vs non-passive event listeners
<TabAtkins> emilio: don't see how having highlights doesn't prevent you from losing optimizations
<TabAtkins> dbaron: what you said about the way to use the proposal is put an El on the document, and look up the point
<TabAtkins> dbaron: And that way of using it can be problematic
<TabAtkins> emilio: right, if you care about scroll and things that benefit from passive
<TabAtkins> emilio: But API doesn't let you use it for other event types
<TabAtkins> dbaron: I think when it's like "this is only for clicks *right now*", unsure we want to design around that
<TabAtkins> emilio: So API right now takes an x/y
<TabAtkins> emilio: Have clear text for mouse events to resolve that
<florian> q?
<TabAtkins> emilio: could let the method take an event object instead, so we can limit it to certain event types
<astearns> ack dbaron
<Zakim> dbaron, you wanted to comment on what inputs emilio's API needs
<TabAtkins> dbaron: ah yeah other point id' forgot. i think taking x/y doesn't handle [missed]
<TabAtkins> dbaron: you might click a piece of text with highlights that obscures another piece with highlights
<TabAtkins> dbaron: you're probably interested in the top element's highlihts, not the obscured one
<flackr> qq+
<TabAtkins> emilio: could define the api similar to getelementfrompoint where you do account for hit-testing
<TabAtkins> emilio: could define to only care about rects, like union the ranges
<florian> q+
<TabAtkins> emilio: or like elementFromPoint which cares about hit-testing and p-e:none/etc
<TabAtkins> emilio: Seems you could get that with just an x/y if you define it to do hit-testing
<TabAtkins> dbaron: yeah that sounds like it works
<florian> q-
<TabAtkins> emilio: So define it in terms of elementsFromPoint
<TabAtkins> emilio: If any of the elements intersect the highlight... something like that
<TabAtkins> emilio: specific semantics for intersecting highlights could be left to...
<astearns> q+
<TabAtkins> emilio: if both intersect the consumer can choose
<astearns> ack flackr
<Zakim> flackr, you wanted to react to dbaron
<TabAtkins> flackr: I think x/y works if you can get highlights on the targeted element
<TabAtkins> flackr: that'll account for hit-testing
<TabAtkins> flackr: but this is def going to be a perf issue, even if limited to mouse events
<TabAtkins> flackr: people might want to drag highlights, so they'll have pointerstart, and that means we have to delay scrolling
<TabAtkins> flackr: but if we fire on the highlight we can recognize it won't stop scrolling
<TabAtkins> emilio: but that would penalize other scenarios
<TabAtkins> emilio: not sure what the best way to do it is
<TabAtkins> emilio: needs real thought especially for pointerdown
<florian> q+
<TabAtkins> emilio: [missed due to truck outside]
<TabAtkins> emilio: there are use-cases for pointerdown
<TabAtkins> emilio: if we extend proposal, we'd dispatch to highlights, then to web content... we'd do it for every event type ayone would care about
<TabAtkins> flackr: highlight needs to be in the event target tree, but yeah currently tree is strictly ancestor tree so that would be a disconnect
<TabAtkins> astearns: q about emilio's proposal
<TabAtkins> astearns: you get coords, ask document fo rhighlights.
<TabAtkins> astearns: are we still able to have top priority highlight prevent other highlights from responding?
<TabAtkins> emilio: Yes, just check if the event was preventDefault'd and return rather than moving on to the next highlight
<dbaron> (do you mean stopPropagation or preventDefault for that case?)
<TabAtkins> (don't think it matters for the discussion)
<TabAtkins> astearns: So it's manual?
<TabAtkins> TabAtkins: Yes, all fully manually handled, they're not fired on the highlights by the browser stuff
<TabAtkins> fremy: So if you have multiple types of code doing highlights they don't know about each other
<TabAtkins> fremy: How can they work together/
<TabAtkins> emilio: Initial proposal was a bool on the highlight
<TabAtkins> emilio: If you need to be aware of one vs the other you need a central place to do it
<TabAtkins> TabAtkins: Right, given dan's initial use-cases (scroll and spellcheck) you *need* them to coordinate with each other anyway
<TabAtkins> emilio: with my proposal there's no change to event prop. you fire the same vents as with no highlights
<TabAtkins> emilio: Up to the website to either centralize their highlight handling, or have different listeners that handle them separately
<TabAtkins> fremy: but you can't decide which acts first
<TabAtkins> emilio: with a bool and two coords appraoch, that's right bc you have to handle the highlights
<dandclark> q+
<TabAtkins> emilio: But if we have the method on the highlight registry that returns highlights in order, you can check yourself if the one you're in charge of is first
<astearns> ack astearns
<TabAtkins> fremy: So if you're not, do you try again?
<TabAtkins> emilio: If you don't have event listeners in the right order you can have weirdness
<TabAtkins> fremy: But order isn't one order
<TabAtkins> fremy: Not sure there's one correct order
<TabAtkins> dbaron: i think emilio's proposal is that page author has to write their own event registraation system if they want to deal with multiple highlights
<TabAtkins> dbaron: we provide an api in the platform that says "here's the ordered list of highlights affected by this" and the author can process the list with their own reg system however they want
<TabAtkins> fremy: Yeah, just weird we don't provide that
<TabAtkins> fremy: you assume libraries will cooperate, not clear to me they will if they're different sources
<fantasai> +1 fremy
<TabAtkins> fremy: strange to assume that
<TabAtkins> emilio: my proposal is minimum that lets you build this
<GameMaker_> +1 fremy as well
<TabAtkins> emilio: but solving general case firing event listenres in an order that solves every use case, yeah it's a much more complicated problem
<TabAtkins> emilio: have to define which events do this, we start with click but have a million events
<dbaron> s/registraation/registration/
<TabAtkins> fremy: another option is we provide it. for each highlight, define which events you want to recieve, and we have an api that dispatches to them in a pseudo-event manner
<TabAtkins> fremy: So does preventDefault for you, etc
<TabAtkins> emilio: I'm not even sure there's a canon order you want to define events on
<TabAtkins> emilio: spellcheck error with a link inside
<TabAtkins> emilio: might seem reasonable to fire spellcheck first and link if not handled, but might be reasonable other way around
<TabAtkins> emilio: I think easiest is to make website choose
<TabAtkins> qq+
<TabAtkins> dandclark: advantage of having platform handle this, where we just propagate the event, is we already had an order of highlights
<TabAtkins> q-
<TabAtkins> dandclark: so think there is an advantage to use the existing order since things are built in already
<TabAtkins> emilio: there is reasons to do before, but also after
<TabAtkins> emilio: not clear to me why we should do them before or after in specific
<TabAtkins> fremy: i think this makes sense, let authors to do this
<TabAtkins> qq+
<TabAtkins> emilio: I think having an api that gives you ranges in order would be low level
<TabAtkins> emilio: if libraries say we can't use this because there's no good way to coordinate, then we can decide whether to fire events before or after
<bkardell_> q+
<TabAtkins> emilio: seems like an elegant way to handle the use-cases without making a call
<astearns> ack TabAtkins
<Zakim> TabAtkins, you wanted to react to astearns
<fantasai> TabAtkins: I think one detail of fremy's proposal is being missed, which is getting the list ordered is reasonable and authors handling on his own
<fantasai> TabAtkins: the problem is that you have to re-invent event dispatch, and hope other libraries can coordinate with you
<fantasai> TabAtkins: Given a list of highlights to displatch on, it fires events using normal event listener mchanics
<astearns> (isn’t this the first option?)
<fantasai> TabAtkins: handles preventDefault etc, as if part of the normal event dispatch order
<fantasai> TabAtkins: so don't have to re-invent
<fantasai> TabAtkins: doesn't require cross-library coordination of the events themselves
<fantasai> TabAtkins: e.g. fire highlight event, take a list of highlights, cranks through them in order as if part of eveent chain
<TabAtkins> emilio: so would be equivalen to making highlight even tlistener
<fantasai> emilio: it would be equivlanet to [..]
<TabAtkins> emilio: so basically it woud be a matter of writing that loop with the current event and dealing the pD/etc in that order
<fantasai> emilio: with api from before, give me a list of highlights, would be a matter of writing that loop with current event and dealing with event propagation and stuff manually
<fantasai> emilio: that seems reasoanble
<fantasai> emilio: page would be responsible at some point, to propagate [missed]
<bkardell_> q-
<fantasai> TabAtkins: on the hihglight registry, eyah
<fantasai> emilio: seems like eihter extension or replacement
<TabAtkins> s/[missed]/the highlight events/
<fantasai> emilio: still requires page's coordination
<fantasai> astearns: Tab described your first option from the issue?
<fantasai> dandclark: sounds similar, except instead of dispatching, need to register to dispatch
<fantasai> TabAtkins: Yes, page author still has to handle ordering and do what they need to do
<fantasai> TabAtkins: but otherwise platform handles events for you
<fantasai> dandclark: so if I have highlights on my page
<fantasai> TabAtkins: Ask the highlight registry are at this x,y, then call this with all of those
<fantasai> TabAtkins: in priority order
<fantasai> TabAtkins: if a highlight preventsDefault, then later things don't get the event
<fantasai> TabAtkins: which is what you want
<fantasai> TabAtkins: [missed]
<fantasai> dandclark: How do we know it's not happening twice?
<fantasai> dandclark: if my final page library says, okay, I need to trigger this ? and propagate the highlights
<fantasai> dandclark: [too fast]
<fantasai> dandclark: I have too different libraries
<fantasai> s/too/two/
<fantasai> TabAtkins: if you have 2 libraries both installing event handler on the element
<fantasai> TabAtkins: if not preventDefault, each one would run thorugh list, and you'd get repetitious behavior
<fantasai> florian: This approach both allows but also requires you to coordinate
<fantasai> florian: simple scenarios can coordinate by accident, but overall, if doing with multiple independent libraries
<fantasai> florian: need some kind of plan for interaction
<fantasai> florian: you need to think through it
<astearns> ack florian
<fantasai> florian: another thing I wanted to ask, unsure if decides,
<dandclark> q-
<fantasai> florian: all this discussion here, we're going to have a very similar discussion for every other type of pseudo
<fantasai> florian: e.g. ellipsis
<fantasai> florian: we shouldn't have 3-4 different solutions for each type of pseudo
<fantasai> florian: I think this approach works better, you're in charge of coordinating, coordinate to your heart's delight
<fantasai> florian: but it is a low-level approach, you *have to* coordinate
<fantasai> florian: For those who have thought about this, how do you generalize to other pseudos?
<fantasai> TabAtkins: Only concern I have is the actual invocation of your behavior, already have API for that called event listeners
<fantasai> TabAtkins: not using that sucks
<fantasai> TabAtkins: but if we recommend to authors to call dispatchEvent on your things, that's probably sufficient
<fantasai> TabAtkins: we have communication API ... but at least not inventing a new callback system
<fantasai> florian: can we do both? By default have a dispatcher that [..]
<fantasai> florian: and if you don't like that, do your own?
<fantasai> florian: If you don't say anything, we dispatch like this. I fyou don't like it, roll your own
<fantasai> fremy: If you want [missed]
<fantasai> fremy: we already have mech or that
<fantasai> emilio: but initial proposal fires at DOM
<fantasai> fremy: if you decide that you want to dispatch something, if it's imporant,, you can say it's ?? phase. We already have mechanism for it, this coordiation happens every day
<fantasai> fremy: we have ? phase, capturing, etc.
<fantasai> fremy: we already have these
<fantasai> fremy: it works
<fantasai> emilio: If the page hasn't called dispatchEvent on the registry
<fantasai> emilio: if we do ourselves, prevents page from doing it later
<fantasai> emilio: so only deault we could choose is after DOM dispatch
<fantasai> emilio: but might want to dispatch ???
<fantasai> emilio: so I think making highlight inherit from EventTarget makes sense
<fantasai> emilio: and propagating to highlight whenever page wants is reasonable
<fantasai> emilio: but how would that look like?
<fantasai> emilio: undefined ?? x,?
<fantasai> emilio: do for mouse events normally?
<fantasai> emilio: do the list thing?
<fantasai> TabAtkins: do the list thing manually
<fantasai> TabAtkins: you can check prevetnDefault by defaultPrevented
<fantasai> TabAtkins: we can put example code in the spec
<fantasai> emilio: So you don't want an API that does it for you?
<fantasai> TabAtkins: probably not
<fantasai> dandclark: If we have 2 libraries that are not coordinating, which one writes the for loop?
<fantasai> TabAtkins: You should check to see if default has been prevented
<fantasai> TabAtkins: if you're first, [...]
<fantasai> TabAtkins: or you can just stop propagation immediately
<fantasai> TabAtkins: event system can handle that
<fantasai> fremy: You may say that you don't want to prevetnt, but still click on the button
<fantasai> TabAtkins: Events should check themselves to see if prevented
<fantasai> fremy: if you preventDefault, the button won't click
<fantasai> emilio: That's the key of the issue, there's no good answer on whatever is the good ansewr
<fantasai> emilio: if you have a spelling eerror on a button, you might want a popup to suggest a fix
<fantasai> emilio: or you might want to click the button
<fantasai> emilio: It's not known which you want
<fantasai> fremy: I agree, but if every library makes its own for loop because it's necessary, then ??
<fantasai> emilio: we're not suggesting that
<fantasai> emilio: The library doesn't need to write the for loop. They add e.g. click event listener on its optject
<fantasai> emilio: something on the page writes the for loop once, and dispatches to all highlights
<fantasai> emilio: so no library needs to write the for loop
<fantasai> emilio: that's done once whenever page wants to handle that
<fantasai> fremy: If I use grammarly, grammarly needs to write the for loop or else it doesn't work
<fantasai> fremy: so they will write it because they need it
<fantasai> emilio: then they would only handle ranges that they register
<fantasai> fremy: but it still does not solve issue of order
<fantasai> fremy: run the grammarly thing and no consideration that some other things on top of them
<fantasai> emilio: if preventDefaulted then ???
<fantasai> fremy: Sitll the same issue, it's not preventDefaulted if there's another highlight before it
<fantasai> fremy: you have multiple highlights. Grammarly wants to work. So they will need to add an event to handle fix. If they want to make for loop, they can only include grammarly stuff
<fantasai> fremy: but they might want to run grammarly code only if no highlight on top of them
<fantasai> emilio: if the page has dispatched the same event to the highlight already
<fantasai> emilio: like you would dispatch the mouse event from the platform
<fantasai> emilio: if another highlight provider did something, registered dom event in capturephase, and have preventDefaulted it, then grammarly can check that and bail out
<fantasai> emilio: this event has been handled before, we may not even need our for loop
<fantasai> fremy: I don't quite understand, it's not clear what are we doing
<fantasai> TabAtkins: There are various ways for event handler to check if I've been called with this before
<fantasai> TabAtkins: In most cases not required, all first-party content, and don't care much
<fantasai> TabAtkins: should be generally OK, and complex cases need compexity anyway
<fantasai> TabAtkins: you can say I only want to be dispatched once, and guarantee that on your own, by checking and immediatley returning if you get called a second time
<fantasai> fremy: ...
<fantasai> TabAtkins: normal event dispatch would not call the same event on the same element more than once
<fantasai> TabAtkins: but we're not using normal event dispatch
<fantasai> astearns: any other lingering questions, or take back to the issue to discuss the current state of this fourth way
<TabAtkins> (store the last event you saw dispatched to yourself, and check if the new one is the same as the last; if so return immediately)
<fantasai> dandclark: I agree with taking back to the issue, I'm also a bit lost
<TabAtkins> so you can avoid the "double-dispatch" entirely on your own without any 3rd party coordination
<fantasai> astearns: Emilio, can I ask you to write up the proposal with all the details so that Dan has something to digest more easily than the minutes?
<fantasai> dandclark: Does this still have the perf issue dbaron raised?
<fantasai> dandclark: [missed]
<fantasai> emilio: for clicks sure there's no perf issue
<fantasai> emilio: for things like pointerdown, if you want notPassive handling of pointerDown, it feels like a global event listener is maybe not ideal
<fantasai> emilio: this may be fixable another way
<fantasai> emilio: like, maybe we need another member of listenerOptions
<fantasai> emilio: I don't have an answer for that right now
<fantasai> flackr: Maybe the highlight could have a different touchAction?
<fantasai> flackr: normally we encourage ppl to use passive events and use touch Action to specifiy whether to ???
<fantasai> flackr: if we had a way to specify the touch Action on the highlights to not depend
<fantasai> emilio: yes, you might want to allow touch Action on highlight pseudos, make that behave properly
<fantasai> emilio: it's a very good idea
<emilio> s/touch Action/touch-action
<fantasai> astearns: Anything more on this issue?
<fantasai> dandclark: Yes, I think we're good
<fantasai> astearns: do we discuss next issue? or wait on the details of this one?
<fantasai> dandclark: I think we wait
<flackr> s/???/to scroll or not
<fantasai> dandclark: next one is about answering question of given dispatch, how to we determine which range was targetted. I suggested adding a new event type
<fantasai> astearns: so closing off current issue and going to next issue
emilio commented 2 years ago

So something like we were mentioning was:

Make Highlight inherit from EventTarget

So that we can dispatch events on them.

Add a hit-testing API to the registry

Something like:

partial interface HighlightRegistry {
  // For parallelism with elementsFromPoint.
  // Returned values in (z-order, priority) order.
  sequence<Highlight> highlightsFromPoint(long x, long y);
};

The idea is that the page could, either before or after handling DOM events, do something like:

document.addEventListener("click", function(e) {
  if (e.defaultPrevented)
    return;
  for (let highlight of CSS.highlights.highlightsFromPoint(e.clientX, e.clientY)) {
    highlight.dispatchEvent(e);
    if (e.defaultPrevented)
      return;
  }
});

Passive event listeners / touch-action

@dbaron / @flackr pointed out that encouraging document-level event listeners may be problematic (because having something like, e.g, a non-passive pointerdown event-listener might cause performance issues with scrolling).

@flackr proposed to support touch-action: none on highlights, so that you can make those event listeners passive as needed.

Knowing specific ranges that are hit.

Per #7512, there are use cases to know the specific ranges that are hit. Which means that maybe the API should do something a bit more complex:

dictionary HighlightHitTestResult {
  Highlight highlight;
  sequence<AbstrangeRange> ranges;
}
partial interface HighlightRegistry {
  // For parallelism with elementsFromPoint.
  // Returned values in (z-order, priority) order.
  sequence<HighlightHitTestResult> highlightsFromPoint(long x, long y);
};

Or so. We could then encourage to do something like putting the highlights in the expando, something like:

document.addEventListener("click", function(e) {
  if (e.defaultPrevented)
    return;
  for (let { highlight, ranges } of CSS.highlights.highlightsFromPoint(e.clientX, e.clientY)) {
    e.ranges = ranges;
    highlight.dispatchEvent(e);
    if (e.defaultPrevented)
      break;
  }
  delete e.ranges;
});

But that feels rather clunky... Not sure I have a better proposal atm tho.

smaug---- commented 2 years ago

That example doesn't work. One can't dispatch an event which is already being dispatched.

dandclark commented 2 years ago

That example doesn't work. One can't dispatch an event which is already being dispatched.

Hmm, that certainly poses a problem. Unless we'd be willing to tell devs to do setTimeout(() => { e.ranges = ranges; highlight.dispatchEvent(e); }, 0) in the Document's event handler, I guess we'd need to add another API to hand off the event to Highlights:

partial interface Highlight {
  // Once the event has finished dispatching, dispatch it again against this Highlight
  // with event.ranges set to ranges.
  void handleEventAsync(Event event, sequence<AbstrangeRange> ranges);
}

This would be called in the for loop of the Document's event handler, instead of calling dispatchEvent directly. Setting e.ranges during the initial for loop no longer works since this is asynchronous, hence the extra ranges parameter.

Is there a better way to work around this problem? This is becoming sufficiently cumbersome that I'm thinking this approach's initial goal of simplicity is no longer being achieved.

dandclark commented 2 years ago

I noticed another problem in the case where the page has two or more highlighting libraries that don't coordinate with each other. In this scenario, both libraries independently set this event listener in order to ensure that events get routed to their highlights:

document.addEventListener("click", function(e) {
  if (e.defaultPrevented)
    return;
  for (let { highlight, ranges } of CSS.highlights.highlightsFromPoint(e.clientX, e.clientY)) {
    e.ranges = ranges;
    highlight.dispatchEvent(e);
    if (e.defaultPrevented)
      break;
  }
  delete e.ranges;
});

If the Highlight event handlers for both libraries call preventDefault, then the defaultPrevented checking means we won't repeat the events. But if neither highlighting library wants to preventDefault, then each Highlight will get the event twice for each user interactions: once from each Document "click" handler set by each respective highlighting library.

It seems hard to get this right when there are multiple non-coordinating highlight libraries. I'd like to reconsider whether an approach that provides platform support closer to a traditional event path makes sense here:

[when the user performs a pointer interaction over a highlight range], dispatch an event against the top-priority intersected highlight such that the event path includes any overlapping highlights in descending priority order. Secondly, the "normal" pointer event is dispatched against the originating element. In this case we could consider defining preventDefault on the highlight-dispatched event to prevent the subsequent dispatch of the "normal" pointer event.

smaug---- commented 2 years ago

So highlight would get something like highlightpointerdown/up events? and if one called preventDefault() on those, pointerdown/up (and possible mousedown/up, touchdown/up) events wouldn't fire?

Another thing to remember, if highlights can be accessed from the event, is that we'd need to tweak event state when events pass shadow DOM boundary, or stop propagation at shadow DOM boundary. Highlights in shadow DOM shouldn't be exposed to light DOM. Perhaps the events just wouldn't be composed.

To me CSS.highlights.highlightsFromPoint() smells like the minimum viable API. Highlights wouldn't need to be event targets. If we later figure out a good way to dispatch events to Highlights themselves, that could be added separately.

dandclark commented 2 years ago

So highlight would get something like highlightpointerdown/up events? and if one called preventDefault() on those, pointerdown/up (and possible mousedown/up, touchdown/up) events wouldn't fire?

Basically. It wouldn't need to be new event name though (like highlightpointerdown/up); it could just be a separate instance of pointerdown/pointerup that gets delivered to highlights before the pointerdown/up that gets delivered to DOM elements.

Another thing to remember, if highlights can be accessed from the event, is that we'd need to tweak event state when events pass shadow DOM boundary, or stop propagation at shadow DOM boundary. Highlights in shadow DOM shouldn't be exposed to light DOM. Perhaps the events just wouldn't be composed.

This is a good point. We also need to think this through for CSS.highlights.highlightsFromPoint(), though. Would it be a problem if that API returned highlights from inside closed shadow DOMs? In a way that would break shadow DOM encapsulation, but it's not necessarily a new issue since those ranges are available via CSS.highlights.get() anyway. So shadow DOM encapsulation is basically already broken as long as there are active Highlights in the shadow.

Currently document.elementsFromPoint excludes content inside shadows, so for consistency's sake it would seem appropriate that highlightsFromPoint do the same. That would make the API useless in a scenario with shadow DOM though -- and in that case it might be better to go straight to a highlight eventing approach that would support shadow DOM properly.

To me CSS.highlights.highlightsFromPoint() smells like the minimum viable API. Highlights wouldn't need to be event targets. If we later figure out a good way to dispatch events to Highlights themselves, that could be added separately.

I'm inclined to agree with this, but only if we can work around the potential performance issues and the shadow DOM questions above, and if the lack of ability to coordinate between different highlight libraries doesn't turn out to be a hard requirement.

dandclark commented 2 years ago

I can think of two ways to expose shadow DOM highlights with highlightsFromPoint().

The first is to handle it like getInnerHTML() does for Declarative Shadow DOM, in which it takes an includeShadowRoots option as well as a list of closed shadow roots so that highlights inside these can be returned without breaking encapsulation: https://web.dev/declarative-shadow-dom/#serialization. Highlights could do something similar:

const hitTestResults = CSS.highlights.highlightsFromPoint({
  includeShadowRoots: true,
  closedRoots: [shadowRoot1, shadowRoot2, ...]
});

Alternatively, we could put highlightsFromPoint() on DocumentOrShadowRoot. When called on a Document, it would return only highlights in that document (not in its shadow roots). When called on a shadow root, it would return only highlights in that shadow, or perhaps highlights in that shadow plus highlights in the document (but not highlights in any nested shadow roots). This is analogous to how elementsFromPoint() works in Blink and Gecko today, though the spec doesn't yet reflect that elementsFromPoint() is callable on shadow DOM and I see some cross-browser inconsistencies in which elements are returned.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed Approaches for dispatching highlight pointer events, and agreed to the following:

The full IRC log of that discussion <heycam> Topic: Approaches for dispatching highlight pointer events
<heycam> dandclark: problem we're trying to solve is making custom highlights interactives
<heycam> ... for scenarios like custom spell check, annotation popups over words
<heycam> ... a few months ago I came to the WG with a proposal for adding eventing for highlights
<heycam> ... when tehre's a pointer event, a highlight, we get a first crack at it
<heycam> ... dispatch events at highlights in priority order
<heycam> ... if none preventDefault() it, we give it to the normal DOM element
<florian> github: https://github.com/w3c/csswg-drafts/issues/7513
<heycam> ... goal here would be to allow non-coordinating highlight libraries to do reasonable things
<Rossen_> q?
<heycam> ... if you have say a spell check library, and a annotation library running, stopping propagation of an event would avoid two popups at once
<heycam> ... normal event propagation would allow this coordination without the 2 librariers knowing about each other
<heycam> ... however this is somewhat complicated, there was a simpler competing proposal
<heycam> ... providing "highlightsFromPoint", put a click event on the body, when there's a click pass the coordinates into this function
<heycam> ... some discussion for coming up with schemes where the events could then be dispatched in a manual way to highlights
<heycam> ... discussed in the issue, not a great way for non-coordinated libraries to coordinate an event loop
<heycam> ... that said, I think it's maybe worth pursuing this as a MVP API
<heycam> ... seems like a useful addition to the platform
<heycam> ... if in practice there turns out to be issues with non-coordinating highlight libraries working together, we then go back to pursue highlighting events in addition to that
<heycam> ... CSS.highlights.highlightFromPoint()
<heycam> ... some issues with shadow DOM
<TabAtkins> Seems useful to me. The coordination issue still needs to be addressed, but I don't think that blocks this as a useful ability. I like the API discussed.
<heycam> ... first let's get some feedback from the group. is this worth it as an initial step?
<Rossen_> q?
<flackr> +1 MVP of highlights from points seems useful even if we add events
<heycam> TabAtkins: seems useful to me. agree the coordination issue needs to be resolved, but that shouldn't block this useful primitive
<heycam> ... if this blocks general abilities, I still think it's worthwhile
<heycam> ... I like the API shape in the proposal
<TabAtkins> s/this blocks/this unblocks/
<heycam> olli: I agree this is probably good enough for now
<heycam> ... might need to think about shadow DOM for now, but we can tweak the proposal for that
<Rossen_> q?
<heycam> s/olli/smaug/
<heycam> dandclark: seem to have agreement on adding this
<heycam> ... would be good to dig into the shadow DOM issue
<heycam> ... does it go on DocumentOrShadowRoot?
<heycam> Rossen: let's resolve on adding the API, if we still have time at the end of this slot, we can come back to this issue
<dbaron> (My initial reaction is that DocumentOrShadowRoot is a good location because then it lives next to elementFromPoint, which it is similar to.)
<heycam> RESOLVED: Add the highlightFromPoint() API to CSS.highlights or DocumentOrShadowRoot
dandclark commented 2 years ago

I filed a new issue (#7766) for the highlightsFromPoint() shadow DOM question, since there's already a lot going on in this thread.

stephanieyzhang commented 2 months ago

Hi all, we are planning on creating a spec and implementing the highlightsFromPoint() API on CSS.highlights (not DocumentOrShadowRoot).

The issue of how the API interacts with shadow DOM is detailed in #7766. In summary, we proposed that the API has an optional dictionary parameter with a key that maps to an array of ShadowRoot objects that can return highlights, similar to caretPositionFromPoint() and getHTML(). Please let us know if there are any questions or concerns. Thanks!

cc: @sanketj