Re: [whatwg/dom] https://dom.spec.whatwg.org/#dom-range-deletecontents may collapse the Range to an invalid position (Issue #1446)

stevenobiajulu left a comment (whatwg/dom#1446)

## Proposed fix: move range collapse before mutations

Both `deleteContents()` and `extract()` compute `(newNode, newOffset)` before mutations but apply them after — by which point script running during `remove` (via removing steps) may have invalidated those values.

### The fix

Move the "set range's start and end to (newNode, newOffset)" step from **after** mutations to **before** mutations. The DOM's built-in live range maintenance mechanisms — `live range pre-remove steps` and `replace data` range adjustments — will automatically keep the range valid through all subsequent operations.

### Why this is correct

1. **Mutation steps never read the range**: Steps after collapse use saved variables (`originalStartNode`, `nodesToRemove`, etc.), not the range itself.

2. **`newNode` survives all removals**: `nodesToRemove` contains only "contained" nodes. Start/end nodes are never contained, nor is the common ancestor. So `newNode` (either the start ancestor or common ancestor) is never removed.

3. **Pre-remove offset arithmetic is correct for multi-node removal**: The `live range pre-remove steps` rule is "decrement offset if `offset > removedNode.index`" (strict `>`). When nodes are removed at indices ≥ `newOffset`, the offset stays unchanged:
   - **Case 2** (different ancestors): `[R, N1, N2, N3, E]`, `newOffset=1`. Remove N1 at index 1: `1 > 1` → false. Remove N2 (now index 1): false. Remove N3 (now index 1): false. Final offset = 1. ✓
   - **Case 1** (start is ancestor): `(div,2)→(div,5)`. Remove at index 2 three times: `2 > 2` → false each time. Final offset = 2. ✓

4. **`replace data` doesn't disturb the early-collapsed range**: In Case 1, `newNode == originalStartNode` and the collapsed offset equals the replace offset; replace-data adjustments use strict `>`, so no movement. In Case 2, `newNode` is an Element ancestor (not the CharacterData node being modified), so adjustments don't apply.

5. **`extract()` append path handled**: `append(contained child)` → `pre-insert` → `insert` → `adopt` → `remove` → `live range pre-remove steps`. Same maintenance mechanism.

### `extract()` has the same bug

The comment at the current step 16 already acknowledges this: _"Now we start with mutations, so we can't refer to this anymore unless we carefully consider how it will have mutated."_ The fix is the same — move collapse before mutations.

### Testing caveat

No current HTML removing step fires synchronous script during `remove` (popover, iframe, dialog, slot removing steps all avoid it), so this is a **preventive spec-correctness fix**. WPT tests focus on conformance/non-regression, verifying the collapsed range position matches expected values.

Spec PR incoming with the change + WPT tests.

-- 
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/1446#issuecomment-3948761384
You are receiving this because you are subscribed to this thread.

Message ID: <whatwg/dom/issues/1446/3948761384@github.com>