Re: [whatwg/dom] Add definition for composed selection range (PR #1342)

@domfarolino approved this pull request.

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](https://whatpr.org/dom/1342.html#:~:text=If%20selection%20range%20is%20not%20null%20and%20range%E2%80%99s%20start%20node%E2%80%99s%20shadow%2Dincluding%20root%20is%20not%20node%E2%80%99s%20shadow%2Dincluding%20root%2C%20set%20selection%20range%E2%80%99s%20end%20to%20bp). 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](https://whatpr.org/dom/1342.html#:~:text=Otherwise%2C%20if%20bp%20is%20after%20the%20range,selection%20range%E2%80%99s%20end%20to%20bp). Implications:
    ```js
    // 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.

> @@ -8265,41 +8296,95 @@ steps:
  <a>boundary point</a>
  (<var>node</var>, <var>offset</var>).
 
+ <li>If <var>range</var> is the <a for="composed selection range">legacy selection range</a> of a
+ <a>composed selection range</a>, set <var>selection range</var> to that
+ <a>composed selection range</a>.
+
+ <li>Otherwise, set <var>selection range</var> to null.

I think I would opt for the model:

1. Let _selection range_ be null.
2. If foo, then set _selection range_ to bar.

That way we're not "setting" the variable before we declare its existence with "let".

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/1342#pullrequestreview-2683197438
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/dom/pull/1342/review/2683197438@github.com>