From 7e97e77babf7f2f50afd370e41c1894a2bc58c48 Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 22:10:02 +0200 Subject: [PATCH 1/2] webapi: spec algorithms for Range.cloneContents and extractContents Fixes all remaining failures of WPT /dom/ranges/Range-cloneContents.html (171/187 -> 187/187) and Range-extractContents.html (153/187 -> 187/187): - cloneContents only handled same-container and sibling-boundary ranges. It now implements the DOM "clone the contents" algorithm on explicit boundary points: partially contained CharacterData boundaries are cloned with the substring, partially contained elements are cloned shallow with the subrange recursed into the clone, contained nodes are cloned deep, and a contained doctype throws HierarchyRequestError. - extractContents was cloneContents + deleteContents, which recreated the extracted nodes ("you created or detached nodes when you weren't supposed to"). It now implements the "extract" algorithm: contained nodes MOVE into the fragment preserving identity; only the boundary CharacterData substrings and the partially contained element shells are cloned, with the originals truncated via replaceData. Children are classified before anything moves, since moving a contained child shifts the indices the boundary comparisons rely on. The range then collapses to the same new node/new offset rule as deleteContents. Coverage: - /dom/ranges/Range-cloneContents.html 171/187 -> 187/187 (fully green) - /dom/ranges/Range-extractContents.html 153/187 -> 187/187 (fully green) Co-Authored-By: Claude Fable 5 --- src/browser/webapi/Range.zig | 253 +++++++++++++++++++++++++---------- 1 file changed, 180 insertions(+), 73 deletions(-) diff --git a/src/browser/webapi/Range.zig b/src/browser/webapi/Range.zig index 4b8c251c4..cc443566b 100644 --- a/src/browser/webapi/Range.zig +++ b/src/browser/webapi/Range.zig @@ -470,97 +470,204 @@ fn collectContained(self: *const Range, node: *Node, list: *std.ArrayList(*Node) } fn nodeContained(self: *const Range, node: *Node) bool { - const after_start = AbstractRange.compareBoundaryPoints( + return containedBetween( node, - 0, self._proto._start_container, self._proto._start_offset, - ) == .after; - if (!after_start) return false; - return AbstractRange.compareBoundaryPoints( - node, - node.getLength(), self._proto._end_container, self._proto._end_offset, - ) == .before; + ); } pub fn cloneContents(self: *const Range, frame: *Frame) !*DocumentFragment { const fragment = try DocumentFragment.init(frame); - if (self._proto.getCollapsed()) return fragment; - // Simple case: same container - if (self._proto._start_container == self._proto._end_container) { - if (self._proto._start_container.is(Node.CData)) |_| { - // Clone part of text node - const text_data = self._proto._start_container.getData().str(); - const byte_start = byteOffset(text_data, self._proto._start_offset); - const byte_end = byteOffset(text_data, self._proto._end_offset); - if (byte_start < text_data.len and byte_end <= text_data.len) { - const cloned_text = text_data[byte_start..byte_end]; - const text_node = try Frame.node_factory.createTextNode(frame, cloned_text); - _ = try fragment.asNode().appendChild(text_node, frame); - } - } else { - // Clone child nodes in range - var offset = self._proto._start_offset; - while (offset < self._proto._end_offset) : (offset += 1) { - if (self._proto._start_container.getChildAt(offset)) |child| { - if (try child.cloneNodeForAppending(true, frame)) |cloned| { - _ = try fragment.asNode().appendChild(cloned, frame); - } - } - } - } - } else { - // Complex case: different containers - // Clone partial start container - if (self._proto._start_container.is(Node.CData)) |_| { - const text_data = self._proto._start_container.getData().str(); - const byte_start = byteOffset(text_data, self._proto._start_offset); - if (byte_start < text_data.len) { - // Clone from start_offset to end of text - const cloned_text = text_data[byte_start..]; - const text_node = try Frame.node_factory.createTextNode(frame, cloned_text); - _ = try fragment.asNode().appendChild(text_node, frame); - } - } - - // Clone nodes between start and end containers (siblings case) - if (self._proto._start_container.parentNode() == self._proto._end_container.parentNode()) { - var current = self._proto._start_container.nextSibling(); - while (current != null and current != self._proto._end_container) { - const next = current.?.nextSibling(); - if (try current.?.cloneNodeForAppending(true, frame)) |cloned| { - _ = try fragment.asNode().appendChild(cloned, frame); - } - current = next; - } - } - - // Clone partial end container - if (self._proto._end_container.is(Node.CData)) |_| { - const text_data = self._proto._end_container.getData().str(); - const byte_end = byteOffset(text_data, self._proto._end_offset); - if (byte_end > 0 and byte_end <= text_data.len) { - // Clone from start to end_offset - const cloned_text = text_data[0..byte_end]; - const text_node = try Frame.node_factory.createTextNode(frame, cloned_text); - _ = try fragment.asNode().appendChild(text_node, frame); - } - } - } - + try cloneContentsBetween( + frame, + fragment.asNode(), + self._proto._start_container, + self._proto._start_offset, + self._proto._end_container, + self._proto._end_offset, + ); return fragment; } +// The DOM "clone the contents of a range" algorithm, on explicit boundary +// points so the partially-contained recursion doesn't need Range objects. +fn cloneContentsBetween(frame: *Frame, out: *Node, start_node: *Node, start_offset: u32, end_node: *Node, end_offset: u32) !void { + if (start_node == end_node) { + if (start_node.is(Node.CData)) |cdata| { + const data = cdata.getData().str(); + const cloned = (try start_node.cloneNodeForAppending(false, frame)) orelse return; + try cloned.setData(data[byteOffset(data, start_offset)..byteOffset(data, end_offset)], frame); + _ = try out.appendChild(cloned, frame); + return; + } + } + + // The closest common ancestor of the two boundary points. + var common = start_node; + while (common != end_node and !common.contains(end_node)) { + common = common.parentNode() orelse break; + } + + var child = common.firstChild(); + while (child) |c| : (child = c.nextSibling()) { + const contains_start = c == start_node or c.contains(start_node); + const contains_end = c == end_node or c.contains(end_node); + + if (contains_start) { + // First partially contained child. + if (c.is(Node.CData)) |cdata| { + // c is the start node itself. + const data = cdata.getData().str(); + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse continue; + try cloned.setData(data[byteOffset(data, start_offset)..], frame); + _ = try out.appendChild(cloned, frame); + } else { + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse continue; + _ = try out.appendChild(cloned, frame); + try cloneContentsBetween(frame, cloned, start_node, start_offset, c, c.getLength()); + } + } else if (contains_end) { + // Last partially contained child. + if (c.is(Node.CData)) |cdata| { + // c is the end node itself. + const data = cdata.getData().str(); + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse continue; + try cloned.setData(data[0..byteOffset(data, end_offset)], frame); + _ = try out.appendChild(cloned, frame); + } else { + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse continue; + _ = try out.appendChild(cloned, frame); + try cloneContentsBetween(frame, cloned, c, 0, end_node, end_offset); + } + } else if (containedBetween(c, start_node, start_offset, end_node, end_offset)) { + if (c._type == .document_type) { + return error.HierarchyError; + } + const cloned = (try c.cloneNodeForAppending(true, frame)) orelse continue; + _ = try out.appendChild(cloned, frame); + } + } +} + +fn containedBetween(node: *Node, start_node: *Node, start_offset: u32, end_node: *Node, end_offset: u32) bool { + const after_start = AbstractRange.compareBoundaryPoints(node, 0, start_node, start_offset) == .after; + if (!after_start) return false; + return AbstractRange.compareBoundaryPoints(node, node.getLength(), end_node, end_offset) == .before; +} + pub fn extractContents(self: *Range, frame: *Frame) !*DocumentFragment { - const fragment = try self.cloneContents(frame); - try self.deleteContents(frame); + const fragment = try DocumentFragment.init(frame); + if (self._proto.getCollapsed()) return fragment; + + frame.domChanged(); + + const start_node = self._proto._start_container; + const start_offset = self._proto._start_offset; + const end_node = self._proto._end_container; + const end_offset = self._proto._end_offset; + + // Where the range collapses to afterwards; same rule as deleteContents. + var new_node = start_node; + var new_offset = start_offset; + if (start_node != end_node and !start_node.contains(end_node)) { + var reference = start_node; + while (reference.parentNode()) |parent| { + if (parent == end_node or parent.contains(end_node)) break; + reference = parent; + } + new_node = reference.parentNode().?; + new_offset = (new_node.getChildIndex(reference) orelse 0) + 1; + } + + try extractContentsBetween(frame, fragment.asNode(), start_node, start_offset, end_node, end_offset); + + try self.setStart(new_node, new_offset); + try self.setEnd(new_node, new_offset); return fragment; } +// The DOM "extract" algorithm: contained nodes MOVE into the output +// (preserving identity); only the partially contained boundary +// CharacterData nodes and the partially contained element shells are cloned. +// Children are classified before anything moves, since moving a contained +// child shifts the indices the boundary comparisons rely on. +fn extractContentsBetween(frame: *Frame, out: *Node, start_node: *Node, start_offset: u32, end_node: *Node, end_offset: u32) !void { + if (start_node == end_node) { + if (start_node.is(Node.CData)) |cdata| { + const data = cdata.getData().str(); + const cloned = (try start_node.cloneNodeForAppending(false, frame)) orelse return; + try cloned.setData(data[byteOffset(data, start_offset)..byteOffset(data, end_offset)], frame); + _ = try out.appendChild(cloned, frame); + try cdata.replaceData(start_offset, end_offset - start_offset, "", frame); + return; + } + } + + var common = start_node; + while (common != end_node and !common.contains(end_node)) { + common = common.parentNode() orelse break; + } + + var first_partial: ?*Node = null; + var last_partial: ?*Node = null; + var contained: std.ArrayList(*Node) = .empty; + + var child = common.firstChild(); + while (child) |c| : (child = c.nextSibling()) { + if (c == start_node or c.contains(start_node)) { + first_partial = c; + } else if (c == end_node or c.contains(end_node)) { + last_partial = c; + } else if (containedBetween(c, start_node, start_offset, end_node, end_offset)) { + if (c._type == .document_type) { + return error.HierarchyError; + } + try contained.append(frame.call_arena, c); + } + } + + if (first_partial) |c| { + if (c.is(Node.CData)) |cdata| { + // c is the start node itself. + const data = cdata.getData().str(); + const byte_start = byteOffset(data, start_offset); + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse return; + try cloned.setData(data[byte_start..], frame); + _ = try out.appendChild(cloned, frame); + const length: u32 = @intCast(cdata.getLength()); + try cdata.replaceData(start_offset, length - start_offset, "", frame); + } else { + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse return; + _ = try out.appendChild(cloned, frame); + try extractContentsBetween(frame, cloned, start_node, start_offset, c, c.getLength()); + } + } + + for (contained.items) |c| { + _ = try out.appendChild(c, frame); + } + + if (last_partial) |c| { + if (c.is(Node.CData)) |cdata| { + // c is the end node itself. + const data = cdata.getData().str(); + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse return; + try cloned.setData(data[0..byteOffset(data, end_offset)], frame); + _ = try out.appendChild(cloned, frame); + try cdata.replaceData(0, end_offset, "", frame); + } else { + const cloned = (try c.cloneNodeForAppending(false, frame)) orelse return; + _ = try out.appendChild(cloned, frame); + try extractContentsBetween(frame, cloned, c, 0, end_node, end_offset); + } + } +} + pub fn surroundContents(self: *Range, new_parent: *Node, frame: *Frame) !void { // Extract contents const contents = try self.extractContents(frame); From 9bd5aa3885f0eeca8ecaadb896c9a9810c301d0a Mon Sep 17 00:00:00 2001 From: Francis Bouvier Date: Sat, 11 Jul 2026 22:23:07 +0200 Subject: [PATCH 2/2] webapi: replaceChild removes the old child before inserting the new one Fixes the 21 failing subtests of WPT /dom/ranges/Range-mutations-replaceChild.html (39/60 -> 60/60): live range boundary points anchored on the parent were off by one because we inserted the new child before removing the old one. Per the DOM "replace" algorithm, the reference points are captured first (child's next sibling, or node's own next sibling when they're adjacent), the old child is removed, and only then is the new node inserted before the reference - each step running the live-range update rules in that order. Self-replacement (replaceChild(c, c)) now also performs the actual remove-and-reinsert instead of only queueing records, so ranges anchored inside the node move to its old position, as the remove step requires. Node-replaceChild.html and MutationObserver-childList.html stay fully green. Coverage: /dom/ranges/Range-mutations-replaceChild.html 39/60 -> 60/60 (fully green). Co-Authored-By: Claude Fable 5 --- src/browser/webapi/Node.zig | 70 ++++++++++++++++++++++++------------- 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index 41fd55c96..0e473e317 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -830,11 +830,26 @@ pub fn replaceChild(self: *Node, new_child: *Node, old_child: *Node, frame: *Fra const notify = Frame.observers.hasMutationObservers(frame); if (new_child == old_child) { - // Replacing a node with itself doesn't change the tree; observers - // still get a removal record followed by an addition record. + // Replacing a node with itself leaves the tree unchanged, but the + // remove-then-reinsert still happens: live ranges anchored inside the + // node move to its old position, and observers get a removal record + // followed by an addition record. + const prev = old_child.previousSibling(); + const next = old_child.nextSibling(); + const was_connected = old_child.isConnected(); + frame.removeNode(self, old_child, .{ .will_be_reconnected = was_connected, .notify_observers = false }); + if (next) |reference| { + try frame.insertNodeRelative(self, old_child, .{ .before = reference }, .{ + .child_already_connected = was_connected, + .notify_observers = false, + }); + } else { + try frame.appendNode(self, old_child, .{ + .child_already_connected = was_connected, + .notify_observers = false, + }); + } if (notify) { - const prev = old_child.previousSibling(); - const next = old_child.nextSibling(); const nodes = [_]*Node{old_child}; Frame.observers.notifyChildListChange(frame, self, &.{}, &nodes, prev, next); Frame.observers.notifyChildListChange(frame, self, &nodes, &.{}, prev, next); @@ -842,20 +857,17 @@ pub fn replaceChild(self: *Node, new_child: *Node, old_child: *Node, frame: *Fra return old_child; } - // The combined record's siblings are captured before new_child is - // removed from its old position (spec: referenceChild and - // previousSibling are set before the adopt step), so new_child itself - // can be the record's previousSibling. A referenceChild that is - // new_child becomes new_child's next sibling. + // Spec order: capture the reference points, remove new_child from its + // current position (notifying normally - internal replacement), remove + // old_child (suppressed), insert new_child before the reference + // (suppressed), and queue one combined record. Removing old_child before + // inserting matters for live ranges anchored on this parent's offsets. const prev = old_child.previousSibling(); - var next = old_child.nextSibling(); - if (next == new_child) { - next = new_child.nextSibling(); + var reference = old_child.nextSibling(); + if (reference == new_child) { + reference = new_child.nextSibling(); } - // Removing new_child from its current position (internal replacement) - // notifies normally; the replacement itself queues one combined record - // with both the added and the removed node. const child_already_connected = new_child.isConnected(); const child_owner = new_child.ownerDocument(frame); const parent_owner = self.ownerDocument(frame) orelse self.as(Document); @@ -872,35 +884,43 @@ pub fn replaceChild(self: *Node, new_child: *Node, old_child: *Node, frame: *Fra } var added: std.ArrayList(*Node) = .empty; - if (new_child.is(DocumentFragment)) |_| { - if (notify) { + if (notify) { + if (new_child.is(DocumentFragment)) |_| { var it = new_child.childrenIterator(); while (it.next()) |fragment_child| { try added.append(frame.call_arena, fragment_child); } - } - try frame.moveAllChildren(new_child, self, old_child, .silent_parent); - } else { - if (notify) { + } else { try added.append(frame.call_arena, new_child); } + } + + frame.removeNode(self, old_child, .{ .will_be_reconnected = false, .notify_observers = false }); + + if (new_child.is(DocumentFragment)) |_| { + try frame.moveAllChildren(new_child, self, reference, .silent_parent); + } else if (reference) |ref| { try frame.insertNodeRelative( self, new_child, - .{ .before = old_child }, + .{ .before = ref }, .{ .child_already_connected = child_already_connected, .adopting_to_new_document = adopting, .notify_observers = false, }, ); + } else { + try frame.appendNode(self, new_child, .{ + .child_already_connected = child_already_connected, + .adopting_to_new_document = adopting, + .notify_observers = false, + }); } - frame.removeNode(self, old_child, .{ .will_be_reconnected = false, .notify_observers = false }); - if (notify) { const removed = [_]*Node{old_child}; - Frame.observers.notifyChildListChange(frame, self, added.items, &removed, prev, next); + Frame.observers.notifyChildListChange(frame, self, added.items, &removed, prev, reference); } return old_child;