Skip to content

Conversation

@dizhang168
Copy link

@dizhang168 dizhang168 commented Dec 17, 2024

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.

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


Preview | Diff

@domfarolino domfarolino self-requested a review December 19, 2024 18:40
@domfarolino domfarolino added the agenda+ To be discussed at a triage meeting label Jan 16, 2025
Copy link
Member

@annevk annevk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@dizhang168
Copy link
Author

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

@dizhang168 dizhang168 changed the title Add definition for composed live range Add definition for composed selection range Jan 28, 2025
@past past removed the agenda+ To be discussed at a triage meeting label Jan 30, 2025
dom.bs Outdated

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, but I don't still like it. It is not legacy, in the sense that there would be some better thing to replace it or so (in case of light dom). "Legacy" doesn't explain what the range is for.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What would you propose?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe the legacy selection range could be same-tree selection range, and composed selection range could be composed-tree selection range?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same-tree selection range sounds kinda nice. I personally think "composed selection range" is fine as-is.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listing the naming options:

  1. "composed live range" and "cached live range"
  2. "composed selection range" and "legacy selection range"
  3. "composed-tree selection range" and "same-tree selection range"
  4. "composed selection range" and "same-tree selection range"
  5. "composed selection range" and "selection range"

All these names seem ok to me, I will let the majority decide.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on an email conversation I'm a bit less sure what the semantics of this range are. Say you have an element with a shadow root that contains the word "Test". The end user selects "es". getSelection() should not provide access to this shadow root, but apparently the set of changes proposed here do not enforce that. Or am I misunderstanding something?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created 2 issues to track the above conversation:
#1363 for the naming choices
#1362 for getSelection() still giving access to the shadow root
Lets discuss these topics at the upcoming Editing WG or WHATNOT meeting.

Copy link
Member

@domfarolino domfarolino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this PR looks good; I'll let Anne comment on editorial/wrapping stuff, as the wrapping seems odd, but it was odd before the PR anyways. But I want to just highlight two interesting implications of this PR, for implementers like @sefeng211 to weigh in on.

The meat of this PR is the modification to "set the start"/"set the end". There are no semantic changes to legacy live ranges returned by getRangeAt(0)—the only change is that when "legacy uncomposed range"s are updated, we propagate the changes up to its "composed selection range", maintained by the Selection API. For example, when we "set the start" of a "legacy uncomposed range", we always set its "composed selection range" start too; the same goes for end. That's not too interesting. But there are two interesting implications around collapsing that we should agree on.

  1. When we "set the start" of a "legacy uncomposed range" across shadow boundaries, the "legacy uncomposed range" collapses to the new start boundary point—that's all fine as it predates this PR. However, this PR DOES NOT collapse its associated "composed selection range", as long as everything is same-document. This is exercised by this line in the spec. Firefox currently collapses the composed selection range in this case, going against this PR, so we should confirm with @sefeng211 if this PR is acceptable and Firefox can change their behavior. @dizhang168 is working on a WPT for this case now, but you can see an example of this scenario in https://gist.github.com/domfarolino/92d04f5e517da13971f69171476043d0.

  2. When we "set the start" of a "legacy uncomposed range" in the same root, but in a way that inverts its start/end, the "legacy uncomposed range" collapses to the new start—that behavior predates this PR too. But per this PR, we ALSO collapse its associated "composed selection range" at the new start boundary point. This is done by this line in the spec. Implications:

    // The composed range is not collapsed, but getRangeAt(0) *is* (inside shadow).
    getSelection().setBaseAndExtent(lightDom, 0, shadowText, 5);
    
    // This inverts the legacy uncomposed range, re-collapsing it at offset `10`.
    // It ALSO collapses the *composed* selection range.
    getSelection().getRangeAt(0).setStart(shadowText, 10);

All of this is fine to me, but since these are the tricker parts of this PR I just want to highlight them here. @sefeng211 + @annevk, how do Firefox / WebKit feel about these implications? Again, this is not about how getRangeAt(0) behaves after these options—it's about how changes to that range collapse or do not collapse the composed selection range. If there are strong preferences on either of the two cases here, we'd love to know.

@sefeng211
Copy link

However, this PR DOES NOT collapse its associated "composed selection range", as long as everything is same-document

Di changed the PR to use shadow-including root rather than comparing the document, I think we now also collapse the composed selection range which is correct IMO.

When we "set the start" of a "legacy uncomposed range" in the same root, but in a way that inverts its start/end, the "legacy uncomposed range" collapses to the new start—that behavior predates this PR too. But per this PR, we ALSO collapse its associated "composed selection range" at the new start boundary point

This also looks fine to me.

@domfarolino
Copy link
Member

Di changed the PR to use shadow-including root rather than comparing the document, I think we now also collapse the composed selection range which is correct IMO.

But shadow-including root will climb up to the document, if the node is connected. So I don't think there's a difference here. I believe the spec does not collapse in this case, because it is basically doing a same-document comparison check.

@sefeng211
Copy link

But shadow-including root will climb up to the document, if the node is connected. So I don't think there's a difference here. I believe the spec does not collapse in this case, because it is basically doing a same-document comparison check

Yeah, I mean with this we collapse if one of the nodes is disconnected which is something the PR didn't do before.

@domfarolino
Copy link
Member

Yeah, I mean with this we collapse if one of the nodes is disconnected which is something the PR didn't do before.

OK, got it. So just to be clear, the behavior of NOT collapsing the composed selection range in the first case I mention is fine with you? I ask only because it looks like Firefox DOES collapse the composed selection range in that case, which I think would fail the incoming WPT being written.

@sefeng211
Copy link

OK, got it. So just to be clear, the behavior of NOT collapsing the composed selection range in the first case I mention is fine with you? I ask only because it looks like Firefox DOES collapse the composed selection range in that case, which I think would fail the incoming WPT being written.

I think we are on the same page but let me reiterate. You mean that we don't collapse the composed selection range as long as both nodes are connected right? I am fine with this. If Firefox collapses in case then I think it's a Firefox bug...

@domfarolino
Copy link
Member

You mean that we don't collapse the composed selection range as long as both nodes are connected right?

Right. The spec collapses the live range (the getRangeAt(0) range) but NOT the composed selection range.

If Firefox collapses in case then I think it's a Firefox bug...

Great to know! Yeah Di is adding a test for this in https://crrev.com/c/6354926 to assert the spec behavior, so hopefully that helps. Thanks a lot!

@dizhang168
Copy link
Author

Hi @domfarolino @sefeng211! I have updated the tests to test out the "set the start/end" algorithm, as discussed in the conversation above.
I have also updated this PR to simplify the algorithm by having the composed selection range's checks in a scoped if condition. Part of it includes adding a new composed position definition. PTAL, thank you.

@sefeng211
Copy link

Thanks, LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

6 participants