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

Add definition for composed live range #1342

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

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

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

dom.bs Outdated
Comment on lines 3032 to 3045
<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>).
Copy link
Member

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

@dizhang168 dizhang168 Jan 21, 2025

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

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

Copy link
Author

Choose a reason for hiding this comment

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

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.

dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated Show resolved Hide resolved
dom.bs Outdated
<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>
Copy link
Member

Choose a reason for hiding this comment

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

Why is this called cached live range?

Copy link
Author

Choose a reason for hiding this comment

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

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.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe "legacy selection range"?

@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

aarongable pushed a commit to chromium/chromium that referenced this pull request Jan 24, 2025
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}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 25, 2025
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}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request Jan 25, 2025
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}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agenda+ To be discussed at a triage meeting
Development

Successfully merging this pull request may close these issues.

3 participants