Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Selection APIs for Shadow DOM #79

Open
hayatoito opened this issue May 25, 2015 · 31 comments
Open

Selection APIs for Shadow DOM #79

hayatoito opened this issue May 25, 2015 · 31 comments

Comments

@hayatoito
Copy link
Contributor

@hayatoito hayatoito commented May 25, 2015

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
25bd518.

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/
25bd518.

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
0887618


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.

@hayatoito hayatoito added the v2 label May 25, 2015
@hayatoito hayatoito changed the title [Shadow]: Find a way for selection to work across shadow DOM subtrees (bugzilla: 15444) Selection APIs for Shadow DOM Mar 16, 2016
@yoichio
Copy link

@yoichio yoichio commented Nov 11, 2017

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
Copy link
Collaborator

@annevk annevk commented Nov 13, 2017

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
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Nov 14, 2017

@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
Copy link
Collaborator

@rniwa rniwa commented Nov 14, 2017

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
Copy link
Member

@TakayoshiKochi TakayoshiKochi commented Nov 14, 2017

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

@smaug----
Copy link

@smaug---- smaug---- commented Nov 14, 2017

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

@rniwa
Copy link
Collaborator

@rniwa rniwa commented Nov 15, 2017

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).

kisg pushed a commit to paul99/webkit-mips that referenced this issue Sep 26, 2018
…e drag

https://bugs.webkit.org/show_bug.cgi?id=151380
<rdar://problem/24363872>

Source/WebCore:

Reviewed by Antti Koivisto and Wenson Hsieh.

This patch adds the basic support for selecting content across shadow DOM boundaries to VisibleSelection,
which is enough to allow users to select content across shadow DOM boundaries via a mouse drag.

This is the first step in allowing users to select, copy and paste content across shadow DOM boundaries,
which is a serious user experience regression right now. The new behavior is disabled by default under
an interal debug feature flag: selectionAcrossShadowBoundariesEnabled.

Like Chrome, we are not going to support selecting editable content across shadow DOM boundaries since
we'd have to generalize every editing commands to make that work, and there aren't any HTML editors that
use shadow DOM boundaries within an editable region yet. For simplicity, we also don't support extending
a selection out of a shadow root which resides inside an editing region.

The keyboard based navigation & manipulation of selection as well as allowing copy & paste of content
across shadow DOM boundaries will be implemented by separate patches. DOMSelection will not expose this new
behavior either. This is tracked in the spec as WICG/webcomponents#79

Tests: editing/selection/selection-across-shadow-boundaries-mixed-editability-1.html
       editing/selection/selection-across-shadow-boundaries-mixed-editability-2.html
       editing/selection/selection-across-shadow-boundaries-mixed-editability-3.html
       editing/selection/selection-across-shadow-boundaries-mixed-editability-4.html
       editing/selection/selection-across-shadow-boundaries-mixed-editability-5.html
       editing/selection/selection-across-shadow-boundaries-readonly-1.html
       editing/selection/selection-across-shadow-boundaries-readonly-2.html
       editing/selection/selection-across-shadow-boundaries-readonly-3.html
       editing/selection/selection-across-shadow-boundaries-user-select-all-1.html

* editing/VisibleSelection.cpp:
(WebCore::isInUserAgentShadowRootOrHasEditableShadowAncestor): Added.
(WebCore::VisibleSelection::adjustSelectionToAvoidCrossingShadowBoundaries): When the feature is enabled,
allow crossing shadow DOM boundaries except when either end is inside an user agent shadow root, or one of
its shadow includign ancestor is inside an editable region. The latter check is needed to disallow
an extension of a selection starting in a shadow tree inside a non-editable region inside an editable region
to outside the editable region. The rest of the editing code is not ready to deal with selection like that.
* page/Settings.yaml: Added an internal debug feature to enable this new behavior.

Source/WebKit:

Reviewed by Antti Koivisto.

Added SelectionAcrossShadowBoundariesEnabled as an internal debug feature,
and moved CSSCustomPropertiesAndValuesEnabled to where other experimental features are located.

* Shared/WebPreferences.yaml:

Source/WebKitLegacy/mac:

Reviewed by Wenson Hsieh.

Added selectionAcrossShadowBoundariesEnabled as a preference to be used in DumpRenderTree.

* WebView/WebPreferenceKeysPrivate.h:
* WebView/WebPreferences.mm:
(+[WebPreferences initialize]):
(-[WebPreferences selectionAcrossShadowBoundariesEnabled]):
(-[WebPreferences setSelectionAcrossShadowBoundariesEnabled:]):
* WebView/WebPreferencesPrivate.h:
* WebView/WebView.mm:
(-[WebView _preferencesChanged:]):

Tools:

Reviewed by Wenson Hsieh.

Added the support for internal:selectionAcrossShadowBoundariesEnabled test option.

* DumpRenderTree/TestOptions.cpp:
(TestOptions::TestOptions):
* DumpRenderTree/TestOptions.h:
* DumpRenderTree/mac/DumpRenderTree.mm:
(resetWebPreferencesToConsistentValues):
(setWebPreferencesForTestOptions):

LayoutTests:

Reviewed by Antti Koivisto and Wenson Hsieh.

Added regression tests using ref tests since getSelection() doesn't expose any node inside a shadow tree.

* editing/selection/selection-across-shadow-boundaries-mixed-editability-1-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-1.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-2-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-2.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-3-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-3.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-4-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-4.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-5-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-mixed-editability-5.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-1-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-1.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-2-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-2.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-3-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-readonly-3.html: Added.
* editing/selection/selection-across-shadow-boundaries-user-select-all-1-expected.html: Added.
* editing/selection/selection-across-shadow-boundaries-user-select-all-1.html: Added.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@236519 268f45cc-cd09-0410-ab3c-d52691b4dbfc
@web-padawan
Copy link

@web-padawan web-padawan commented Oct 4, 2018

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
Copy link

@aadamsx aadamsx commented Oct 4, 2018

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

@yoichio
Copy link

@yoichio yoichio commented Oct 5, 2018

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

@yoichio
Copy link

@yoichio yoichio commented Oct 19, 2018

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
Copy link
Collaborator

@rniwa rniwa commented Oct 25, 2018

Rough consensus at TPAC F2F:

  • Add getComposedRange, which takes a list of closed trees, to Selection.
  • Instead of adding a new setBaseAndExtent variant, update the existing methods to support setting selection which crosses shadow boundaries.

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

@smaug----
Copy link

@smaug---- smaug---- commented Oct 25, 2018

and getComposedRange would return a StaticRange

@mfreed7
Copy link

@mfreed7 mfreed7 commented Oct 6, 2021

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 (#79 (comment)) 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
Copy link
Collaborator

@rniwa rniwa commented Oct 6, 2021

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
Copy link

@mfreed7 mfreed7 commented Oct 6, 2021

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
Copy link
Collaborator

@rniwa rniwa commented Oct 7, 2021

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
Copy link

@web-padawan web-padawan commented Oct 7, 2021

My 2 cents as a maintainer of Quill fork which has to deal with Safari encapsulation (see vaadin/quill#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
Copy link

@mfreed7 mfreed7 commented Oct 12, 2021

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
Copy link
Collaborator

@rniwa rniwa commented Oct 14, 2021

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
Copy link

@web-padawan web-padawan commented Oct 14, 2021

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
Copy link

@sspi sspi commented Oct 14, 2021

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
Copy link

@mfreed7 mfreed7 commented Oct 18, 2021

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
Copy link
Collaborator

@rniwa rniwa commented Oct 18, 2021

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

@mfreed7
Copy link

@mfreed7 mfreed7 commented Oct 18, 2021

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
Copy link

@mfreed7 mfreed7 commented Oct 18, 2021

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
Copy link
Collaborator

@rniwa rniwa commented Oct 19, 2021

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
Copy link
Collaborator

@annevk annevk commented Oct 19, 2021

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
Copy link

@boazsender boazsender commented Oct 19, 2021

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
Copy link

@web-padawan web-padawan commented Oct 19, 2021

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 #79 (comment)

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
Copy link

@mfreed7 mfreed7 commented Oct 19, 2021

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 #79 (comment), but also the ones listed in the explainer.

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

Successfully merging a pull request may close this issue.

None yet