Add definition for composed selection range #1342

dizhang168 · 2024-12-17T23:55:59Z

As discussed at TPAC 2024, the specification for the new getComposedRanges() API need the introduction of the new definition "Composed live range" [1], which should react to mutations. This PR attempts to define that new concept. Once landed, we will add the spec language that uses composed live range in the Selection API. An overview of the spec changes necessary can be found at [2].

[1] w3c/selection-api#2 (comment)
[2] https://github.com/dizhang168/shadow-dom-selection/blob/main/selection-api-spec-changes.md

01/28/2025 Edit: Changed naming to Composed Selection Range.

At least two implementers are interested (and none opposed):
- Chromium: Positive
- Webkit: Positive (Selection API: getComposedRanges mozilla/standards-positions#1055)
- Gecko: Positive (Selection API: getComposedRanges WebKit/standards-positions#376)
Tests are written and can be reviewed and commented upon at:
- This PR introduces an internal concept as a Composed Live Range is not exposed directly to the user
- getComposedRanges() function tests can be found at: https://wpt.fyi/results/selection/shadow-dom/tentative?label=experimental&label=master&aligned
Implementation bugs are filed:
- Chromium: https://issues.chromium.org/issues/40286116
- Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1430308
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=216325
MDN issue is filed: …
The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)

Preview | Diff

annevk

@smaug---- you should review this as well in due course. And @rniwa too I suspect.

annevk · 2025-01-17T09:12:08Z

dom.bs

+ <li><p>For each <a>composed live range</a> whose <a for=range>start node</a> is a
+ <a>shadow-including inclusive descendant</a> of <var>node</var>, set its <a for=range>start</a> to
+ (<var>parent</var>, <var>index</var>).
+
+ <li><p>For each <a>composed live range</a> whose <a for=range>end node</a> is an
+ <a>shadow-including inclusive descendant</a> of <var>node</var>, set its <a for=range>end</a> to
+ (<var>parent</var>, <var>index</var>).


This will need to be rebased once we land moveBefore().

However, I also think this needs to be reconciled with the "for each live range" above as we don't want to do duplicate work.

Shouldn't this update the cached live range as well?

The cached live range is covered by the existing "For each live range ..." rules. The cached live range is not composed and hence shouldn't consider for shadow-including inclusive descendant.

Good call on the cached live range. But don't the existing rules also update the "composed live range" (because a composed live range is a live range)?

Yes, the rule will be applied twice. I think that's the expected behavior. This will help to keep mutation results consistent, especially when we are not crossing shadow boundaries.

I'm not sure I understand. Can you elaborate in which case it helps? It just looks inefficient to me.

Hum, I think my original statement that the rule should be applied twice is wrong. Suppose the document stores a list of all live ranges, including cached live range. For each of them, it will update per mutation rule. Then if it is a cached live range and hence, attached to a composed live range, it will update the composed live range per the "set the start or end" algorithm redefined in this PR. If so, the composed live range do not need be updated again by the mutation, as it is already up to date.

Now, to avoid inefficiency, we could change every "For each live range" to "For each {{Range}} object".

Note that the "set the start or end" algorithm is not invoked from #concept-node-remove at all, so it is unrelated. So with that out of the way, it seems that for all composed ranges that are ordinary inclusive descendants of the to-be-removed node, these composed ranges would get updated "twice" (idempotently): first by steps 4/5, second by steps 6/7. In the case where the composed range is a shadow-inclusive descendant of the removed node but not an ordinary inclusive descendant, than only steps 6/7 will apply, and there's no redundancy.

So it's inefficient as @annevk points out, but not broken. Should we fix this? Does it matter?

I think we should fix it as it's somewhat confusing. We can exclude the composed ranges from the first set of checks.

dom.bs

annevk · 2025-01-17T09:15:51Z

dom.bs

+<p>A <dfn export id=concept-composed-live-range>composed live range</dfn> is a <a>live range</a>
+that has one associated {{Range}} object - <dfn export
+id=concept-composed-live-range-cached-live-range for="composed live range">cached live
+range</dfn>.</p>


Why is this called cached live range?

I wanted to give it a name so it doesn't get confusion between the "composed" live range and its associated "live range". Further, it is cached within the composed live range. I am open to naming suggestions.

Maybe "legacy selection range"?

I think "legacy selection range" sounds good, to make clear that it's only maintained for behavior expected by the Selection API. I might also go further and rename "composed live range" to "composed selection range" or something, to also indicate that this concept only exists for Selection API purposes.. thoughts?

I renamed them to "composed selection range" and "legacy selection range".

dizhang168 · 2025-01-24T21:56:46Z

I have opened the draft PR for Selection API to use the definition described here, to help visualize:
w3c/selection-api#345

When a live range attached to a document is modified, this change is upstreamed to the FrameSelection. New spec [1] says to only update the composed live range (frame selection)'s start position if setStart is called and only update end position if setEnd is called: whatwg/dom#1342 This is the proposal (B) discussed here: whatwg/dom#772 (comment) To do this, we define enum UpdateSelectionIfAddedToSelection to have three possible update selection behavior: 1. kAll, set selection to have the same start and end as range. --> Default case, when both setStart, setEnd are called. 2. kStartOnly, set selection to have the same start as range only. --> When only setStart is called. 3. kEndOnly, set selection to have the same end as range only. --> When only setEnd is called. We add a WPT test for this new behavior, which only affects the output of getComposedRanges() as it is the only API that accesses the frame selection's endpoints directly. Change-Id: I51ea53fe6156164ba3fbe38b14bc47ff502633b1 Bug: 40286116 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6188157 Reviewed-by: Siye Liu <[email protected]> Commit-Queue: Di Zhang <[email protected]> Cr-Commit-Position: refs/heads/main@{#1411209}

…dateSelectionIfAddedToSelection, a=testonly Automatic update from web-platform-tests Add UpdateSelectionBehavior to Range::UpdateSelectionIfAddedToSelection When a live range attached to a document is modified, this change is upstreamed to the FrameSelection. New spec [1] says to only update the composed live range (frame selection)'s start position if setStart is called and only update end position if setEnd is called: whatwg/dom#1342 This is the proposal (B) discussed here: whatwg/dom#772 (comment) To do this, we define enum UpdateSelectionIfAddedToSelection to have three possible update selection behavior: 1. kAll, set selection to have the same start and end as range. --> Default case, when both setStart, setEnd are called. 2. kStartOnly, set selection to have the same start as range only. --> When only setStart is called. 3. kEndOnly, set selection to have the same end as range only. --> When only setEnd is called. We add a WPT test for this new behavior, which only affects the output of getComposedRanges() as it is the only API that accesses the frame selection's endpoints directly. Change-Id: I51ea53fe6156164ba3fbe38b14bc47ff502633b1 Bug: 40286116 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6188157 Reviewed-by: Siye Liu <[email protected]> Commit-Queue: Di Zhang <[email protected]> Cr-Commit-Position: refs/heads/main@{#1411209} -- wpt-commits: e679be39aa83e65a06627a8a5b911648f5312f13 wpt-pr: 50283

dom.bs

domfarolino · 2025-01-28T19:02:07Z

dom.bs

+<p>A <dfn export id=concept-live-range>live range</dfn> is a <a>range</a> that is affected by
+mutations to the <a>node tree</a>.</p>
+
+<p>Objects implementing the {{Range}} interface are <a>live ranges</a>.


Before this PR, we have the following layout of definitions:

"range" == AbstractRange IDL object

"live range" == Range IDL object

After this PR, we have:

"range" == AbstractRange IDL object (no changes)

"live range" == any "range" (AbstractRange) responsive to mutations

"live range" includes Range IDL objects

"live range" includes "composed live range" AbstractRange IDL

So what we've done is make "live range" now include AbstractRange objects. I think you've done this to avoid making "composed live range" a full-blown Range IDL object, since it doesn't need to be one—as we discussed, implementations appropriately use some internal non-IDL object to represent a "composed live range". But even with your changes I think "composed live range" is still an IDL object—it's just an AbstractRange instead of a Range.

So I think the definition rejiggering in this PR is kinda half way in between two solutions. We can either:

Stick to our guns that "composed live range" should NOT be an IDL object, and do what Fetch does where we have IDL objects like Request and also "internal" spec concepts like #concept-request. In our case we'd have AbstractRange and then an internal #concept-range-like struct. "composed live range" would be-a #concept-range, and not an IDL object. It would likely hold a "cached" Range inside.

Decide we're OK with "composed live range" being an IDL object in the spec, even though it never is one in implementation, nor is it ever web-exposed. In that case, we can either keep it as an AbstractRange IDL object which it is in this PR. OR we can simplify this PR a bit and leave the definition of "live range" alone—then we'd make "composed live range" a Range IDL object that just so happens to never be exposed to script directly. That feels weird since there isn't a reason for "composed live range" to be an IDL object besides it would make this PR smaller/easier.

The details of these options are discussed in w3c/selection-api#2 (comment) and w3c/selection-api#2 (comment), and I'd love @annevk's thoughts.

Redefining live range seems potentially problematic: https://dontcallmedom.github.io/webdex/l.html#live%20ranges%40%40dom%25%25dfn

We'd also have to go through all callers in the DOM specification to be sure this doesn't break anything.

The only references to "live ranges" are from Selection API and Highlight API. In both case, they mean to refer to Range object. We can update those references, although maybe it would be simpler to keep the current live ranges definition and change "composed live range" to:

A composed live range is a range that is affected by mutations to the node tree and has an associated live range called a legacy live range.

dom.bs

…dateSelectionIfAddedToSelection, a=testonly Automatic update from web-platform-tests Add UpdateSelectionBehavior to Range::UpdateSelectionIfAddedToSelection When a live range attached to a document is modified, this change is upstreamed to the FrameSelection. New spec [1] says to only update the composed live range (frame selection)'s start position if setStart is called and only update end position if setEnd is called: whatwg/dom#1342 This is the proposal (B) discussed here: whatwg/dom#772 (comment) To do this, we define enum UpdateSelectionIfAddedToSelection to have three possible update selection behavior: 1. kAll, set selection to have the same start and end as range. --> Default case, when both setStart, setEnd are called. 2. kStartOnly, set selection to have the same start as range only. --> When only setStart is called. 3. kEndOnly, set selection to have the same end as range only. --> When only setEnd is called. We add a WPT test for this new behavior, which only affects the output of getComposedRanges() as it is the only API that accesses the frame selection's endpoints directly. Change-Id: I51ea53fe6156164ba3fbe38b14bc47ff502633b1 Bug: 40286116 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6188157 Reviewed-by: Siye Liu <[email protected]> Commit-Queue: Di Zhang <[email protected]> Cr-Commit-Position: refs/heads/main@{#1411209} -- wpt-commits: e679be39aa83e65a06627a8a5b911648f5312f13 wpt-pr: 50283

… start/end algorithm

domfarolino · 2025-01-30T17:52:33Z

dom.bs

+
+     <li>If <var>selection range</var> is not null,
+     set <var>selection range</var>’s
+     <a for="range">start</a> to <var>bp</var>.


This is interesting because we're always propagating changes from the internal live range up to the composed selection range. Imagine a scenario where the composed selection range's start is in the light DOM while its end is in the shadow DOM—getSelection().setBaseAndExtent(light, 0, shadow, 0). In this case, the composed selection range's legacy live range will be collapsed in the shadow DOM. I think what we have here in the spec is fine, but it's worth pointing out that moving the internal live range's start node around in the shadow dom will drag the composed live range's start node down into the shadow DOM with it, which could be surprising to people that would expect the composed selection range to not change shadow roots if it doesn't have to (i.e., some might expect the two ranges to move freely with respect to each other).

I think the model we've landed on here is the simplest, but I just wanted to point it out since this case was mentioned by schenney on the WHATNOT call.

smaug---- · 2025-02-19T17:03:18Z

dom.bs

+
+<p>A <dfn export id=concept-composed-selection-range>composed selection range</dfn> is a
+<a>live range</a> that has an associated {{Range}} object, a
+<dfn export id=concept-legacy-selection-range for="composed selection range">legacy selection range</dfn>.</p>


Nit, I don't like calling the normal Range as legacy. Nothing really legacy there.

It's not the Range object that's legacy, it's having to keep around a pretend range for the purposes of the selection API as it was designed pre-shadow-trees that's kinda legacy.

sefeng211 · 2025-02-19T17:09:16Z

dom.bs

+
+       <li>If <var>selection range</var> is not null,
+       set <var>selection range</var>’s
+       <a for="range">end</a> to <var>bp</var>.


It looks like we only collapse the composed selection range when the nodes are in the same tree. Shouldn't we also compare the nodes if they are in different tree (with a new algorithm like compose-after), and collapse the composed selection range if the new start is compose-after the existing end? This also applies to the setting the end case.

domfarolino self-requested a review December 19, 2024 18:40

domfarolino added the agenda+ To be discussed at a triage meeting label Jan 16, 2025

past mentioned this pull request Jan 16, 2025

Upcoming WHATNOT meeting on 2025-01-16 whatwg/html#10906

Closed

annevk reviewed Jan 17, 2025

View reviewed changes

dizhang168 added 2 commits January 21, 2025 17:20

Add definition for composed live range

7c48fa5

Review, format changes

487cac0

dizhang168 force-pushed the composed-live-range branch from 54e6984 to 487cac0 Compare January 22, 2025 01:21

annevk mentioned this pull request Jan 23, 2025

Upcoming WHATNOT meeting on 2025-01-23 whatwg/html#10923

Closed

Fix collapse logic for set the start or end

461aed7

dizhang168 mentioned this pull request Jan 24, 2025

Change Selection to be associated with a composed live range w3c/selection-api#345

Draft

8 tasks

Fix for= typo

413348d

chromium-wpt-export-bot mentioned this pull request Jan 25, 2025

Add UpdateSelectionBehavior to Range::UpdateSelectionIfAddedToSelection web-platform-tests/wpt#50283

Merged

domfarolino added 2 commits January 28, 2025 10:34

Linking nit

c7f38b1

Remove em dash to fix spacing in rendered spec

1f74a51

domfarolino reviewed Jan 28, 2025

View reviewed changes

Review changes: rename composed live range, improve format of set the…

f5b6935

… start/end algorithm

dizhang168 changed the title ~~Add definition for composed live range~~ Add definition for composed selection range Jan 28, 2025

domfarolino reviewed Jan 30, 2025

View reviewed changes

past removed the agenda+ To be discussed at a triage meeting label Jan 30, 2025

past mentioned this pull request Jan 30, 2025

Upcoming WHATNOT meeting on 2025-01-30 whatwg/html#10941

Closed

smaug---- reviewed Feb 19, 2025

View reviewed changes

sefeng211 reviewed Feb 19, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add definition for composed selection range #1342

Add definition for composed selection range #1342

dizhang168 commented Dec 17, 2024 •

edited by pr-preview bot

Loading

annevk left a comment

annevk Jan 17, 2025

annevk Jan 17, 2025

dizhang168 Jan 21, 2025 •

edited

Loading

annevk Jan 24, 2025

dizhang168 Jan 24, 2025

annevk Jan 25, 2025

dizhang168 Jan 27, 2025

domfarolino Jan 28, 2025

annevk Feb 13, 2025

annevk Jan 17, 2025

dizhang168 Jan 21, 2025

annevk Jan 24, 2025

domfarolino Jan 28, 2025

dizhang168 Jan 28, 2025

dizhang168 commented Jan 24, 2025

domfarolino Jan 28, 2025

annevk Feb 13, 2025

dizhang168 Feb 19, 2025

domfarolino Jan 30, 2025

smaug---- Feb 19, 2025

annevk Feb 24, 2025

sefeng211 Feb 19, 2025

Add definition for composed selection range #1342

Are you sure you want to change the base?

Add definition for composed selection range #1342

Conversation

dizhang168 commented Dec 17, 2024 • edited by pr-preview bot Loading

annevk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dizhang168 Jan 21, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dizhang168 commented Jan 24, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dizhang168 commented Dec 17, 2024 •

edited by pr-preview bot

Loading

dizhang168 Jan 21, 2025 •

edited

Loading