From 1a83e69669000cd110ea68f02a8a0e0027831392 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 16 May 2025 11:29:51 +0800 Subject: [PATCH] Fix HTMLCollection named property issues 1 - Named properties should not be enumerable 2 - Empty key should always result in a null/undefined (depending on the API) even if there's an element with an empty id/name To address the first issue, we now require PropertyAttributes to be specified when setting an object's value. --- src/browser/dom/html_collection.zig | 10 ++++++++-- src/browser/dom/nodelist.zig | 2 +- src/runtime/js.zig | 24 ++++++++++++++++-------- 3 files changed, 25 insertions(+), 11 deletions(-) diff --git a/src/browser/dom/html_collection.zig b/src/browser/dom/html_collection.zig index 830792fd..1291a0aa 100644 --- a/src/browser/dom/html_collection.zig +++ b/src/browser/dom/html_collection.zig @@ -406,10 +406,16 @@ pub const HTMLCollection = struct { for (0..len) |i| { const node = try self.item(@intCast(i)) orelse unreachable; const e = @as(*parser.Element, @ptrCast(node)); - try js_this.setIndex(@intCast(i), e); + try js_this.setIndex(@intCast(i), e, .{}); if (try item_name(e)) |name| { - try js_this.set(name, e); + // Even though an entry might have an empty id, the spec says + // that namedItem("") should always return null + if (name.len > 0) { + // Named fields should not be enumerable (it is defined with + // the LegacyUnenumerableNamedProperties flag.) + try js_this.set(name, e, .{ .DONT_ENUM = true }); + } } } } diff --git a/src/browser/dom/nodelist.zig b/src/browser/dom/nodelist.zig index 439e1eca..ca09723b 100644 --- a/src/browser/dom/nodelist.zig +++ b/src/browser/dom/nodelist.zig @@ -173,7 +173,7 @@ pub const NodeList = struct { const len = self.get_length(); for (0..len) |i| { const node = try self._item(@intCast(i)) orelse unreachable; - try js_this.setIndex(i, node); + try js_this.setIndex(@intCast(i), node, .{}); } } }; diff --git a/src/runtime/js.zig b/src/runtime/js.zig index fbb0065f..6d7cd275 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -894,20 +894,28 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { // for this declaration is _a lot_ easier. const _JSOBJECT_ID_KLUDGE = true; - pub fn setIndex(self: JsObject, index: usize, value: anytype) !void { + const SetOpts = packed struct(u32) { + READ_ONLY: bool = false, + DONT_ENUM: bool = false, + DONT_DELETE: bool = false, + _: u29 = 0, + }; + pub fn setIndex(self: JsObject, index: u32, value: anytype, opts: SetOpts) !void { const key = switch (index) { inline 0...1000 => |i| std.fmt.comptimePrint("{d}", .{i}), else => try std.fmt.allocPrint(self.scope.scope_arena, "{d}", .{index}), }; - return self.set(key, value); + return self.set(key, value, opts); } - pub fn set(self: JsObject, key: []const u8, value: anytype) !void { + pub fn set(self: JsObject, key: []const u8, value: anytype, opts: SetOpts) !void { const scope = self.scope; const js_key = v8.String.initUtf8(scope.isolate, key); const js_value = try scope.zigValueToJs(value); - if (!self.js_obj.setValue(scope.context, js_key, js_value)) { + + const res = self.js_obj.defineOwnProperty(scope.context, js_key.toName(), js_value, @bitCast(opts)) orelse false; + if (!res) { return error.FailedToSet; } } @@ -957,12 +965,12 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { const _JSTHIS_ID_KLUDGE = true; - pub fn setIndex(self: JsThis, index: usize, value: anytype) !void { - return self.obj.setIndex(index, value); + pub fn setIndex(self: JsThis, index: u32, value: anytype, opts: JsObject.SetOpts) !void { + return self.obj.setIndex(index, value, opts); } - pub fn set(self: JsThis, key: []const u8, value: anytype) !void { - return self.obj.set(key, value); + pub fn set(self: JsThis, key: []const u8, value: anytype, opts: JsObject.SetOpts) !void { + return self.obj.set(key, value, opts); } };