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

Should a Range object be able to update a Selection object #772

Open
annevk opened this issue Jul 4, 2019 · 8 comments
Open

Should a Range object be able to update a Selection object #772

annevk opened this issue Jul 4, 2019 · 8 comments
Labels
interop Implementations are not interoperable with each other topic: ranges

Comments

@annevk
Copy link
Member

annevk commented Jul 4, 2019

https://mozilla.pettay.fi/moztests/selection_dynamic.html demonstrates that in Chrome and Firefox you can update a Selection object through a Range object. Safari does not support this (and is in line with the DOM Standard as Range objects do not have that capability in theory).

cc @rniwa @smaug---- @foolip @tkent-google @mbrodesser

@annevk annevk added interop Implementations are not interoperable with each other topic: ranges labels Jul 4, 2019
@smaug----
Copy link
Collaborator

If selection does not react to the changes to range object(s), the API becomes rather weird. getRangetAt returning a mutable object, but after changes selection and the relevant range object wouldn't correlate in any way.

@rniwa
Copy link
Collaborator

rniwa commented Jul 4, 2019

This is covered by the selection API specification:
http://w3c.github.io/selection-api/#dom-selection-getrangeat

I'd consider WebKit's behavior as a bug. There is a long standing WebKit bug. Blink fixed this after forking WebKit.

@annevk
Copy link
Member Author

annevk commented Jul 5, 2019

I don't think it's covered that mutating the range affects the selection as that'd require a pointer from the range to the selection and some kind of update steps.

@rniwa
Copy link
Collaborator

rniwa commented Jul 7, 2019

http://w3c.github.io/selection-api/#definition

Once a selection is associated with a given range, it must continue to be associated with that same range until this specification requires otherwise.

This is the statement which is meant to implicitly convey that message. Specifically, the note right beneath it says:

For instance, if the DOM changes in a way that changes the range's boundary points, or a script modifies the boundary points of the range, the same range object must continue to be associated with the selection. However, if the user changes the selection or a script calls addRange(), the selection must be associated with a new range object, as required elsewhere in this specification.

Honestly, I don't think we don't really need every API to be defined algorithmically. The declarative statement like this works in this case although I admit this statement as is quite vague and could benefit from a more precise definition.

@annevk
Copy link
Member Author

annevk commented Jul 8, 2019

I think it's rather problematic that when looking at the Range object definition you cannot tell it might be associated with a Selection object. It also creates ambiguity, does cloneRange() copy this pointer or drop it?

@dizhang168
Copy link

Letting a Range object update a Selection becomes trickier when crossing shadow trees. Here is an example:

<html>
<body>
<div id="light">Start outside shadow DOM</div>
<div id="outerHost">outerHost
 <template shadowrootmode="open">
   <slot></slot>
   <div id="innerHost">innerHost
    <template shadowrootmode="open">
      <slot></slot>
    </template>
   </div>
 </template>
</div>

<script>
selection = getSelection();
outerHost = document.getElementById('outerHost')
outerRoot = outerHost.shadowRoot;
innerHost = outerRoot.getElementById('innerHost');

// Step 1
selection.setBaseAndExtent(light.firstChild, 10, innerHost.firstChild, 5);

// Step 2
range = selection.getRangeAt(0);
range.setEnd(innerHost.firstChild, 6);
</script>

</body>
</html>

After step 1,

  • Composed range should be at start{light.firstChild, 10} and end{innerHost.firstChild, 5}
  • Live range is collapsed because it is across tree scopes, at start{innerHost.firstChild, 5} and end{innerHost.firstChild, 5}

After step 2,

  • Live range is changed to start{innerHost.firstChild, 5} and end{innerHost.firstChild, 6}

  • Composed range should either:

    • A) be updated to same as live range, start{innerHost.firstChild, 5} and end{innerHost.firstChild, 6}
    • B) only update the setEnd, it should be at start{light.firstChild, 10} and end{innerHost.firstChild, 6}
    • C) does not update composed range. It is still at start{light.firstChild, 10} and end{innerHost.firstChild, 5}

Currently, Blink is following option A and Gecko is following option C. Webkit I believe follows C, although it seems to have a bug with its getRangeAt(0) throwing IndexSizeError.

@annevk
Copy link
Member Author

annevk commented Nov 25, 2024

Do we have reasonable test coverage to ensure that mutation of the live range through the API updates the selection without shadow trees? And is that interoperable? (I suspect it is, but it would be good to confirm that is indeed our baseline.)

Now, when you manipulate the live range I think we have to manipulate the selection (and therefore the live composed range) as well, as otherwise you could detect the presence of a shadow root. There might be some ways to do that anyway, but in general I think it's a good rule of thumb to try and avoid doing that and let decisions flow from that principle.

However, this presents an interesting case. While the live range now knows about a shadow tree, does that mean that window.getSelection() should too? I guess that's okay as telling this particular range about a shadow tree essentially destroys its encapsulation due to getRangeAt(). So Selection APIs can probably stay in sync with getRangeAt() in that respect and sometimes reveal shadow trees if the live range does so too.

But what does that mean for the live composed range? C seems weird. The API says the selection is bigger than it is presented to the end user with no benefit to encapsulation. B seems reasonable, though I wonder what the Selection API returns? The same as getRangeAt()? A also seems reasonable. After all, if you wanted to update the selection you really shouldn't be using the Range object to do so.

@dizhang168
Copy link

dizhang168 commented Nov 27, 2024

Yes, there are extensive mutation WPT tests and all browsers are passing 100%. Note all these don't test shadow trees cases.

I agree. The spec and implementation says the live range should be reflected on the selection. We should make decisions with that as a guiding principle. I also think option A is reasonable. Users modifying a live range should only be able to modify within its tree scope. Beyond that, they should consider using Selection's collapse() and extend().

B seems reasonable, though I wonder what the Selection API returns? The same as getRangeAt()?

Yes, the same as getRangeAt(). For this example, both Blink and Gecko has:
The live range has start{innerHost.firstChild, 5} and end{innerHost.firstChild, 6}
The Selection has anchor{innerHost.firstChild, 5} and focus{innerHost.firstChild, 6}

aarongable pushed a commit to chromium/chromium that referenced this issue 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 <siliu@microsoft.com>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1411209}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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 <siliu@microsoft.com>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1411209}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue 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 <siliu@microsoft.com>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1411209}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jan 28, 2025
…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 <siliu@microsoft.com>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1411209}

--

wpt-commits: e679be39aa83e65a06627a8a5b911648f5312f13
wpt-pr: 50283
i3roly pushed a commit to i3roly/firefox-dynasty that referenced this issue Jan 28, 2025
…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 <siliu@microsoft.com>
Commit-Queue: Di Zhang <dizhangg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1411209}

--

wpt-commits: e679be39aa83e65a06627a8a5b911648f5312f13
wpt-pr: 50283
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
interop Implementations are not interoperable with each other topic: ranges
Development

No branches or pull requests

4 participants