WICG / webcomponents

Web Components specifications
Other
4.36k stars 370 forks source link

Selection APIs for Shadow DOM #79

Closed hayatoito closed 1 year ago

hayatoito commented 9 years ago

Title: [Shadow]: Find a way for selection to work across shadow DOM subtrees (bugzilla: 15444)

Migrated from: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444


comment: 0 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c0 Dimitri Glazkov wrote on 2012-01-06 18:40:35 +0000.

As specified in http://dvcs.w3.org/hg/webcomponents/rev/3fb19f98bead, window.getSelection() may never retrieve content from shadow DOM subtrees. Also, a user can't select content from both document tree and shadow DOM tree. We must fix that somehow.


comment: 1 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c1 Dimitri Glazkov wrote on 2012-01-06 20:29:06 +0000.

Should we allow shadow DOM subtrees to specify whether they want to be treated as part of "as-rendered" structure or as a separate subtree?

Currently, for getSelection(), the WebKit implementation returns serialized value of the Selection object inside of a shadow DOM subtree, but node values are adjusted to avoid leaking shadow DOM nodes.


comment: 2 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c2 Steve Orvell wrote on 2013-09-05 01:50:34 +0000.

This is an important UX concern. I think it's fine to limit access to selection data as defined by the spec. However, users expect to be able to select and copy text in a web page. To have that limited by invisible ShadowDOM boundaries would be very annoying. Ideally, this just always works and is separate from the encapsulation provided via ShadowDOM.


comment: 3 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c3 Dimitri Glazkov wrote on 2014-03-06 00:11:06 +0000.

One thing that Jonas suggested at the recent spec review is to make our selection language non-normative. It's a tough subject, so we shouldn't freeze this into the spec. The suggestion was to have the language along these lines:

"Selection is not defined. Implementation should do their best to do what's best for them. Here's one possible, admittedly naive way: <insert current normative wording, but make it informative>"


comment: 4 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c4 Hayato Ito wrote on 2014-03-10 06:09:43 +0000.

(In reply to Dimitri Glazkov from comment #3)

One thing that Jonas suggested at the recent spec review is to make our selection language non-normative. It's a tough subject, so we shouldn't freeze this into the spec. The suggestion was to have the language along these lines:

"Selection is not defined. Implementation should do their best to do what's best for them. Here's one possible, admittedly naive way: <insert current normative wording, but make it informative>"

Done at https://github.com/w3c/webcomponents/commit/25bd518701866866a26d9d7e3e50d90a45f62d93.

I'll keep this bug open until we have a better model, that is a tough issue for us.


comment: 5 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c5 Dimitri Glazkov wrote on 2014-03-10 16:07:28 +0000.

(In reply to Hayato Ito from comment #4)

(In reply to Dimitri Glazkov from comment #3)

One thing that Jonas suggested at the recent spec review is to make our selection language non-normative. It's a tough subject, so we shouldn't freeze this into the spec. The suggestion was to have the language along these lines:

"Selection is not defined. Implementation should do their best to do what's best for them. Here's one possible, admittedly naive way: <insert current normative wording, but make it informative>"

Done at https://github.com/w3c/webcomponents/commit/ 25bd518701866866a26d9d7e3e50d90a45f62d93.

I'll keep this bug open until we have a better model, that is a tough issue for us.

Maybe kill the 6.1.1 section title and remove the musty language from the non-normative parts?


comment: 6 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c6 Hayato Ito wrote on 2014-03-11 07:45:41 +0000.

(In reply to Dimitri Glazkov from comment #5)

Maybe kill the 6.1.1 section title and remove the musty language from the non-normative parts?

Sure. Done at https://github.com/w3c/webcomponents/commit/0887618b6f247d1d59f37fbc474313014d81f227


comment: 7 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c7 Hayato Ito wrote on 2014-11-19 05:06:12 +0000.

* Bug 25038 has been marked as a duplicate of this bug. *


comment: 8 comment_url: https://www.w3.org/Bugs/Public/show_bug.cgi?id=15444#c8 Hayato Ito wrote on 2015-04-22 21:31:06 +0000.

Status Update: This bug is still on our radar, but we couldn't spend much time on this issue in terms of the spec.

FYI. In Blink, we are working on supporting selection across shadow boundaries 1. However, there is no update on API in the spec yet.

yoichio commented 6 years ago

I proposed updating Selection API for Shadow in TPAC: https://github.com/yoichio/public-documents/blob/master/selectionapiforshadow.md
Discussion log: https://www.w3.org/2017/11/10-webplat-minutes.html#item05

@smaug----, do you have any opinion?

annevk commented 6 years ago

I suspect @rniwa is also interested in this as I believe he was working on the Selection API last.

(It's encouraging to read that it attempts to preserve the encapsulation boundary.)

TakayoshiKochi commented 6 years ago

@yoichio and @rniwa (and others) discussed the topic off-meeting-room, which is not captured in the meeting notes. In a nutshell, I understand @rniwa's idea is to preserve the both ends of selection via opaque handles to point somewhere in the whole tree, not explicit node + offset pairs. Lots of details to be fleshed out, but sounded a feasible idea.

rniwa commented 6 years ago

Basically, the idea is to provide a mechanism to refer to a specific position within a shadow DOM with a mechanism that can be also used to refer to a specific position in pseudo element, SVG use element's shadow tree, etc...

It's probably sensible to introduce an interface on ShadowRoot to see the boundary points within a specific shadow tree but that shouldn't be Selection interface since that interface has a bunch of methods like collapse and extend which don't make sense to have it for every shadow root.

We also discussed that we need a mechanism to pick a mode between having a separate selection & having a selection that's shared with the parent tree. e.g. if you're creating an editor, you may want to have its own selection whereas if you're just an article, you probably want the selection to be shared with the rest of the document.

TakayoshiKochi commented 6 years ago

Also discussed there that saving / restoring selection states (ie. serializable selection) was the biggest request from web-based editor authors.

smaug---- commented 6 years ago

How would serializable selection work with closed ShadowRoots? I guess some proposal will explain that?

rniwa commented 6 years ago

So the idea is to use (shadowHost, position identifier) pair for selection start & end where position identifier is an author-script defined location within each shadow tree. If a position lies within another shadow tree, then the identifier (some integer) should be able to distinguish any selection end points within the inner shadow tree (recursively).

For pseudo element, textarea, input, SVG use element, etc... UA defines this identifier (probably needs to be spec'ed).

web-padawan commented 5 years ago

Recently did some research on practical aspects while trying to get a WYSIWYG editor working in Shadow DOM, submitted the report to the polyfill repo: https://github.com/webcomponents/shadydom/issues/113#issuecomment-427066346

Hope to see some progress on this topic after F2F in October.

aadamsx commented 5 years ago

You'll find an example of it not working here: https://github.com/ckeditor/ckeditor5-engine/issues/692#issuecomment-427027745

yoichio commented 5 years ago

Thank you for investigation! Pain points from web authors are great motivation for us to move this forward:)

yoichio commented 5 years ago

I wrote up new ComposedSelection API Explainer: http://bit.ly/2yPAd5h. I'm going to propose this on this TPAC next week. Any comment, especially from web developers, is welcome(you also can comment on the doc).

rniwa commented 5 years ago

Rough consensus at TPAC F2F:

Action Item: Figure out what happens to each method in Selection.

smaug---- commented 5 years ago

and getComposedRange would return a StaticRange

mfreed7 commented 2 years ago

So I dusted off @yoichio's 2018 proposal to support Shadow DOM with the Selection APIs, and created an updated version. This one attempts to finish the action items (https://github.com/WICG/webcomponents/issues/79#issuecomment-432974389) that were opened after that initial proposal/discussion. I updated the shape of the API (getComposedRange() instead of getComposedSelection()), and added details for how each of the other Selection APIs needs to change.

New explainer: https://github.com/mfreed7/shadow-dom-selection#readme

This is a first (rewrite) draft, so thoughts and input are appreciated!

rniwa commented 2 years ago

Like I've stated elsewhere, getComposedRange should take shadow roots as arguments regardless of whether they're open or not. This is particularly important because otherwise, a code which is supposed to work with nodes within a given shadow root (of a specific component) can accidentally stumble upon nodes inside other shadow roots.

mfreed7 commented 2 years ago

Like I've stated elsewhere, getComposedRange should take shadow roots as arguments regardless of whether they're open or not. This is particularly important because otherwise, a code which is supposed to work with nodes within a given shadow root (of a specific component) can accidentally stumble upon nodes inside other shadow roots.

Thanks for the comment. The reason I didn't want to do that was ergonomics. It seems like it would make this API very hard to use:

// Do full tree walk to find all shadow roots
let allRoots = [];
document.body.querySelectorAll('elements-that-support-shadowroots').forEach(e => {
  if (e.shadowRoot)
    allRoots.push(e.shadowRoot);
});

// Now, see where the selection is
let range = getComposedRange(allRoots);

I can definitely see the case for needing to provide closed roots, to avoid leaking them. But requiring the developer to "tell" us where all of the open roots are just seems like extra work, to avoid leaking something that is already public.

What do developers think about this? Perhaps the ergonomics aren't as bad as I think? Perhaps the concern (accidentally seeing nodes inside of "unknown" shadow roots) is a valid/good concern?

Perhaps, if that turns out to be a common developer concern, we could add some API (subject to bikeshedding) to optionally restrict to just the provided roots?

let rangeInKnownElementsOnly = getComposedRange({onlyIncludeTheseRoots: roots});
let rangeAnywhere = getComposedRange();
rniwa commented 2 years ago

Like I've stated elsewhere, getComposedRange should take shadow roots as arguments regardless of whether they're open or not. This is particularly important because otherwise, a code which is supposed to work with nodes within a given shadow root (of a specific component) can accidentally stumble upon nodes inside other shadow roots.

Thanks for the comment. The reason I didn't want to do that was ergonomics. It seems like it would make this API very hard to use:

// Do full tree walk to find all shadow roots
let allRoots = [];
document.body.querySelectorAll('elements-that-support-shadowroots').forEach(e => {
  if (e.shadowRoot)
    allRoots.push(e.shadowRoot);
});

// Now, see where the selection is
let range = getComposedRange(allRoots);

What is a concrete use case for doing this?

I can definitely see the case for needing to provide closed roots, to avoid leaking them. But requiring the developer to "tell" us where all of the open roots are just seems like extra work, to avoid leaking something that is already public.

The issue isn't so much about leaks in the case of open shadow roots but more about ergonomics & separations of concerns. If the only way to get the range of selection when its end points are in a shadow root is to get selection end points in any shadow root, then you're effectively back to not having shadow root encapsulation at all. You'd have to constantly check against end points to see if it's in your shadow root or not.

web-padawan commented 2 years ago

My 2 cents as a maintainer of Quill fork which has to deal with Safari encapsulation (see https://github.com/vaadin/quill/pull/2).

In case of rich text editor component, a shadow root is typically known in advance, some example:

this.rootDocument = (this.root.getRootNode ? this.root.getRootNode() : document);
// get selection
const selection = this.rootDocument.getSelection();

Then there is a relatively small polyfill for getSelection().

The question is: would this use case be possible to achieve by using proposed getComposedRange(rootDocument)? Please correct me if I'm wrong, but the answer seems to be "yes". So I'm fine with what @rniwa suggests.

mfreed7 commented 2 years ago

What is a concrete use case for doing this?

I guess the most basic one: click a button that highlights whatever the user has selected with some color. To do that, you'd need to provide all shadow roots on the page, otherwise you'll get it wrong. I.e. if a user has selected half-way through some text inside a shadow root, you'll change the color for the entire shadow root contents, unless you pre-provide that shadow root in the call to getComposedRange().

What's a concrete use case that would be "confused" by receiving Nodes that live in any (open) shadow root?

The issue isn't so much about leaks in the case of open shadow roots but more about ergonomics & separations of concerns. If the only way to get the range of selection when its end points are in a shadow root is to get selection end points in any shadow root, then you're effectively back to not having shadow root encapsulation at all. You'd have to constantly check against end points to see if it's in your shadow root or not.

I suppose the counterpoint is that for pages that want to make extensive use of shadow dom, this API shape gets really cumbersome. E.g.:

<body>
<my-app></my-app>
</body>

In this case, assuming <my-app> uses shadow DOM, getComposedSelection(allRoots) just doesn't work at all unless you pre-walk the tree and provide everything in allRoots.

I'm curious why you think shadow dom is special here. The user is already able to select anything on the page, including inside deeply nested shadow DOM. The existing getRangeAt() already returns any Node on the page, not just inside some sub-tree. This new API is meant to extend that current behavior to Nodes inside shadow trees. If you're writing some code that doesn't understand Shadow DOM, great, just keep using getRangeAt().

The question is: would this use case be possible to achieve by using proposed getComposedRange(rootDocument)? Please correct me if I'm wrong, but the answer seems to be "yes". So I'm fine with what @rniwa suggests.

Sorry, I'm a bit unclear on the specific use case you're asking about here. You should be able to use either version of getComposedRange() to get the composed range. The difference is what arguments you need to pre-provide.

rniwa commented 2 years ago

What is a concrete use case for doing this?

I guess the most basic one: click a button that highlights whatever the user has selected with some color. To do that, you'd need to provide all shadow roots on the page, otherwise you'll get it wrong. I.e. if a user has selected half-way through some text inside a shadow root, you'll change the color for the entire shadow root contents, unless you pre-provide that shadow root in the call to getComposedRange().

That does directly against the basic feature of Shadow DOM: encapsulation. The whole point of having a Shadow DOM is to encapsulate the implementation details of a component from the rest of a web page. The idea that some scripts may need to apply new behaviors across components without those components opt'ing in goes against this basic principle.

What's a concrete use case that would be "confused" by receiving Nodes that live in any (open) shadow root?

Consider an editor component, which consists of a content editable region and a toolbar for things like bolding & italicizing text. As the user moves selection to different parts of the content editable region, the toolbar status may need to be updated. The component wants to do that by observing selection changes within the component. The proposed API which returns a node anywhere in the document will mean that this component will then have to check whether the selection resides within the content editable element of this component's shadow tree or not. This will be worse developer ergonomics than the proprietary API Blink has on ShadowRoot interface right now.

The issue isn't so much about leaks in the case of open shadow roots but more about ergonomics & separations of concerns. If the only way to get the range of selection when its end points are in a shadow root is to get selection end points in any shadow root, then you're effectively back to not having shadow root encapsulation at all. You'd have to constantly check against end points to see if it's in your shadow root or not.

I suppose the counterpoint is that for pages that want to make extensive use of shadow dom, this API shape gets really cumbersome. E.g.:

<body>
<my-app></my-app>
</body>

In this case, assuming <my-app> uses shadow DOM, getComposedSelection(allRoots) just doesn't work at all unless you pre-walk the tree and provide everything in allRoots.

It's unclear that getComposedSelection(allRoots) is something we want allow anyone to be using. Again, this goes directly against the basic idea of encapsulation.

I'm curious why you think shadow dom is special here. The user is already able to select anything on the page, including inside deeply nested shadow DOM. The existing getRangeAt() already returns any Node on the page, not just inside some sub-tree. This new API is meant to extend that current behavior to Nodes inside shadow trees. If you're writing some code that doesn't understand Shadow DOM, great, just keep using getRangeAt().

Because the whole point of Shadow DOM is to encapsulate its contents. If there is a desire for some scripts to access anywhere in the document, then why use Shadow DOM at all? Shadow DOM isn't designed to be general purpose API that can be used for any purpose whatsoever; its primary function is to provide encapsulation.

web-padawan commented 2 years ago

Consider an editor component, which consists of a content editable region and a toolbar for things like bolding & italicizing text. As the user moves selection to different parts of the content editable region, the toolbar status may need to be updated. The component wants to do that by observing selection changes within the component.

This is exactly what I described in the above comment. And the API proposal for getComposedRange(shadowRoot) is indeed aligned with shadowRoot.getSelection() implemented in Chrome.

sspi commented 2 years ago

In real life, shadow DOM encapsulation is used for many different purposes. One is css isolation for example. All applicative features must not always respect all these encapsulations.

In my app, I use intensively ShadowDom. I have 2 libraries for scanning nodes. "DOM" without crossing ShadowRoot, and "DOMSH" for Shadow abstraction. I have counted the uses of these libraries in my project core code : ~1300 calls for DOM and ~650 calls for DOMSH.

My 2 cent's: Selection is a transversal feature, we sometimes want to use intra-shadowRoot approach, sometimes the inter-shadowRoot approach. You should offer both.

mfreed7 commented 2 years ago

That does directly against the basic feature of Shadow DOM: encapsulation. The whole point of having a Shadow DOM is to encapsulate the implementation details of a component from the rest of a web page. The idea that some scripts may need to apply new behaviors across components without those components opt'ing in goes against this basic principle.

I think it's important to point out that open Shadow DOM is not a security or perfect-isolation boundary. I do agree that closed Shadow DOM should prohibit JS from having access to inner details. But the distinction vs. open Shadow DOM is precisely and only that internal details are accessible, just not via CSS or naive querySelector() calls. Javascript already can access all of the details of any open Shadow Root. This proposal doesn't change that.

Consider an editor component, which consists of a content editable region and a toolbar for things like bolding & italicizing text. As the user moves selection to different parts of the content editable region, the toolbar status may need to be updated. The component wants to do that by observing selection changes within the component. The proposed API which returns a node anywhere in the document will mean that this component will then have to check whether the selection resides within the content editable element of this component's shadow tree or not. This will be worse developer ergonomics than the proprietary API Blink has on ShadowRoot interface right now.

I think to actually implement this example, you'll need to handle the endpoints being anywhere in the document. For example, what happens when a user mouses-down outside the editor, drags the selection into the middle of the editor text, and mouses-up there. Then they go to click the Bold/Italic button. They should (rightly) expect the portion of the editor text that is highlighted to turn Bold/Italics. However, in this case, one endpoint of the selection is within the component and the other is outside. You can try this example right here in the Github editor, by the way. While it doesn't use Shadow DOM, the behavior is somewhat supported.

It's unclear that getComposedSelection(allRoots) is something we want allow anyone to be using. Again, this goes directly against the basic idea of encapsulation.

Maybe we're misunderstanding each other, but getComposedSelection(allRoots) is your suggestion. I.e. your comment above says "getComposedRange should take shadow roots as arguments regardless of whether they're open or not".

I'm curious why you think shadow dom is special here. The user is already able to select anything on the page, including inside deeply nested shadow DOM. The existing getRangeAt() already returns any Node on the page, not just inside some sub-tree. This new API is meant to extend that current behavior to Nodes inside shadow trees. If you're writing some code that doesn't understand Shadow DOM, great, just keep using getRangeAt().

Because the whole point of Shadow DOM is to encapsulate its contents. If there is a desire for some scripts to access anywhere in the document, then why use Shadow DOM at all? Shadow DOM isn't designed to be general purpose API that can be used for any purpose whatsoever; its primary function is to provide encapsulation.

See my comment above on this. But script already has full access to all open Shadow Roots, while only closed Shadow Roots are meant to fully encapsulate their contents from JS.

There are two potential reasons to want to require developers to specify all shadow roots in the call to getComposedRange():

  1. To help avoid accidental access to the wrong shadow root. I.e. it might be more ergonomic for most developers.
  2. To prohibit access to the "wrong" shadow root. I.e. to provide security for open shadow roots.

I hope it is clear that #2 can't be a thing. Open shadow roots are "open" already. That leaves #1 - the ergonomics question - as the one we need to explore here. I'm open to the idea that it might be easier to pre-limit the shadow roots that we might receive from getComposedRange(), but it'd be great to hear from developers on that. Absent that, it seems like it makes the most sense to offer both ways. I.e. one calling syntax (e.g. just getComposedRange()) that returns endpoints anywhere, and another (e.g. getComposedRange({onlyThese: [...roots]})) for developers who would like to limit to just a list of shadow roots. WDYT?

rniwa commented 2 years ago

It seems clear that we don't have any consensus to move forward on this proposal.

mfreed7 commented 2 years ago

This API will be discussed at tomorrow's TPAC breakout session at 3pm UTC (slides). This is an open meeting, so anyone is welcome to attend and contribute! We can hopefully dedicate some significant discussion time for the open/closed shadow root question, and anything else that comes up.

mfreed7 commented 2 years ago

It seems clear that we don't have any consensus to move forward on this proposal.

While I agree that we're not yet 100% aligned on the shape of the API, I would hope you're interested in trying to reach a consensus? Do you have concrete recommendations to try to move us closer to agreement?

rniwa commented 2 years ago

It seems clear that we don't have any consensus to move forward on this proposal.

While I agree that we're not yet 100% aligned on the shape of the API, I would hope you're interested in trying to reach a consensus? Do you have concrete recommendations to try to move us closer to agreement?

I don't find any of your arguments in favor of having API across open shadow trees. In fact, I'm pretty much against the primary API for selection inside shadow root to behave differently between open and closed shadow trees. Given this, it seems like we're at stalemate.

Just FYI, I'm on an extended medical leave of absence so I'm definitely not attending whatever breakout sessions happening during TPAC. I simply felt compelled enough to leave comments here so that the group won't proceed assuming that whatever is currently proposed for everyone involved. It definitely isn't.

annevk commented 2 years ago

FWIW, I agree with Ryosuke that APIs should work across open and closed shadow trees and we shouldn't create APIs that solely work for open trees. There is some precedent for the latter with composedPath(), but that was more of a compromise than a desired design path.

boazsender commented 2 years ago

At the TPAC 2021 breakout on this topic, @hunterloftis mentioned that there are a number of rich text editors that want to use shadow dom, but are blocked on this issue. Hunter linked to these three issues:

What if we put together a quick survey and post it to these issues for input and/or contact folks participating in those issues to fill it out?

web-padawan commented 2 years ago

What if we put together a quick survey and post it to these issues for input

Regarding Quill, feel free to check out the fork that I mentioned above https://github.com/WICG/webcomponents/issues/79#issuecomment-937518617

It's based on the work done by @43081j who is the author of the PR mentioned above. On top of that, it also includes some fixes done by @Gabez0r in Nuxeo Quill fork.

mfreed7 commented 2 years ago

We just had a very good discussion of this proposal/issue at a TPAC 2021 breakout - thanks to all that participated! I will summarize the conclusions and takeaways as I heard them - feel free to correct me if I missed something:

  1. We discussed several alternative API shapes: a. Have getComposedRange() return a list of ranges, partitioned by shadow boundaries. b. Have shadowRoot.getSelection() which returns a "fake" range whose endpoints live entirely within that shadow root. c. Have getComposedRange() return something like Event.composedPath(), with lists of nodes up to the document root.
  2. We generally agreed that the best way forward for a "V1 API" was to move ahead with a getComposedRange() very similar to the one being proposed, but that requires passing in any (open or closed) shadow roots.
  3. If/when a use case comes to light for which developers end up wanting access to all open roots, we can add a parameter (e.g. getComposedRange({anyOpenShadowRoot:true})).
  4. I took an action item to dig further into the details of the "fake" range returned by getRangeAt() under this proposal. How would this be represented, exactly? What happens when the live Range is mutated by Javascript (e.g. with range.setStart())? What happens when a tree mutation changes the selection boundaries within a shadow root?
  5. I took an action item to dig further into browser selection canonicalization behavior, which adjusts selection endpoints in various ways, such as pushing them to the deepest equivalent point in the tree. The suggestion is that this behavior shouldn't materially change, for compat reasons, but this needs more attention.

I will incorporate all of the above (including the conclusions of my action items) back into the explainer, and I'll post here when that's done. At that point, the meeting participants thought it would be worthwhile to reach out to editor component authors, to see if the (new) proposed API would meet most of their needs. We should include not only the components listed in https://github.com/WICG/webcomponents/issues/79#issuecomment-946842531, but also the ones listed in the explainer.

mfreed7 commented 2 years ago

So I'm in the process of updating the explainer with the points mentioned in my prior comment. And I ran across a set of issues that I don't see an immediate (good) solution for. Both can be seen using the last part of example #2 from the explainer. In that example, an editor Web Component (<x-editor>) that does not use shadow DOM is nested inside another Web Component (<parent-component>) that does use shadow DOM.

Problem #1

Since the editor component itself, which needs selection info, does not contain a shadow root, what should it pass in to the shadowRoots argument here: selection.getComposedRange({shadowRoots: [what?]})? I suppose it could walk up the shadow-including tree from this, and pass in the first shadow root it finds? That feels a bit odd, for a component that really doesn't want to use Shadow DOM, but finds itself located inside one.

Problem #2

This brings up another issue in the definition of exactly which shadow roots are required in the call to getComposedRange({shadowRoots: []}). When we discussed this, and in the explainer, only the "deepest" shadow root was needed, which was enough to reveal knowledge of that shadow root and all ancestor shadow roots. But that will necessarily reveal selections in the entire shadow-including tree above the component, which might be deeply nested. If the purpose of the shadowRoots parameter to getComposedRange() is to limit selections to "known" shadow roots, then it doesn't really work in this case. A fix to this would be to require all shadow roots to be included, but then that's ambiguous. What gets returned if you only provide "your" shadow root, but the selection surrounds "you" but begins and ends in a shadow root above you?


Both of these point (me at least) back to the original proposal which doesn't require passing in open shadow roots. It feels like all of the same problems will exist in that case, but we won't require developers to also do extra work to walk the tree looking for shadow roots to provide. Do the proponents of requiring open shadow roots to be specified have a suggestion for a fix for these problems?

rniwa commented 2 years ago

So I'm in the process of updating the explainer with the points mentioned in my prior comment. And I ran across a set of issues that I don't see an immediate (good) solution for. Both can be seen using the last part of example #2 from the explainer. In that example, an editor Web Component (<x-editor>) that does not use shadow DOM is nested inside another Web Component (<parent-component>) that does use shadow DOM.

Problem #1

Since the editor component itself, which needs selection info, does not contain a shadow root, what should it pass in to the shadowRoots argument here: selection.getComposedRange({shadowRoots: [what?]})? I suppose it could walk up the shadow-including tree from this, and pass in the first shadow root it finds? That feels a bit odd, for a component that really doesn't want to use Shadow DOM, but finds itself located inside one.

You can just use container.getRootNode()? This problem needs to be resolved regardless if it's a problem for open shadow root since the same problem will exist for a closed shadow root. So this isn't a good argument that open shadow roots should be automatically included in this API.

Problem #2

This brings up another issue in the definition of exactly which shadow roots are required in the call to getComposedRange({shadowRoots: []}). When we discussed this, and in the explainer, only the "deepest" shadow root was needed, which was enough to reveal knowledge of that shadow root and all ancestor shadow roots. But that will necessarily reveal selections in the entire shadow-including tree above the component, which might be deeply nested. If the purpose of the shadowRoots parameter to getComposedRange() is to limit selections to "known" shadow roots, then it doesn't really work in this case.

Again, this is a problem with this particular API shape regardless of whether open shadow roots are automatically included or not since it would be an issue for closed shadow roots. Perhaps we need an optional parameter specifying the root node under which the selection end points need to be found. e.g. getComposedRange({shadowRoots: [editor.getRootNode()], root: editor}).

A fix to this would be to require all shadow roots to be included, but then that's ambiguous. What gets returned if you only provide "your" shadow root, but the selection surrounds "you" but begins and ends in a shadow root above you?

This is problematic regardless. It's not an argument for including just open shadow roots.

Both of these point (me at least) back to the original proposal which doesn't require passing in open shadow roots.

Decidedly not.

Generally, any problems you've raised about open shadow roots are also applicable to closed shadow roots so I don't find any of those points remotely close to supporting the argument for automatically including all open shadow roots.

mfreed7 commented 2 years ago

I wasn't trying to make the case for going back to only specifying open shadow roots, but I realize that my last paragraph came across that way. I'm really asking for brainstorming ideas for what seem like some potential issues with this shape.

You can just use container.getRootNode()? This problem needs to be resolved regardless if it's a problem for open shadow root since the same problem will exist for a closed shadow root. So this isn't a good argument that open shadow roots should be automatically included in this API.

That's true, perhaps this isn't such an imposition after all. It still does seem funny to require any non-shadow-dom component to pass in shadow roots from above it, just to be able to see the selection within the bounds of the component itself.

Again, this is a problem with this particular API shape regardless of whether open shadow roots are automatically included or not since it would be an issue for closed shadow roots. Perhaps we need an optional parameter specifying the root node under which the selection end points need to be found. e.g. getComposedRange({shadowRoots: [editor.getRootNode()], root: editor}).

And then what happens if the selection endpoints are outside the provided root node? They get re-written to start just inside the provided root? I suppose that would simply things if you just want to handle selections within your component. I think that actually solves many of the problems.

I'm wondering if perhaps instead of requiring a list of shadowRoots at all, you just provide a root node? That would prove that you have access to whatever shadow root contains it, and avoids the prior point that it's weird for non-shadow-dom components to need to pass in a shadow root. Perhaps you optionally provide shadowRoots and/or a root node, but either suffice to "unlock" that shadow root? Thoughts?

rniwa commented 2 years ago

I wasn't trying to make the case for going back to only specifying open shadow roots, but I realize that my last paragraph came across that way. I'm really asking for brainstorming ideas for what seem like some potential issues with this shape.

Ok, thanks for the clarification.

Again, this is a problem with this particular API shape regardless of whether open shadow roots are automatically included or not since it would be an issue for closed shadow roots. Perhaps we need an optional parameter specifying the root node under which the selection end points need to be found. e.g. getComposedRange({shadowRoots: [editor.getRootNode()], root: editor}).

And then what happens if the selection endpoints are outside the provided root node? They get re-written to start just inside the provided root? I suppose that would simply things if you just want to handle selections within your component. I think that actually solves many of the problems.

We could do that but then we may need to indicate that those end points either start before/after the root or not. Some components may not want to do anything unless the entirety of the selection resides within; e.g. text editor. If the user selected the entire document, the component may need to differentiate that from when the user selected everything in the component.

I'm wondering if perhaps instead of requiring a list of shadowRoots at all, you just provide a root node? That would prove that you have access to whatever shadow root contains it, and avoids the prior point that it's weird for non-shadow-dom components to need to pass in a shadow root. Perhaps you optionally provide shadowRoots and/or a root node, but either suffice to "unlock" that shadow root? Thoughts?

Perhaps. We probably still want to allow a list of shadow roots as well so that scripts can "see" the entire selection when there are multiple components that coordinate with one another when the user selects content across shadow boundaries. e.g. table cells / rows.

mfreed7 commented 2 years ago

We could do that but then we may need to indicate that those end points either start before/after the root or not. Some components may not want to do anything unless the entirety of the selection resides within; e.g. text editor. If the user selected the entire document, the component may need to differentiate that from when the user selected everything in the component.

This is a good point. Any thoughts on how to communicate that? Augment StaticRange with another two boolean fields to indicate whether each endpoint has been "re-written"?

Perhaps. We probably still want to allow a list of shadow roots as well so that scripts can "see" the entire selection when there are multiple components that coordinate with one another when the user selects content across shadow boundaries. e.g. table cells / rows.

Right, I was thinking something like this:

partial interface Selection {
  StaticRange getComposedRange(optional GetComposedRangeOptions options = {});
};

dictionary GetComposedRangeOptions {
  Node selectionRoot;
  sequence<ShadowRoot> shadowRoots;
};

... you can specify selectionRoot and/or shadowRoots. Specifying selectionRoot automatically adds selectionRoot.getRootNode() to shadowRoots.

mfreed7 commented 2 years ago

Ok, I've completed my action items from https://github.com/WICG/webcomponents/issues/79#issuecomment-946897700, and I've updated the explainer.

I also incorporated the brainstormed changes just above, including adding a selectionRoot argument to getComposedRange() plus more descriptions and examples about how it works. I added new sections for live Range modification and canonicalization.

Only this question remains, at least from my point of view, about how to pass back the information that "scoped" range endpoints were actually outside the selectionRoot:

This is a good point. Any thoughts on how to communicate that? Augment StaticRange with another two boolean fields to indicate whether each endpoint has been "re-written"?

Overall thoughts and questions appreciated!

mfreed7 commented 2 years ago

Since my comment above just re-links back to the updated explainer, I thought it might be helpful to quickly summarize the current state of the proposal in a comment. I'm proposing that we:

  1. Add getComposedRange() to Selection:

      window.getSelection().getComposedRange({
          shadowRoots: [shadowRoot1, shadowRoot2, ...], // (Optionally) include selections that start/end within any of the provided `shadowRoot`s.
          selectionRoot: element, // (Optionally) scope the returned selection to live within the provided root node
      });

    In the above call, the shadowRoots argument allows the selection to be "revealed" within any of the provided shadowRoots. For example, if the user selection starts or ends within a custom element that uses Shadow DOM, and the developer would like to retrieve that selection endpoint (within the shadow root), this parameter can be used to "unlock" it. If the selection starts/ends within a non-provided shadow root, the return value will make it appear that the entire shadow host is selected.

    The selectionRoot argument "scopes" the selection, so that the returned selection endpoints are always descendants of element. This is meant to be used, for example, by a rich text editor custom element that wants to manage selections within it, but doesn't want to know about or have to deal with selections outside the component. This also implicitly adds any containing shadowRoots to the shadowRoots parameter, so that selections are always available, even when the custom element is contained within an ancestor shadow root.

  2. Update the existing Selection APIs:

    The rest of the Selection object will stay the same, except that cross-shadow-root selections will now be allowed. For example, when calling window.getSelection().setBaseAndExtent(node1,offset1,node2,offset2), the node1 and node2 endpoints will now be able to live in different shadow trees. Previously, this was not supported behavior.

Again, see the full proposal for all the details. But I hope this summary might help the conversation and feedback.

spocke commented 2 years ago

We have struggled to get tinymce to work within a shadowRoot without using an iframe and part of that is that we can't get the selection out from within the shadowRoot this getComposedRange just partially solves that since we also need to know the selection direction currently we determine that from comparing the focusNode/anchorNode/focusOffset/anchorOffset properties of the selection. From my understanding of the proposal the focusNode/anchorNode/focusOffset/anchorOffset will just be set to the Selection.getRange(0) that would mean it would always be collapsed or always be in one direction. Not sure how to solve this maybe have a function similar to getComposedRange that takes a set of roots and returns if it's forwards or backwards or that it instead of a StaticRange returns a new type of object that has focusNode/anchorNode/focusOffset/anchorOffset/collapsed properties like a StaticSelection or something.

mfreed7 commented 2 years ago

Thanks @spocke - this is a very good point that isn't captured in the current proposal. So currently, Selection.getRangeAt() returns a Range with startContainer always being the left-side of the selection (even for RTL, if seems?), and endContainer always being the right side. I am unable to find the place in the Selection specs where this behavior is defined, but all browsers seem to agree.

I'm wondering if the best way around this would just be to expose the direction directly on the Selection object? That wouldn't require changing the definition of AbstractRange, and it would even make getRangeAt() a bit easier to work with, I think. Something like this:

const selection = window.getSelection();
const range = selection.getComposedRange(); // As proposed
const dir = selection.direction; // "forwards", "backwards", or "directionless"

Thoughts?

spocke commented 2 years ago

That sounds like it would work and simple to use. We need to be able to get/set the selection and maintaining the direction so if we have all those things we should be able to get the editor working properly inside a shadowRoot.

rniwa commented 2 years ago

Yeah, exposing direction property on Selection makes most sense since we already have selectionDirection on input and textarea.

annevk commented 2 years ago

@mfreed7 this looks pretty good to me overall. One thing that would be great to see flushed out a bit more is the data model (which I realize is currently lacking as well). Is the idea that Selection holds a true range and Range holds a selection (null or a Selection) and when mutating Range, if it has a non-null selection, we'd use the Range's selection's true range in various ways?

mfreed7 commented 2 years ago

Great, sounds like no objections to exposing direction on Selection - I'll add that to the proposal text.

@mfreed7 this looks pretty good to me overall. One thing that would be great to see flushed out a bit more is the data model (which I realize is currently lacking as well). Is the idea that Selection holds a true range and Range holds a selection (null or a Selection) and when mutating Range, if it has a non-null selection, we'd use the Range's selection's true range in various ways?

Glad you think it looks ok also! I allude to this in this section of the explainer, but generally I think the answer is "yes". I've been thinking of it as if Selection holds the true range as you said, but Range is a view into that true range. Mutations to the Range will reach through and mutate the true range in the prescribed ways. I'm open to suggestions about how to better document this data model.

mfreed7 commented 2 years ago

...ok, I added a new section to the explainer that discusses the addition of Selection.direction.

annevk commented 2 years ago

I think what I described makes sense, whereby Range holds a pointer to a Selection. That way it can serve as a view (if bound to a Selection) or as something more concrete (when it's standalone). (I think this might already be a requirement as otherwise how would the Selection know when to update itself, but it's just not defined at the moment.)

mfreed7 commented 2 years ago

So from the last triage meeting, it sounds like there's roughly general agreement on the shape of the proposal. The next step is to put up some spec PRs to implement it. If someone would like to do that, great, please let me know! Otherwise our plan is to try to tackle this in early to mid 2022.

mfreed7 commented 2 years ago

FYI, the TAG just concluded their design review of the new design, and they said they were reasonably happy with the shape.

They did raise one question:

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())?

It seems like a reasonable question. I believe getComposedRange() should be usable for all use cases, including any covered by getRangeAt(). Perhaps we should rename and deprecate as they recommend?

smaug---- commented 2 years ago

getComposedRange does not cover all the cases. getRangeAt is handy when modifying the range.

mfreed7 commented 2 years ago

getComposedRange does not cover all the cases. getRangeAt is handy when modifying the range.

Yeah, that's a good point. getComposedRange() is good for all of the "read only" use cases, but not the mutation ones.