Improve correctness of Node.compareDocumentPosition and Range api.

karlseguin · karlseguin · commit d51a03f1b60f · 2025-08-19T18:23:54.000+08:00
Should fix a good chunk (~20K I think) of the recently broken WPT tests.
diff --git a/src/browser/dom/exceptions.zig b/src/browser/dom/exceptions.zig
@@ -68,23 +68,24 @@ pub const DOMException = struct {
     }
 
     // TODO: deinit
-    pub fn init(alloc: std.mem.Allocator, err: anyerror, callerName: []const u8) !DOMException {
-        const errCast = @as(parser.DOMError, @errorCast(err));
-        const errName = DOMException.name(errCast);
-        const str = switch (errCast) {
+    pub fn init(alloc: std.mem.Allocator, err: anyerror, caller_name: []const u8) !DOMException {
+        const dom_error = @as(parser.DOMError, @errorCast(err));
+        const error_name = DOMException.name(dom_error);
+        const str = switch (dom_error) {
             error.HierarchyRequest => try allocPrint(
                 alloc,
                 "{s}: Failed to execute '{s}' on 'Node': The new child element contains the parent.",
-                .{ errName, callerName },
+                .{ error_name, caller_name },
             ),
-            error.NoError => unreachable,
+            // todo add more custom error messages
             else => try allocPrint(
                 alloc,
-                "{s}: TODO message", // TODO: implement other messages
-                .{DOMException.name(errCast)},
+                "{s}: Failed to execute '{s}' : {s}",
+                .{ error_name, caller_name, error_name },
             ),
+            error.NoError => unreachable,
         };
-        return .{ .err = errCast, .str = str };
+        return .{ .err = dom_error, .str = str };
     }
 
     fn error_from_str(name_: []const u8) ?parser.DOMError {
diff --git a/src/browser/dom/node.zig b/src/browser/dom/node.zig
@@ -107,6 +107,13 @@ pub const Node = struct {
     pub const _ENTITY_NODE = @intFromEnum(parser.NodeType.entity);
     pub const _NOTATION_NODE = @intFromEnum(parser.NodeType.notation);
 
+    pub const _DOCUMENT_POSITION_DISCONNECTED = @intFromEnum(parser.DocumentPosition.disconnected);
+    pub const _DOCUMENT_POSITION_PRECEDING = @intFromEnum(parser.DocumentPosition.preceding);
+    pub const _DOCUMENT_POSITION_FOLLOWING = @intFromEnum(parser.DocumentPosition.following);
+    pub const _DOCUMENT_POSITION_CONTAINS = @intFromEnum(parser.DocumentPosition.contains);
+    pub const _DOCUMENT_POSITION_CONTAINED_BY = @intFromEnum(parser.DocumentPosition.contained_by);
+    pub const _DOCUMENT_POSITION_IMPLEMENTATION_SPECIFIC = @intFromEnum(parser.DocumentPosition.implementation_specific);
+
     // JS funcs
     // --------
 
@@ -266,8 +273,18 @@ pub const Node = struct {
         const docother = try parser.nodeOwnerDocument(other);
 
         // Both are in different document.
-        if (docself == null or docother == null or docother.? != docself.?) {
-            return @intFromEnum(parser.DocumentPosition.disconnected);
+        if (docself == null or docother == null or docself.? != docother.?) {
+            return @intFromEnum(parser.DocumentPosition.disconnected) +
+                @intFromEnum(parser.DocumentPosition.implementation_specific) +
+                @intFromEnum(parser.DocumentPosition.preceding);
+        }
+
+        const rootself = try parser.nodeGetRootNode(self);
+        const rootother = try parser.nodeGetRootNode(other);
+        if (rootself != rootother) {
+            return @intFromEnum(parser.DocumentPosition.disconnected) +
+                @intFromEnum(parser.DocumentPosition.implementation_specific) +
+                @intFromEnum(parser.DocumentPosition.preceding);
         }
 
         // TODO Both are in a different trees in the same document.
diff --git a/src/browser/dom/range.zig b/src/browser/dom/range.zig
@@ -21,8 +21,9 @@ const std = @import("std");
 const parser = @import("../netsurf.zig");
 const Page = @import("../page.zig").Page;
 
-const NodeUnion = @import("node.zig").Union;
 const Node = @import("node.zig").Node;
+const NodeUnion = @import("node.zig").Union;
+const DOMException = @import("exceptions.zig").DOMException;
 
 pub const Interfaces = .{
     AbstractRange,
@@ -32,9 +33,9 @@ pub const Interfaces = .{
 pub const AbstractRange = struct {
     collapsed: bool,
     end_container: *parser.Node,
-    end_offset: i32,
+    end_offset: u32,
     start_container: *parser.Node,
-    start_offset: i32,
+    start_offset: u32,
 
     pub fn updateCollapsed(self: *AbstractRange) void {
         // TODO: Eventually, compare properly.
@@ -49,20 +50,21 @@ pub const AbstractRange = struct {
         return Node.toInterface(self.end_container);
     }
 
-    pub fn get_endOffset(self: *const AbstractRange) i32 {
+    pub fn get_endOffset(self: *const AbstractRange) u32 {
         return self.end_offset;
     }
 
     pub fn get_startContainer(self: *const AbstractRange) !NodeUnion {
         return Node.toInterface(self.start_container);
     }
 
-    pub fn get_startOffset(self: *const AbstractRange) i32 {
+    pub fn get_startOffset(self: *const AbstractRange) u32 {
         return self.start_offset;
     }
 };
 
 pub const Range = struct {
+    pub const Exception = DOMException;
     pub const prototype = *AbstractRange;
 
     proto: AbstractRange,
@@ -82,18 +84,83 @@ pub const Range = struct {
         return .{ .proto = proto };
     }
 
-    pub fn _setStart(self: *Range, node: *parser.Node, offset: i32) void {
+    pub fn _setStart(self: *Range, node: *parser.Node, offset_: i32) !void {
+        const relative = self._comparePoint(node, offset_) catch |err| switch (err) {
+            error.WrongDocument => blk: {
+                // comparePoint doesn't check this on WrongDocument.
+                try ensureValidOffset(node, offset_);
+
+                // allow a node with a different root than the current, or
+                // a disconnected one. Treat it as if it's "after", so that
+                // we also update the end_offset and end_container.
+                break :blk 1;
+            },
+            else => return err,
+        };
+
+        const offset: u32 = @intCast(offset_);
+        if (relative == 1) {
+            // if we're setting the node after the current start, the end must
+            // be set too.
+            self.proto.end_offset = offset;
+            self.proto.end_container = node;
+        }
         self.proto.start_container = node;
         self.proto.start_offset = offset;
         self.proto.updateCollapsed();
     }
 
-    pub fn _setEnd(self: *Range, node: *parser.Node, offset: i32) void {
+    pub fn _setStartBefore(self: *Range, node: *parser.Node) !void {
+        const parent, const index = try getParentAndIndex(node);
+        self.proto.start_container = parent;
+        self.proto.start_offset = index;
+    }
+
+    pub fn _setStartAfter(self: *Range, node: *parser.Node) !void {
+        const parent, const index = try getParentAndIndex(node);
+        self.proto.start_container = parent;
+        self.proto.start_offset = index + 1;
+    }
+
+    pub fn _setEnd(self: *Range, node: *parser.Node, offset_: i32) !void {
+        const relative = self._comparePoint(node, offset_) catch |err| switch (err) {
+            error.WrongDocument => blk: {
+                // comparePoint doesn't check this on WrongDocument.
+                try ensureValidOffset(node, offset_);
+
+                // allow a node with a different root than the current, or
+                // a disconnected one. Treat it as if it's "before", so that
+                // we also update the end_offset and end_container.
+                break :blk -1;
+            },
+            else => return err,
+        };
+
+        const offset: u32 = @intCast(offset_);
+        if (relative == -1) {
+            // if we're setting the node before the current start, the start
+            // must be
+            self.proto.start_offset = offset;
+            self.proto.start_container = node;
+        }
+
         self.proto.end_container = node;
         self.proto.end_offset = offset;
         self.proto.updateCollapsed();
     }
 
+    pub fn _setEndBefore(self: *Range, node: *parser.Node) !void {
+        const parent, const index = try getParentAndIndex(node);
+        self.proto.end_container = parent;
+        self.proto.end_offset = index;
+    }
+
+    pub fn _setEndAfter(self: *Range, node: *parser.Node) !void {
+        const parent, const index = try getParentAndIndex(node);
+        self.proto.end_container = parent;
+        self.proto.end_offset = index + 1;
+    }
+
     pub fn _createContextualFragment(_: *Range, fragment: []const u8, page: *Page) !*parser.DocumentFragment {
         const document_html = page.window.document;
         const document = parser.documentHTMLToDocument(document_html);
@@ -127,13 +194,159 @@ pub const Range = struct {
         self.proto.updateCollapsed();
     }
 
+    // creates a copy
+    pub fn _cloneRange(self: *const Range) Range {
+        return .{
+            .proto = .{
+                .collapsed = self.proto.collapsed,
+                .end_container = self.proto.end_container,
+                .end_offset = self.proto.end_offset,
+                .start_container = self.proto.start_container,
+                .start_offset = self.proto.start_offset,
+            },
+        };
+    }
+
+    pub fn _comparePoint(self: *const Range, ref_node: *parser.Node, offset_: i32) !i32 {
+        const start = self.proto.start_container;
+        if (try parser.nodeGetRootNode(start) != try parser.nodeGetRootNode(ref_node)) {
+            // WPT really wants this error to be first. Later, when we check
+            // if the relative position is 'disconnected', it'll also catch this
+            // case, but WPT will complain because it sometimes also sends
+            // invalid offsets, and it wants WrongDocument to be raised.
+            return error.WrongDocument;
+        }
+
+        if (try parser.nodeType(ref_node) == .document_type) {
+            return error.InvalidNodeType;
+        }
+
+        try ensureValidOffset(ref_node, offset_);
+
+        const offset: u32 = @intCast(offset_);
+        if (ref_node == start) {
+            // This is a simple and common case, where the reference node and
+            // our start node are the same, so we just have to compare the offsets
+            const start_offset = self.proto.start_offset;
+            if (offset == start_offset) {
+                return 0;
+            }
+            return if (offset < start_offset) -1 else 1;
+        }
+
+        // We're probably comparing two different nodes. "Probably", because the
+        // above case on considered the offset if the two nodes were the same
+        // as-is. They could still be the same here, if we first consider the
+        // offset.
+        // Furthermore, as far as I can tell, if either or both nodes are textual,
+        // then we're doing a node comparison of their parents. This kind of
+        // makes sense, one/two text nodes which aren't the same, can only
+        // be positionally compared in relation to it/their parents.
+
+        const adjusted_start = try getNodeForCompare(start, self.proto.start_offset);
+        const adjusted_ref_node = try getNodeForCompare(ref_node, offset);
+
+        const relative = try Node._compareDocumentPosition(adjusted_start, adjusted_ref_node);
+
+        if (relative & @intFromEnum(parser.DocumentPosition.disconnected) == @intFromEnum(parser.DocumentPosition.disconnected)) {
+            return error.WrongDocument;
+        }
+
+        if (relative & @intFromEnum(parser.DocumentPosition.preceding) == @intFromEnum(parser.DocumentPosition.preceding)) {
+            return -1;
+        }
+
+        if (relative & @intFromEnum(parser.DocumentPosition.following) == @intFromEnum(parser.DocumentPosition.following)) {
+            return 1;
+        }
+
+        // DUNNO
+        // unreachable??
+        return 0;
+    }
+
+    pub fn _isPointInRange(self: *const Range, ref_node: *parser.Node, offset_: i32) !bool {
+        return self._comparePoint(ref_node, offset_) catch |err| switch (err) {
+            error.WrongDocument => return false,
+            else => return err,
+        } == 0;
+    }
+
     // The Range.detach() method does nothing. It used to disable the Range
     // object and enable the browser to release associated resources. The
     // method has been kept for compatibility.
     // https://developer.mozilla.org/en-US/docs/Web/API/Range/detach
     pub fn _detach(_: *Range) void {}
 };
 
+fn getNodeForCompare(node: *parser.Node, offset: u32) !*parser.Node {
+    if (try isTextual(node)) {
+        // when we're comparing a text node to another node which is not the same
+        // then we're really compare the position of the parent. It doesn't
+        // matter if the other node is a text node itself or not, all that matters
+        // is we're sure it isn't the same text node (because if they are the
+        // same text node, then we're comparing the offset (character position)
+        // of the text node)
+
+        // not sure this is the correct error
+        return (try parser.nodeParentNode(node)) orelse return error.WrongDocument;
+    }
+    if (offset == 0) {
+        return node;
+    }
+
+    const children = try parser.nodeGetChildNodes(node);
+
+    // not sure about this error
+    // - 1 because, while the offset is 0 based, 0 seems to represent the parent
+    return (try parser.nodeListItem(children, offset - 1)) orelse error.IndexSize;
+}
+
+fn ensureValidOffset(node: *parser.Node, offset: i32) !void {
+    if (offset < 0) {
+        return error.IndexSize;
+    }
+
+    // not >= because 0 seems to represent the node itself.
+    if (offset > try nodeLength(node)) {
+        return error.IndexSize;
+    }
+}
+
+fn nodeLength(node: *parser.Node) !usize {
+    switch (try isTextual(node)) {
+        true => return ((try parser.nodeTextContent(node)) orelse "").len,
+        false => {
+            const children = try parser.nodeGetChildNodes(node);
+            return @intCast(try parser.nodeListLength(children));
+        },
+    }
+}
+
+fn isTextual(node: *parser.Node) !bool {
+    return switch (try parser.nodeType(node)) {
+        .text, .comment, .cdata_section => true,
+        else => false,
+    };
+}
+
+fn getParentAndIndex(child: *parser.Node) !struct { *parser.Node, u32 } {
+    const parent = (try parser.nodeParentNode(child)) orelse return error.InvalidNodeType;
+    const children = try parser.nodeGetChildNodes(parent);
+    const ln = try parser.nodeListLength(children);
+    var i: u32 = 0;
+    while (i < ln) {
+        defer i += 1;
+        const c = try parser.nodeListItem(children, i) orelse continue;
+        if (c == child) {
+            return .{ parent, i };
+        }
+    }
+
+    // should not be possible to reach this point
+    return error.InvalidNodeType;
+}
+
 const testing = @import("../../testing.zig");
 test "Browser.Range" {
     var runner = try testing.jsRunner(testing.tracking_allocator, .{});
diff --git a/src/main_wpt.zig b/src/main_wpt.zig
@@ -18,6 +18,7 @@
 
 const std = @import("std");
 
+const log = @import("log.zig");
 const Allocator = std.mem.Allocator;
 const ArenaAllocator = std.heap.ArenaAllocator;
 
@@ -29,11 +30,6 @@ const polyfill = @import("browser/polyfill/polyfill.zig");
 
 const WPT_DIR = "tests/wpt";
 
-pub const std_options = std.Options{
-    // Set the log level to info
-    .log_level = .info,
-};
-
 // TODO For now the WPT tests run is specific to WPT.
 // It manually load js framwork libs, and run the first script w/ js content in
 // the HTML page.
@@ -43,6 +39,7 @@ pub fn main() !void {
     var gpa: std.heap.DebugAllocator(.{}) = .init;
     defer _ = gpa.deinit();
     const allocator = gpa.allocator();
+    log.opts.level = .warn;
 
     // An arena for the runner itself, lives for the duration of the the process
     var ra = ArenaAllocator.init(allocator);
diff --git a/src/runtime/js.zig b/src/runtime/js.zig
@@ -3866,7 +3866,7 @@ const NamedFunction = struct {
 // this can add as much as 10 seconds of compilation time.
 fn logFunctionCallError(arena: Allocator, isolate: v8.Isolate, context: v8.Context, err: anyerror, function_name: []const u8, info: v8.FunctionCallbackInfo) void {
     const args_dump = serializeFunctionArgs(arena, isolate, context, info) catch "failed to serialize args";
-    log.warn(.js, "function call error", .{
+    log.info(.js, "function call error", .{
         .name = function_name,
         .err = err,
         .args = args_dump,
