From bdb059b6c9b194cc7c14a1fc7f2a0b52b3bfb648 Mon Sep 17 00:00:00 2001 From: egrs Date: Thu, 19 Feb 2026 14:23:11 +0100 Subject: [PATCH] spec compliance: missing validation guards - Event.preventDefault() and returnValue respect cancelable=false - MutationObserver.observe() validates options per spec - detached checkbox/radio click suppresses input/change events - doctype insertion into non-document throws HierarchyRequestError - error.TypeError maps to JS TypeError (not generic Error) - enable dom_exception on Element/DocumentFragment mutation methods --- src/browser/EventManager.zig | 3 +- src/browser/js/Caller.zig | 1 + src/browser/webapi/DocumentFragment.zig | 6 +-- src/browser/webapi/Element.zig | 10 ++-- src/browser/webapi/Event.zig | 11 +++- src/browser/webapi/MutationObserver.zig | 68 +++++++++++++++++++++---- src/browser/webapi/Node.zig | 5 ++ 7 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/browser/EventManager.zig b/src/browser/EventManager.zig index 640d6bf5..f89ea7ca 100644 --- a/src/browser/EventManager.zig +++ b/src/browser/EventManager.zig @@ -681,9 +681,10 @@ const ActivationState = struct { } // Commit: fire input and change events only if state actually changed + // and the element is connected to a document (detached elements don't fire). // For checkboxes, state always changes. For radios, only if was unchecked. const state_changed = (input._input_type == .checkbox) or !self.old_checked; - if (state_changed) { + if (state_changed and input.asElement().asNode().isConnected()) { fireEvent(page, input, "input") catch |err| { log.warn(.event, "input event", .{ .err = err }); }; diff --git a/src/browser/js/Caller.zig b/src/browser/js/Caller.zig index 7cd8ec35..e20bff8d 100644 --- a/src/browser/js/Caller.zig +++ b/src/browser/js/Caller.zig @@ -311,6 +311,7 @@ fn handleError(comptime T: type, comptime F: type, local: *const Local, err: any const js_err: *const v8.Value = switch (err) { error.TryCatchRethrow => return, error.InvalidArgument => isolate.createTypeError("invalid argument"), + error.TypeError => isolate.createTypeError(""), error.OutOfMemory => isolate.createError("out of memory"), error.IllegalConstructor => isolate.createError("Illegal Contructor"), else => blk: { diff --git a/src/browser/webapi/DocumentFragment.zig b/src/browser/webapi/DocumentFragment.zig index 0967879a..f804b08e 100644 --- a/src/browser/webapi/DocumentFragment.zig +++ b/src/browser/webapi/DocumentFragment.zig @@ -256,9 +256,9 @@ pub const JsApi = struct { pub const childElementCount = bridge.accessor(DocumentFragment.getChildElementCount, null, .{}); pub const firstElementChild = bridge.accessor(DocumentFragment.firstElementChild, null, .{}); pub const lastElementChild = bridge.accessor(DocumentFragment.lastElementChild, null, .{}); - pub const append = bridge.function(DocumentFragment.append, .{}); - pub const prepend = bridge.function(DocumentFragment.prepend, .{}); - pub const replaceChildren = bridge.function(DocumentFragment.replaceChildren, .{}); + pub const append = bridge.function(DocumentFragment.append, .{ .dom_exception = true }); + pub const prepend = bridge.function(DocumentFragment.prepend, .{ .dom_exception = true }); + pub const replaceChildren = bridge.function(DocumentFragment.replaceChildren, .{ .dom_exception = true }); pub const innerHTML = bridge.accessor(_innerHTML, DocumentFragment.setInnerHTML, .{}); fn _innerHTML(self: *DocumentFragment, page: *Page) ![]const u8 { diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index b8732e09..fd61cc39 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -1595,12 +1595,12 @@ pub const JsApi = struct { return self.attachShadow(init.mode, page); } pub const replaceChildren = bridge.function(Element.replaceChildren, .{}); - pub const replaceWith = bridge.function(Element.replaceWith, .{}); + pub const replaceWith = bridge.function(Element.replaceWith, .{ .dom_exception = true }); pub const remove = bridge.function(Element.remove, .{}); - pub const append = bridge.function(Element.append, .{}); - pub const prepend = bridge.function(Element.prepend, .{}); - pub const before = bridge.function(Element.before, .{}); - pub const after = bridge.function(Element.after, .{}); + pub const append = bridge.function(Element.append, .{ .dom_exception = true }); + pub const prepend = bridge.function(Element.prepend, .{ .dom_exception = true }); + pub const before = bridge.function(Element.before, .{ .dom_exception = true }); + pub const after = bridge.function(Element.after, .{ .dom_exception = true }); pub const firstElementChild = bridge.accessor(Element.firstElementChild, null, .{}); pub const lastElementChild = bridge.accessor(Element.lastElementChild, null, .{}); pub const nextElementSibling = bridge.accessor(Element.nextElementSibling, null, .{}); diff --git a/src/browser/webapi/Event.zig b/src/browser/webapi/Event.zig index 1ab895f5..b8568e79 100644 --- a/src/browser/webapi/Event.zig +++ b/src/browser/webapi/Event.zig @@ -189,7 +189,9 @@ pub fn getCurrentTarget(self: *const Event) ?*EventTarget { } pub fn preventDefault(self: *Event) void { - self._prevent_default = true; + if (self._cancelable) { + self._prevent_default = true; + } } pub fn stopPropagation(self: *Event) void { @@ -210,7 +212,12 @@ pub fn getReturnValue(self: *const Event) bool { } pub fn setReturnValue(self: *Event, v: bool) void { - self._prevent_default = !v; + if (!v) { + // Setting returnValue=false is equivalent to preventDefault() + if (self._cancelable) { + self._prevent_default = true; + } + } } pub fn getCancelBubble(self: *const Event) bool { diff --git a/src/browser/webapi/MutationObserver.zig b/src/browser/webapi/MutationObserver.zig index 7d548ee7..65c828e1 100644 --- a/src/browser/webapi/MutationObserver.zig +++ b/src/browser/webapi/MutationObserver.zig @@ -49,10 +49,11 @@ node: std.DoublyLinkedList.Node = .{}, const Observing = struct { target: *Node, - options: ObserveOptions, + options: ResolvedOptions, }; -pub const ObserveOptions = struct { +/// Internal options with all nullable bools resolved to concrete values. +const ResolvedOptions = struct { attributes: bool = false, attributeOldValue: bool = false, childList: bool = false, @@ -62,6 +63,16 @@ pub const ObserveOptions = struct { attributeFilter: ?[]const []const u8 = null, }; +pub const ObserveOptions = struct { + attributes: ?bool = null, + attributeOldValue: ?bool = null, + childList: bool = false, + characterData: ?bool = null, + characterDataOldValue: ?bool = null, + subtree: bool = false, + attributeFilter: ?[]const []const u8 = null, +}; + pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver { const arena = try page.getArena(.{ .debug = "MutationObserver" }); errdefer page.releaseArena(arena); @@ -88,24 +99,61 @@ pub fn deinit(self: *MutationObserver, shutdown: bool) void { pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, page: *Page) !void { const arena = self._arena; + // Per spec: if attributeOldValue/attributeFilter present and attributes + // not explicitly set, imply attributes=true. Same for characterData. + var resolved = options; + if (resolved.attributes == null and (resolved.attributeOldValue != null or resolved.attributeFilter != null)) { + resolved.attributes = true; + } + if (resolved.characterData == null and resolved.characterDataOldValue != null) { + resolved.characterData = true; + } + + const attributes = resolved.attributes orelse false; + const character_data = resolved.characterData orelse false; + + // Validate: at least one of childList/attributes/characterData must be true + if (!resolved.childList and !attributes and !character_data) { + return error.TypeError; + } + + // Validate: attributeOldValue/attributeFilter require attributes != false + if ((resolved.attributeOldValue orelse false) and !attributes) { + return error.TypeError; + } + if (resolved.attributeFilter != null and !attributes) { + return error.TypeError; + } + + // Validate: characterDataOldValue requires characterData != false + if ((resolved.characterDataOldValue orelse false) and !character_data) { + return error.TypeError; + } + + // Build resolved options with concrete bool values + var store_options = ResolvedOptions{ + .attributes = attributes, + .attributeOldValue = resolved.attributeOldValue orelse false, + .childList = resolved.childList, + .characterData = character_data, + .characterDataOldValue = resolved.characterDataOldValue orelse false, + .subtree = resolved.subtree, + .attributeFilter = resolved.attributeFilter, + }; + // Deep copy attributeFilter if present - var copied_options = options; if (options.attributeFilter) |filter| { const filter_copy = try arena.alloc([]const u8, filter.len); for (filter, 0..) |name, i| { filter_copy[i] = try arena.dupe(u8, name); } - copied_options.attributeFilter = filter_copy; - } - - if (options.characterDataOldValue) { - copied_options.characterData = true; + store_options.attributeFilter = filter_copy; } // Check if already observing this target for (self._observing.items) |*obs| { if (obs.target == target) { - obs.options = copied_options; + obs.options = store_options; return; } } @@ -118,7 +166,7 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, try self._observing.append(arena, .{ .target = target, - .options = copied_options, + .options = store_options, }); } diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index 124da937..42eae2e1 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -209,6 +209,11 @@ fn validateNodeInsertion(parent: *Node, node: *Node) !void { if (node._type == .attribute) { return error.HierarchyError; } + + // Doctype nodes can only be inserted into a Document + if (node._type == .document_type and parent._type != .document) { + return error.HierarchyError; + } } pub fn appendChild(self: *Node, child: *Node, page: *Page) !*Node {