diff --git a/src/browser/EventManagerBase.zig b/src/browser/EventManagerBase.zig index 4b64ca080..e55a2ec5e 100644 --- a/src/browser/EventManagerBase.zig +++ b/src/browser/EventManagerBase.zig @@ -219,6 +219,7 @@ pub const DispatchError = error{ pub const DispatchDirectOptions = struct { context: []const u8 = "dispatchDirect", inject_target: bool = true, + run_microtasks: bool = true, }; /// Direct dispatch for non-DOM targets. No propagation - just calls the property @@ -249,7 +250,9 @@ pub fn dispatchDirect( var ls: js.Local.Scope = undefined; ctx.localScope(&ls); defer { - ls.local.runMicrotasks(); + if (comptime opts.run_microtasks) { + ls.local.runMicrotasks(); + } ls.deinit(); } diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index c046fc98b..97ddf42d1 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -232,6 +232,10 @@ _customized_builtin_disconnected_callback_invoked: std.AutoHashMapUnmanaged(*Ele // The constructor can access this to get the element being upgraded. _upgrading_element: ?*Node = null, +// _upgrading_element can be consumed once. A second HTMLElement construction +// during upgrade is a TypeError. +_upgrading_consumed: bool = false, + // Set when materializing the fragment parser's context element. The element // is never inserted into the tree so if its a custom element ,we must not run // its constructor (else we'll end up in an endless loop if the constructor diff --git a/src/browser/frame/node_factory.zig b/src/browser/frame/node_factory.zig index f4ee58df2..9cd822889 100644 --- a/src/browser/frame/node_factory.zig +++ b/src/browser/frame/node_factory.zig @@ -948,11 +948,15 @@ pub fn createElementNS(frame: *Frame, namespace: Element.Namespace, name: []cons // Runs a custom element constructor for a token being created (parser or // createElement), and validates the result against the post-conditions. fn constructForToken(frame: *Frame, definition: *CustomElementDefinition, tag_name: String, comptime from_parser: bool) !*Element { - // This is a creation, not an upgrade. super() ha to build a new element const prev_upgrading = frame._upgrading_element; + const prev_consumed = frame._upgrading_consumed; frame._upgrading_element = null; - defer frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = false; + defer { + frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = prev_consumed; + } var ls: JS.Local.Scope = undefined; frame.js.localScope(&ls); @@ -968,38 +972,64 @@ fn constructForToken(frame: *Frame, definition: *CustomElementDefinition, tag_na }; const name = tag_name.str(); - var caught: JS.TryCatch.Caught = .{}; - const object = ls.toLocal(definition.constructor).newInstance(&caught) catch |err| { - log.warn(.js, "custom element constructor", .{ .name = name, .err = err, .caught = caught, .type = frame._type, .url = frame.url }); + const local = &ls.local; + + var try_catch: JS.TryCatch = undefined; + try_catch.init(local); + defer try_catch.deinit(); + + const object = ls.toLocal(definition.constructor).newInstanceThrow() catch |err| { + if (err != error.ExecutionTerminated) { + log.warn(.js, "custom element constructor", .{ .name = name, .err = err, .type = frame._type, .url = frame.url }); + if (try_catch.exceptionValue()) |exc| { + // Spec: report the exception + frame.window.reportError(exc, frame) catch {}; + } + } return err; }; - const reason: []const u8 = blk: { - // validate the result - const node = object.toZig(*Node) catch break :blk "not a node"; - const element = node.is(Element) orelse break :blk "not an element"; + const reason: []const u8, const exc: JS.Value = blk: { + // Validate the result. A result that isn't an HTMLElement is a + // TypeError; any other violation is a NotSupportedError. + const node = object.toZig(*Node) catch break :blk .{ "not a node", typeError(local) }; + const element = node.is(Element) orelse break :blk .{ "not an element", typeError(local) }; if (element._namespace != .html) { - break :blk "wrong namespace"; + break :blk .{ "wrong namespace", typeError(local) }; } if (node._parent != null) { - break :blk "has a parent"; + break :blk .{ "has a parent", notSupportedError(local) }; } if (node.firstChild() != null) { - break :blk "has children"; + break :blk .{ "has children", notSupportedError(local) }; } if (element._attributes.isEmpty() == false) { - break :blk "has attributes"; + break :blk .{ "has attributes", notSupportedError(local) }; + } + if (node.ownerDocument(frame) != frame.document) { + break :blk .{ "wrong document", notSupportedError(local) }; } if (std.mem.eql(u8, element.getTagNameLower(), name) == false) { - break :blk "wrong local name"; + break :blk .{ "wrong local name", notSupportedError(local) }; } return element; }; log.warn(.js, "custom element not usable", .{ .name = name, .reason = reason, .type = frame._type, .url = frame.url }); + frame.window.reportError(exc, frame) catch {}; return error.CustomElementConstructionFailed; } +fn typeError(local: *const JS.Local) JS.Value { + return .{ .local = local, .handle = local.isolate.createTypeError("Invalid custom element constructor return value") }; +} + +fn notSupportedError(local: *const JS.Local) JS.Value { + const DOMException = @import("../webapi/DOMException.zig"); + const ex = DOMException.fromError(error.NotSupported).?; + return local.zigValueToJs(ex, .{}) catch .{ .local = local, .handle = local.isolate.createError("not supported") }; +} + fn createHtmlElementT(frame: *Frame, comptime E: type, namespace: Element.Namespace, attribute_iterator: anytype, html_element: E) !*Node { const html_element_ptr = try frame._factory.htmlElement(html_element); const element = html_element_ptr.asElement(); diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index f81590f8f..325bb8381 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -49,6 +49,21 @@ pub fn withThis(self: *const Function, value: anytype) !Function { } pub fn newInstance(self: *const Function, caught: *js.TryCatch.Caught) !js.Object { + var try_catch: js.TryCatch = undefined; + try_catch.init(self.local); + defer try_catch.deinit(); + + return self.newInstanceThrow() catch |err| { + if (err == error.JsConstructorFailed) { + caught.* = try_catch.caughtOrError(self.local.call_arena, error.Unknown); + } + return err; + }; +} + +// Like newInstance, but with no TryCatch of our own. Gives more flexibility to +// the caller on how to handle the error (e.g. window.reportError) +pub fn newInstanceThrow(self: *const Function) !js.Object { const local = self.local; if (comptime lp.IS_DEBUG == false) { @@ -70,17 +85,12 @@ pub fn newInstance(self: *const Function, caught: *js.TryCatch.Caught) !js.Objec return error.ExecutionTerminated; } - var try_catch: js.TryCatch = undefined; - try_catch.init(local); - defer try_catch.deinit(); - // This creates a new instance using this Function as a constructor. // const c_args = @as(?[*]const ?*c.Value, @ptrCast(&.{})); const handle = v8.v8__Function__NewInstance(self.handle, local.handle, 0, null) orelse { if (local.ctx.env.terminatePending()) { return error.ExecutionTerminated; } - caught.* = try_catch.caughtOrError(local.call_arena, error.Unknown); return error.JsConstructorFailed; }; diff --git a/src/browser/tests/custom_elements/error_reporting.html b/src/browser/tests/custom_elements/error_reporting.html new file mode 100644 index 000000000..0b608c671 --- /dev/null +++ b/src/browser/tests/custom_elements/error_reporting.html @@ -0,0 +1,286 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig index 94bc0502f..36ab863e0 100644 --- a/src/browser/webapi/CustomElementRegistry.zig +++ b/src/browser/webapi/CustomElementRegistry.zig @@ -181,7 +181,9 @@ fn upgradeElement(self: *CustomElementRegistry, element: *Element, frame: *Frame return Custom.checkAndAttachBuiltIn(element, frame); }; - if (custom._definition != null) return; + if (custom._definition != null or custom._upgrade_failed) { + return; + } const name = custom._tag_name.str(); const definition = self._definitions.get(name) orelse return; @@ -198,19 +200,49 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio const node = custom.asNode(); const prev_upgrading = frame._upgrading_element; + const prev_consumed = frame._upgrading_consumed; frame._upgrading_element = node; - defer frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = false; + defer { + frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = prev_consumed; + } var ls: js.Local.Scope = undefined; frame.js.localScope(&ls); defer ls.deinit(); - var caught: js.TryCatch.Caught = .{}; - _ = ls.toLocal(definition.constructor).newInstance(&caught) catch |err| { - log.warn(.js, "custom element upgrade", .{ .name = definition.name, .err = err, .caught = caught }); + const local = &ls.local; + var try_catch: js.TryCatch = undefined; + try_catch.init(local); + defer try_catch.deinit(); + + const object = ls.toLocal(definition.constructor).newInstanceThrow() catch |err| { + if (err == error.ExecutionTerminated) { + custom._definition = null; + return err; + } + log.warn(.js, "custom element upgrade", .{ .name = definition.name, .err = err }); + upgradeFailed(custom); + if (try_catch.exceptionValue()) |exc| { + frame.window.reportError(exc, frame) catch {}; + } return error.CustomElementUpgradeFailed; }; + const same = if (object.toZig(*Node)) |result| result == node else |_| false; + if (!same) { + // the construction result must be the element being upgraded. + log.warn(.js, "custom element upgrade", .{ .name = definition.name, .reason = "constructor returned another value" }); + upgradeFailed(custom); + const exc: js.Value = .{ + .local = local, + .handle = local.isolate.createTypeError("custom element constructor must return the upgraded element"), + }; + frame.window.reportError(exc, frame) catch {}; + return error.CustomElementUpgradeFailed; + } + // Enqueue attributeChangedCallback for existing observed attributes const element = custom.asElement(); for (element.attributeEntries()) |*attr| { @@ -227,6 +259,11 @@ pub fn upgradeCustomElement(custom: *Custom, definition: *CustomElementDefinitio } } +fn upgradeFailed(custom: *Custom) void { + custom._definition = null; + custom._upgrade_failed = true; +} + fn validateName(name: []const u8) !void { if (name.len == 0) { return error.SyntaxError; @@ -288,6 +325,6 @@ pub const JsApi = struct { const testing = @import("../../testing.zig"); test "WebApi: CustomElementRegistry" { - testing.expectLog(&.{ .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js }); + testing.expectLog(&.{ .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js, .js }); try testing.htmlRunner("custom_elements", .{}); } diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index f248f1282..14ccb86f2 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -639,6 +639,7 @@ pub fn reportError(self: *Window, err: js.Value, frame: *Frame) !void { // We still dispatch so that addEventListener('error', ...) listeners fire. try frame._event_manager.dispatchDirect(target, event, null, .{ .context = "window.reportError", + .run_microtasks = false, }); if (comptime lp.IS_TEST == false) { diff --git a/src/browser/webapi/element/Html.zig b/src/browser/webapi/element/Html.zig index d8d6d0574..1dc08f973 100644 --- a/src/browser/webapi/element/Html.zig +++ b/src/browser/webapi/element/Html.zig @@ -116,6 +116,10 @@ _proto_canary: if (lp.IS_DEBUG) *Element else void = undefined, // which custom element class was invoked; look it up in the registry. pub fn construct(new_target: js.Function, frame: *Frame) !*Element { if (frame._upgrading_element) |node| { + if (frame._upgrading_consumed) { + return error.TypeError; + } + frame._upgrading_consumed = true; return node.is(Element) orelse return error.IllegalConstructor; } return Frame.node_factory.constructCustomElement(frame, new_target); @@ -127,6 +131,10 @@ pub fn construct(new_target: js.Function, frame: *Frame) !*Element { // constructors routed here. pub fn upgradeConstruct(frame: *Frame) !*Element { const node = frame._upgrading_element orelse return error.TypeError; + if (frame._upgrading_consumed) { + return error.TypeError; + } + frame._upgrading_consumed = true; return node.is(Element) orelse return error.TypeError; } diff --git a/src/browser/webapi/element/html/Custom.zig b/src/browser/webapi/element/html/Custom.zig index 5b2ce0110..153657bef 100644 --- a/src/browser/webapi/element/html/Custom.zig +++ b/src/browser/webapi/element/html/Custom.zig @@ -41,6 +41,7 @@ _tag_name: String, _definition: ?*CustomElementDefinition, _connected_callback_invoked: bool = false, _disconnected_callback_invoked: bool = false, +_upgrade_failed: bool = false, // a failed upgrade is never retried pub fn asElement(self: *Custom) *Element { return Factory.protoOf(self).asElement(); @@ -62,6 +63,9 @@ pub fn enqueueConnectedCallbackOnElement(comptime from_parser: bool, element: *E if (element.is(Custom)) |custom| { // Upgrade if a definition exists but isn't yet attached if (custom._definition == null) { + if (custom._upgrade_failed) { + return; + } const name = custom._tag_name.str(); if (frame.window._custom_elements._definitions.get(name)) |definition| { const CustomElementRegistry = @import("../../CustomElementRegistry.zig"); @@ -257,9 +261,14 @@ pub fn checkAndAttachBuiltIn(element: *Element, frame: *Frame) !void { // Invoke constructor const prev_upgrading = frame._upgrading_element; + const prev_consumed = frame._upgrading_consumed; const node = element.asNode(); frame._upgrading_element = node; - defer frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = false; + defer { + frame._upgrading_element = prev_upgrading; + frame._upgrading_consumed = prev_consumed; + } // PERFORMANCE OPTIMIZATION: This pattern is discouraged in general code. // Used here because: (1) multiple early returns before needing Local,