From f0ca9728aef7a97792c27895dbf35b76af551b44 Mon Sep 17 00:00:00 2001
From: Karl Seguin <k@openmymind.io>
Date: Tue, 26 Aug 2025 13:10:55 +0800
Subject: [PATCH] Support dynamic scripts which are added to the DOM before src
 is set

This should load the "src.js":

```
const s = document.createElement('script');
document.getElementsByTagName('body')[0].appendChild(s);
s.src = "src.js"
```

Notice that src is set AFTER the element is added to the DOM. This PR enables
the above, by
1 - skipping dynamically added scripts which don't have a src
2 - trying to load a script whenever `set_src` is called.

(2) is safe because the ScriptManager already prevents scripts from being
processed multiple times.

Additionally, not only can the src be set after the script is added to the DOM,
but onload and onerror can be set after the src:

```
s.src = "src.js"
s.onload = ...;
s.onerror = ...;
```

This PR also delays reading the onload/onerror callbacks until the script is
done loading.

This behavior is seen on reddit.
---
 src/browser/ScriptManager.zig | 48 +++++++++++++----------------------
 src/browser/events/event.zig  |  1 -
 src/browser/html/elements.zig | 21 +++++++++++----
 src/browser/page.zig          | 11 +++++++-
 4 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig
index bbed5633..e6f3bc93 100644
--- a/src/browser/ScriptManager.zig
+++ b/src/browser/ScriptManager.zig
@@ -147,32 +147,7 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
         return;
     };
 
-    var onload: ?Script.Callback = null;
-    var onerror: ?Script.Callback = null;
-
     const page = self.page;
-    if (page.getNodeState(@ptrCast(element))) |se| {
-        // if the script has a node state, then it was dynamically added and thus
-        // the onload/onerror were saved in the state (if there are any)
-        if (se.onload) |function| {
-            onload = .{ .function = function };
-        }
-        if (se.onerror) |function| {
-            onerror = .{ .function = function };
-        }
-    } else {
-        // if the script has no node state, then it could still be dynamically
-        // added (could have been dynamically added, but no attributes were set
-        // which required a node state to be created) or it could be a inline
-        // <script>.
-        if (try parser.elementGetAttribute(element, "onload")) |string| {
-            onload = .{ .string = string };
-        }
-        if (try parser.elementGetAttribute(element, "onerror")) |string| {
-            onerror = .{ .string = string };
-        }
-    }
-
     var source: Script.Source = undefined;
     var remote_url: ?[:0]const u8 = null;
     if (try parser.elementGetAttribute(element, "src")) |src| {
@@ -188,8 +163,6 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
 
     var script = Script{
         .kind = kind,
-        .onload = onload,
-        .onerror = onerror,
         .element = element,
         .source = source,
         .url = remote_url orelse page.url.raw,
@@ -562,8 +535,6 @@ const Script = struct {
     is_async: bool,
     is_defer: bool,
     source: Source,
-    onload: ?Callback,
-    onerror: ?Callback,
     element: *parser.Element,
 
     const Kind = enum {
@@ -648,7 +619,7 @@ const Script = struct {
     }
 
     fn executeCallback(self: *const Script, comptime typ: []const u8, page: *Page) void {
-        const callback = @field(self, typ) orelse return;
+        const callback = self.getCallback(typ, page) orelse return;
 
         switch (callback) {
             .string => |str| {
@@ -691,6 +662,23 @@ const Script = struct {
             },
         }
     }
+
+    fn getCallback(self: *const Script, comptime typ: []const u8, page: *Page) ?Callback {
+        const element = self.element;
+        // first we check if there was an el.onload set directly on the
+        // element in JavaScript (if so, it'd be stored in the node state)
+        if (page.getNodeState(@ptrCast(element))) |se| {
+            if (@field(se, typ)) |function| {
+                return .{ .function = function };
+            }
+        }
+        // if we have no node state, or if the node state has no onload/onerror
+        // then check for the onload/onerror attribute
+        if (parser.elementGetAttribute(element, typ) catch null) |string| {
+            return .{ .string = string };
+        }
+        return null;
+    }
 };
 
 const BufferPool = struct {
diff --git a/src/browser/events/event.zig b/src/browser/events/event.zig
index 8d16be9e..11ae9f4c 100644
--- a/src/browser/events/event.zig
+++ b/src/browser/events/event.zig
@@ -262,7 +262,6 @@ pub const EventHandler = struct {
                 },
             }
         }
-
         const callback = (try listener.callback(target)) orelse return null;
 
         if (signal) |s| {
diff --git a/src/browser/html/elements.zig b/src/browser/html/elements.zig
index e51a8d5b..4799ea76 100644
--- a/src/browser/html/elements.zig
+++ b/src/browser/html/elements.zig
@@ -862,12 +862,23 @@ pub const HTMLScriptElement = struct {
         ) orelse "";
     }
 
-    pub fn set_src(self: *parser.Script, v: []const u8) !void {
-        return try parser.elementSetAttribute(
+    pub fn set_src(self: *parser.Script, v: []const u8, page: *Page) !void {
+        try parser.elementSetAttribute(
             parser.scriptToElt(self),
             "src",
             v,
         );
+
+        if (try Node.get_isConnected(@alignCast(@ptrCast(self)))) {
+            // There are sites which do set the src AFTER appending the script
+            // tag to the document:
+            //    const s = document.createElement('script');
+            //    document.getElementsByTagName('body')[0].appendChild(s);
+            //    s.src = '...';
+            // This should load the script.
+            // addFromElement protects against double execution.
+            try page.script_manager.addFromElement(@alignCast(@ptrCast(self)));
+        }
     }
 
     pub fn get_type(self: *parser.Script) !?[]const u8 {
@@ -878,7 +889,7 @@ pub const HTMLScriptElement = struct {
     }
 
     pub fn set_type(self: *parser.Script, v: []const u8) !void {
-        return try parser.elementSetAttribute(
+        try parser.elementSetAttribute(
             parser.scriptToElt(self),
             "type",
             v,
@@ -893,7 +904,7 @@ pub const HTMLScriptElement = struct {
     }
 
     pub fn set_text(self: *parser.Script, v: []const u8) !void {
-        return try parser.elementSetAttribute(
+        try parser.elementSetAttribute(
             parser.scriptToElt(self),
             "text",
             v,
@@ -908,7 +919,7 @@ pub const HTMLScriptElement = struct {
     }
 
     pub fn set_integrity(self: *parser.Script, v: []const u8) !void {
-        return try parser.elementSetAttribute(
+        try parser.elementSetAttribute(
             parser.scriptToElt(self),
             "integrity",
             v,
diff --git a/src/browser/page.zig b/src/browser/page.zig
index a533091f..5270a611 100644
--- a/src/browser/page.zig
+++ b/src/browser/page.zig
@@ -1103,13 +1103,22 @@ fn timestamp() u32 {
 // so that's handled. And because we're only executing the inline <script> tags
 // after the document is loaded, it's ok to execute any async and defer scripts
 // immediately.
-pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.C) void {
+pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.c) void {
     const self: *Page = @alignCast(@ptrCast(ctx.?));
+
     if (self.delayed_navigation) {
         // if we're planning on navigating to another page, don't run this script
         return;
     }
 
+    // It's posisble for a script to be dynamically added without a src.
+    //   const s = document.createElement('script');
+    //   document.getElementsByTagName('body')[0].appendChild(s);
+    // The src can be set after. We handle that in HTMLScriptElement.set_src,
+    // but it's important we don't pass such elements to the script_manager
+    // here, else the script_manager will flag it as already-processed.
+    _ = parser.elementGetAttribute(element.?, "src") catch return orelse return;
+
     self.script_manager.addFromElement(element.?) catch |err| {
         log.warn(.browser, "dynamic script", .{ .err = err });
     };