Merge pull request #1629 from lightpanda-io/script_event_dispatch

Use EventManager.dispatch for Script events

src/browser/EventManager.zig

@@ -61,6 +61,7 @@ arena: Allocator,
 // 'load' event (e.g. amazon product page has no listener and ~350 resources)
 has_dom_load_listener: bool,
 listener_pool: std.heap.MemoryPool(Listener),
+ignore_list: std.ArrayList(*Listener),
 list_pool: std.heap.MemoryPool(std.DoublyLinkedList),
 lookup: std.HashMapUnmanaged(
     EventKey,
@@ -76,6 +77,7 @@ pub fn init(arena: Allocator, page: *Page) EventManager {
         .page = page,
         .lookup = .{},
         .arena = arena,
+        .ignore_list = .{},
         .list_pool = .init(arena),
         .listener_pool = .init(arena),
         .dispatch_depth = 0,
@@ -155,6 +157,11 @@ pub fn register(self: *EventManager, target: *EventTarget, typ: []const u8, call
     };
     // append the listener to the list of listeners for this target
     gop.value_ptr.*.append(&listener.node);
+
+    // Track load listeners for script execution ignore list
+    if (type_string.eql(comptime .wrap("load"))) {
+        try self.ignore_list.append(self.arena, listener);
+    }
 }
 
 pub fn remove(self: *EventManager, target: *EventTarget, typ: []const u8, callback: Callback, use_capture: bool) void {
@@ -167,6 +174,10 @@ pub fn remove(self: *EventManager, target: *EventTarget, typ: []const u8, callba
     }
 }
 
+pub fn clearIgnoreList(self: *EventManager) void {
+    self.ignore_list.clearRetainingCapacity();
+}
+
 // Dispatching can be recursive from the compiler's point of view, so we need to
 // give it an explicit error set so that other parts of the code can use and
 // inferred error.
@@ -178,7 +189,21 @@ const DispatchError = error{
     ExecutionError,
     JsException,
 };
+
+pub const DispatchOpts = struct {
+    // A "load" event triggered by a script (in ScriptManager) should not trigger
+    // a "load" listener added within that script. Therefore, any "load" listener
+    // that we add go into an ignore list until after the script finishes executing.
+    // The ignore list is only checked when apply_ignore  == true, which is only
+    // set by the ScriptManager when raising the script's "load" event.
+    apply_ignore: bool = false,
+};
+
 pub fn dispatch(self: *EventManager, target: *EventTarget, event: *Event) DispatchError!void {
+    return self.dispatchOpts(target, event, .{});
+}
+
+pub fn dispatchOpts(self: *EventManager, target: *EventTarget, event: *Event, comptime opts: DispatchOpts) DispatchError!void {
     defer if (!event._v8_handoff) event.deinit(false, self.page);
 
     if (comptime IS_DEBUG) {
@@ -197,7 +222,7 @@ pub fn dispatch(self: *EventManager, target: *EventTarget, event: *Event) Dispat
     };
 
     switch (target._type) {
-        .node => |node| try self.dispatchNode(node, event, &was_handled),
+        .node => |node| try self.dispatchNode(node, event, &was_handled, opts),
         .xhr,
         .window,
         .abort_signal,
@@ -214,7 +239,7 @@ pub fn dispatch(self: *EventManager, target: *EventTarget, event: *Event) Dispat
                 .event_target = @intFromPtr(target),
                 .type_string = event._type_string,
             }) orelse return;
-            try self.dispatchAll(list, target, event, &was_handled);
+            try self.dispatchAll(list, target, event, &was_handled, opts);
         },
     }
 }
@@ -262,10 +287,10 @@ pub fn dispatchWithFunction(self: *EventManager, target: *EventTarget, event: *E
         .event_target = @intFromPtr(target),
         .type_string = event._type_string,
     }) orelse return;
-    try self.dispatchAll(list, target, event, &was_dispatched);
+    try self.dispatchAll(list, target, event, &was_dispatched, .{});
 }
 
-fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: *bool) !void {
+fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled: *bool, comptime opts: DispatchOpts) !void {
     const ShadowRoot = @import("webapi/ShadowRoot.zig");
 
     const page = self.page;
@@ -346,7 +371,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled:
             .event_target = @intFromPtr(current_target),
             .type_string = event._type_string,
         })) |list| {
-            try self.dispatchPhase(list, current_target, event, was_handled, true);
+            try self.dispatchPhase(list, current_target, event, was_handled, comptime .init(true, opts));
         }
     }
 
@@ -380,7 +405,7 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled:
             .type_string = event._type_string,
             .event_target = @intFromPtr(target_et),
         })) |list| {
-            try self.dispatchPhase(list, target_et, event, was_handled, null);
+            try self.dispatchPhase(list, target_et, event, was_handled, comptime .init(null, opts));
             if (event._stop_propagation) {
                 return;
             }
@@ -397,13 +422,25 @@ fn dispatchNode(self: *EventManager, target: *Node, event: *Event, was_handled:
                 .type_string = event._type_string,
                 .event_target = @intFromPtr(current_target),
             })) |list| {
-                try self.dispatchPhase(list, current_target, event, was_handled, false);
+                try self.dispatchPhase(list, current_target, event, was_handled, comptime .init(false, opts));
             }
         }
     }
 }
 
-fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, comptime capture_only: ?bool) !void {
+const DispatchPhaseOpts = struct {
+    capture_only: ?bool = null,
+    apply_ignore: bool = false,
+
+    fn init(capture_only: ?bool, opts: DispatchOpts) DispatchPhaseOpts {
+        return .{
+            .capture_only = capture_only,
+            .apply_ignore = opts.apply_ignore,
+        };
+    }
+};
+
+fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, comptime opts: DispatchPhaseOpts) !void {
     const page = self.page;
 
     // Track dispatch depth for deferred removal
@@ -429,7 +466,7 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
     // Iterate through the list, stopping after we've encountered the last_listener
     var node = list.first;
     var is_done = false;
-    while (node) |n| {
+    node_loop: while (node) |n| {
         if (is_done) {
             break;
         }
@@ -439,7 +476,7 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
         node = n.next;
 
         // Skip non-matching listeners
-        if (comptime capture_only) |capture| {
+        if (comptime opts.capture_only) |capture| {
             if (listener.capture != capture) {
                 continue;
             }
@@ -458,6 +495,14 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
             }
         }
 
+        if (comptime opts.apply_ignore) {
+            for (self.ignore_list.items) |ignored| {
+                if (ignored == listener) {
+                    continue :node_loop;
+                }
+            }
+        }
+
         // Remove "once" listeners BEFORE calling them so nested dispatches don't see them
         if (listener.once) {
             self.removeListener(list, listener);
@@ -502,8 +547,8 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
 }
 
 //  Non-Node dispatching (XHR, Window without propagation)
-fn dispatchAll(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool) !void {
-    return self.dispatchPhase(list, current_target, event, was_handled, null);
+fn dispatchAll(self: *EventManager, list: *std.DoublyLinkedList, current_target: *EventTarget, event: *Event, was_handled: *bool, comptime opts: DispatchOpts) !void {
+    return self.dispatchPhase(list, current_target, event, was_handled, comptime .init(null, opts));
 }
 
 fn getInlineHandler(self: *EventManager, target: *EventTarget, event: *Event) ?js.Function.Global {

src/browser/ScriptManager.zig

@@ -20,13 +20,14 @@ const std = @import("std");
 const lp = @import("lightpanda");
 const builtin = @import("builtin");
 
-const js = @import("js/js.zig");
 const log = @import("../log.zig");
+const Http = @import("../http/Http.zig");
+const String = @import("../string.zig").String;
 
+const js = @import("js/js.zig");
 const URL = @import("URL.zig");
 const Page = @import("Page.zig");
 const Browser = @import("Browser.zig");
-const Http = @import("../http/Http.zig");
 
 const Element = @import("webapi/Element.zig");
 
@@ -830,13 +831,15 @@ pub const Script = struct {
                     .kind = self.kind,
                     .cacheable = cacheable,
                 });
-                self.executeCallback("error", local.toLocal(script_element._on_error), page);
+                self.executeCallback(comptime .wrap("error"), page);
                 return;
             };
-            self.executeCallback("load", local.toLocal(script_element._on_load), page);
+            self.executeCallback(comptime .wrap("load"), page);
             return;
         }
 
+        defer page._event_manager.clearIgnoreList();
+
         var try_catch: js.TryCatch = undefined;
         try_catch.init(local);
         defer try_catch.deinit();
@@ -855,7 +858,7 @@ pub const Script = struct {
         };
 
         if (comptime IS_DEBUG) {
-            log.debug(.browser, "executed script", .{ .src = url, .success = success, .on_load = script_element._on_load != null });
+            log.debug(.browser, "executed script", .{ .src = url, .success = success });
         }
 
         defer {
@@ -867,7 +870,7 @@ pub const Script = struct {
         }
 
         if (success) {
-            self.executeCallback("load", local.toLocal(script_element._on_load), page);
+            self.executeCallback(comptime .wrap("load"), page);
             return;
         }
 
@@ -878,14 +881,12 @@ pub const Script = struct {
             .cacheable = cacheable,
         });
 
-        self.executeCallback("error", local.toLocal(script_element._on_error), page);
+        self.executeCallback(comptime .wrap("error"), page);
     }
 
-    fn executeCallback(self: *const Script, comptime typ: []const u8, cb_: ?js.Function, page: *Page) void {
-        const cb = cb_ orelse return;
-
+    fn executeCallback(self: *const Script, typ: String, page: *Page) void {
         const Event = @import("webapi/Event.zig");
-        const event = Event.initTrusted(comptime .wrap(typ), .{}, page) catch |err| {
+        const event = Event.initTrusted(typ, .{}, page) catch |err| {
             log.warn(.js, "script internal callback", .{
                 .url = self.url,
                 .type = typ,
@@ -893,14 +894,11 @@ pub const Script = struct {
             });
             return;
         };
-        defer if (!event._v8_handoff) event.deinit(false, self.manager.page);
-
-        var caught: js.TryCatch.Caught = undefined;
-        cb.tryCall(void, .{event}, &caught) catch {
+        page._event_manager.dispatchOpts(self.script_element.?.asNode().asEventTarget(), event, .{ .apply_ignore = true }) catch |err| {
             log.warn(.js, "script callback", .{
                 .url = self.url,
                 .type = typ,
-                .caught = caught,
+                .err = err,
             });
         };
     }

src/browser/tests/element/html/script/script.html

@@ -2,11 +2,28 @@
 <script src="../../../testing.js"></script>
 
 <script id="script">
-   {
-    let s = document.createElement('script');
-    testing.expectEqual('', s.src);
+{
+   let dom_load = false;
+   let attribute_load = false;
 
-    s.src = 'over.9000.js';
-    testing.expectEqual(testing.BASE_URL + 'element/html/script/over.9000.js', s.src);
+   let s = document.createElement('script');
+   document.documentElement.addEventListener('load', (e) => {
+      testing.expectEqual(s, e.target);
+      dom_load = true;
+   }, true);
+
+   testing.expectEqual('', s.src);
+   s.onload = function(e) {
+      testing.expectEqual(s, e.target);
+      attribute_load = true;
    }
+   s.src = 'empty.js';
+   testing.expectEqual(testing.BASE_URL + 'element/html/script/empty.js', s.src);
+   document.head.appendChild(s);
+
+   testing.eventually(() => {
+      testing.expectEqual(true, dom_load);
+      testing.expectEqual(true, attribute_load);
+   });
+}
 </script>

src/browser/webapi/element/html/Script.zig

@@ -30,8 +30,6 @@ const Script = @This();
 
 _proto: *HtmlElement,
 _src: []const u8 = "",
-_on_load: ?js.Function.Global = null,
-_on_error: ?js.Function.Global = null,
 _executed: bool = false,
 
 pub fn asElement(self: *Script) *Element {
@@ -108,22 +106,6 @@ pub fn setDefer(self: *Script, value: bool, page: *Page) !void {
     }
 }
 
-pub fn getOnLoad(self: *const Script) ?js.Function.Global {
-    return self._on_load;
-}
-
-pub fn setOnLoad(self: *Script, cb: ?js.Function.Global) void {
-    self._on_load = cb;
-}
-
-pub fn getOnError(self: *const Script) ?js.Function.Global {
-    return self._on_error;
-}
-
-pub fn setOnError(self: *Script, cb: ?js.Function.Global) void {
-    self._on_error = cb;
-}
-
 pub fn getNoModule(self: *const Script) bool {
     return self.asConstElement().getAttributeSafe(comptime .wrap("nomodule")) != null;
 }
@@ -147,8 +129,6 @@ pub const JsApi = struct {
     pub const @"type" = bridge.accessor(Script.getType, Script.setType, .{});
     pub const nonce = bridge.accessor(Script.getNonce, Script.setNonce, .{});
     pub const charset = bridge.accessor(Script.getCharset, Script.setCharset, .{});
-    pub const onload = bridge.accessor(Script.getOnLoad, Script.setOnLoad, .{});
-    pub const onerror = bridge.accessor(Script.getOnError, Script.setOnError, .{});
     pub const noModule = bridge.accessor(Script.getNoModule, null, .{});
     pub const innerText = bridge.accessor(_innerText, Script.setInnerText, .{});
     fn _innerText(self: *Script, page: *const Page) ![]const u8 {
@@ -160,26 +140,10 @@ pub const JsApi = struct {
 };
 
 pub const Build = struct {
-    pub fn complete(node: *Node, page: *Page) !void {
+    pub fn complete(node: *Node, _: *Page) !void {
         const self = node.as(Script);
         const element = self.asElement();
         self._src = element.getAttributeSafe(comptime .wrap("src")) orelse "";
-
-        if (element.getAttributeSafe(comptime .wrap("onload"))) |on_load| {
-            if (page.js.stringToPersistedFunction(on_load)) |func| {
-                self._on_load = func;
-            } else |err| {
-                log.err(.js, "script.onload", .{ .err = err, .str = on_load });
-            }
-        }
-
-        if (element.getAttributeSafe(comptime .wrap("onerror"))) |on_error| {
-            if (page.js.stringToPersistedFunction(on_error)) |func| {
-                self._on_error = func;
-            } else |err| {
-                log.err(.js, "script.onerror", .{ .err = err, .str = on_error });
-            }
-        }
     }
 };