w3ctag / design-reviews

W3C specs and API reviews
Creative Commons Zero v1.0 Universal
332 stars 56 forks source link

Modification of selection APIs to account for shadow dom #694

Closed mfreed7 closed 2 years ago

mfreed7 commented 2 years ago

Braw mornin' TAG!

I'm requesting a TAG review of modifying the selection APIs to account for shadow dom.

Shadow DOM (v1) is now supported by all evergreen rendering engines. However, the user selection APIs are not well supported for the case that the user selection crosses shadow boundaries. The existing Selection API specifies that there is only a single selection in a document and the selection is bound to a single Range, which means that it cannot represent a range over the composed tree. Said another way, if the selection crosses shadow boundaries, the existing API cannot represent this situation correctly. For backwards compatibility, we also can't update the Range API to support ranges that cross shadow boundaries, because that could break existing assumptions about ranges.

This proposal suggests a new API that lets web authors control and query selections in the composed tree.

Further details:

We'd prefer the TAG provide feedback as (please delete all but the desired option):

💬 leave review feedback as a comment in this issue and @-notify @mfreed7

plinss commented 2 years ago

Took a look at this today with @LeaVerou and @maxpassion, we were wondering what the purpose was of adding getComposedRange vs overloading getRangeAt to accept the ComposedRangeOptions. As far as we could tell the only apparent difference is the result of a live Range vs a StaticRange, which seems like an orthogonal issue.

mfreed7 commented 2 years ago

Thanks for the review and the question. (And sorry for the delay replying!)

So I think the main reason for the new function, vs. re-using the old getRangeAt(), is backwards compatibility. This is mentioned a few places in the explainer, but the thought is that we would likely break sites if we just change getRangeAt() to return a StaticRange (rather than a live Range) that also crosses shadow bounds. Perhaps you were implying that if the developer passed a ComposedRangeOptions to getRangeAt(), you'd get the new behavior? The problem there is that the ComposedRangeOptions is optional, so a call like getRangeAt() (with no arguments) would now get the new behavior. That's likely ok compat-wise, since the existing behavior is to throw an exception with no arguments. However, in all current rendering engines, getSelection().getRangeAt({}) currently returns the live Range. So there's the possibility of compat problems if existing sites are using getRangeAt({}) for whatever reason.

Overall, I think the motivation for a new API was that we're rather-significantly changing the selection API, and beyond the calling syntax, developers will have to understand more about shadow DOM and how this new API works. Giving it a fresh/different function name helps (in my opinion) them understand that there's a difference.

I'm open to suggestions, of course.

LeaVerou commented 2 years ago

Hi @mfreed7,

We revisited this today during our Hybrid F2F. We understand the reasoning for defining a new function rather than overloading getRangeAt() and we agree that a new function is needed (having a function return different types depending on arguments is not a good idea).

One thing we noticed is that with the addition of this new function, we will now basically have two independent concepts: a. "Does the range cross Shadow DOM boundaries?" and b. "Do we need a live or a static range?"

While these are orthogonal, authors cannot specify any of the 4 combinations independently, but only two ([a. no, b. live] or [a. yes, b. static]), which is a coupling that could be confusing since these are not intrinsically related. We understand why [a. yes b. live] is not desirable, but perhaps there is value in designing the API so that [a. no, b. static] can be specified as well, which would make the new function a more general replacement for getRangeAt() as long as live ranges are not desired. What do you think?

mfreed7 commented 2 years ago

Thanks for the feedback!

I understand your question - you're saying it'd be nice to allow users of the new API to get/use StaticRange as a return value, while not crossing shadow bounds. If that's correct, I think the existing API already offers a few ways to do that. First, just calling selection.getComposedRange() with no arguments will result in a StaticRange that does not cross shadow bounds (because none were provided). So that might be the API you're asking for. If authors are building a component, and would just like a StaticRange that is scoped within their tree, the selectionRoot parameter (see this section of the explainer) can be used:

selection.getComposedRange({selectionRoot: componentRoot});

In both of these cases, the returned StaticRange will not cross shadow bounds. Let me know if I missed something about your question.

I appreciate (very much) that you didn't ask for the other permutation, returning a live Range that crosses shadow bounds. 😄

LeaVerou commented 2 years ago

We were under the impression that you only need to provide shadowRoots for closed shadows. It may be a little tedious to have to loop over the elements and recursively get all their shadows to provide to shadowRoots.

mfreed7 commented 2 years ago

We were under the impression that you only need to provide shadowRoots for closed shadows. It may be a little tedious to have to loop over the elements and recursively get all their shadows to provide to shadowRoots.

Right, the proposal started with only needing to provide closed shadow roots (and with a parameter called something like closedShadowRoots). I felt the same way - it seems like a hassle to have to go enumerate all of the shadow roots. However, after some discussion (e.g. see this summary from TPAC https://github.com/WICG/webcomponents/issues/79#issuecomment-946897700, point #2), it became clear that not only were other implementers against just requiring closed shadow roots, but also that perhaps for the typical use cases for this API it wasn't such a big ask. In particular, editor component vendors (the #1 use case) seem to want to get selection information within their component only, and receiving endpoints outside that tree, particularly inside other unknown shadow roots, was counterproductive. For these use cases, we added the selectionRoot argument, which makes the typical case work without needing to specify shadowRoots at all. There may be other use cases that require selection information anywhere in the document, but these haven't come to light. When they do, we could think about expanding the API with something like getComposedRange({includeAllOpenShadowRoots: true}) or something. And in the meantime, they can certainly enumerate the shadow roots themselves, today.

LMK if that answers your question.

LeaVerou commented 2 years ago

Hi @mfreed7, we took another look today and we understand your reasoning and are happy with the proposal so we are going to close this.

One thing we wondered about: once this is implemented are there any remaining use cases for getRangeAt()? It seems that the new method covers all of them, and since it returns a static range it's more performant and less error-prone. If that assumption is correct, would it make sense to deprecate getRangeAt() and name the new method in a more generic way (e.g. getRange())?

mfreed7 commented 2 years ago

Thanks! Your question is a good one, and I've raised it on the issue to gather comments.