Skip to content

Commit f0ca972

Browse files
committed
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.
1 parent ccc9618 commit f0ca972

File tree

4 files changed

+44
-37
lines changed

4 files changed

+44
-37
lines changed

src/browser/ScriptManager.zig

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -147,32 +147,7 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
147147
return;
148148
};
149149

150-
var onload: ?Script.Callback = null;
151-
var onerror: ?Script.Callback = null;
152-
153150
const page = self.page;
154-
if (page.getNodeState(@ptrCast(element))) |se| {
155-
// if the script has a node state, then it was dynamically added and thus
156-
// the onload/onerror were saved in the state (if there are any)
157-
if (se.onload) |function| {
158-
onload = .{ .function = function };
159-
}
160-
if (se.onerror) |function| {
161-
onerror = .{ .function = function };
162-
}
163-
} else {
164-
// if the script has no node state, then it could still be dynamically
165-
// added (could have been dynamically added, but no attributes were set
166-
// which required a node state to be created) or it could be a inline
167-
// <script>.
168-
if (try parser.elementGetAttribute(element, "onload")) |string| {
169-
onload = .{ .string = string };
170-
}
171-
if (try parser.elementGetAttribute(element, "onerror")) |string| {
172-
onerror = .{ .string = string };
173-
}
174-
}
175-
176151
var source: Script.Source = undefined;
177152
var remote_url: ?[:0]const u8 = null;
178153
if (try parser.elementGetAttribute(element, "src")) |src| {
@@ -188,8 +163,6 @@ pub fn addFromElement(self: *ScriptManager, element: *parser.Element) !void {
188163

189164
var script = Script{
190165
.kind = kind,
191-
.onload = onload,
192-
.onerror = onerror,
193166
.element = element,
194167
.source = source,
195168
.url = remote_url orelse page.url.raw,
@@ -562,8 +535,6 @@ const Script = struct {
562535
is_async: bool,
563536
is_defer: bool,
564537
source: Source,
565-
onload: ?Callback,
566-
onerror: ?Callback,
567538
element: *parser.Element,
568539

569540
const Kind = enum {
@@ -648,7 +619,7 @@ const Script = struct {
648619
}
649620

650621
fn executeCallback(self: *const Script, comptime typ: []const u8, page: *Page) void {
651-
const callback = @field(self, typ) orelse return;
622+
const callback = self.getCallback(typ, page) orelse return;
652623

653624
switch (callback) {
654625
.string => |str| {
@@ -691,6 +662,23 @@ const Script = struct {
691662
},
692663
}
693664
}
665+
666+
fn getCallback(self: *const Script, comptime typ: []const u8, page: *Page) ?Callback {
667+
const element = self.element;
668+
// first we check if there was an el.onload set directly on the
669+
// element in JavaScript (if so, it'd be stored in the node state)
670+
if (page.getNodeState(@ptrCast(element))) |se| {
671+
if (@field(se, typ)) |function| {
672+
return .{ .function = function };
673+
}
674+
}
675+
// if we have no node state, or if the node state has no onload/onerror
676+
// then check for the onload/onerror attribute
677+
if (parser.elementGetAttribute(element, typ) catch null) |string| {
678+
return .{ .string = string };
679+
}
680+
return null;
681+
}
694682
};
695683

696684
const BufferPool = struct {

src/browser/events/event.zig

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,6 @@ pub const EventHandler = struct {
262262
},
263263
}
264264
}
265-
266265
const callback = (try listener.callback(target)) orelse return null;
267266

268267
if (signal) |s| {

src/browser/html/elements.zig

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -862,12 +862,23 @@ pub const HTMLScriptElement = struct {
862862
) orelse "";
863863
}
864864

865-
pub fn set_src(self: *parser.Script, v: []const u8) !void {
866-
return try parser.elementSetAttribute(
865+
pub fn set_src(self: *parser.Script, v: []const u8, page: *Page) !void {
866+
try parser.elementSetAttribute(
867867
parser.scriptToElt(self),
868868
"src",
869869
v,
870870
);
871+
872+
if (try Node.get_isConnected(@alignCast(@ptrCast(self)))) {
873+
// There are sites which do set the src AFTER appending the script
874+
// tag to the document:
875+
// const s = document.createElement('script');
876+
// document.getElementsByTagName('body')[0].appendChild(s);
877+
// s.src = '...';
878+
// This should load the script.
879+
// addFromElement protects against double execution.
880+
try page.script_manager.addFromElement(@alignCast(@ptrCast(self)));
881+
}
871882
}
872883

873884
pub fn get_type(self: *parser.Script) !?[]const u8 {
@@ -878,7 +889,7 @@ pub const HTMLScriptElement = struct {
878889
}
879890

880891
pub fn set_type(self: *parser.Script, v: []const u8) !void {
881-
return try parser.elementSetAttribute(
892+
try parser.elementSetAttribute(
882893
parser.scriptToElt(self),
883894
"type",
884895
v,
@@ -893,7 +904,7 @@ pub const HTMLScriptElement = struct {
893904
}
894905

895906
pub fn set_text(self: *parser.Script, v: []const u8) !void {
896-
return try parser.elementSetAttribute(
907+
try parser.elementSetAttribute(
897908
parser.scriptToElt(self),
898909
"text",
899910
v,
@@ -908,7 +919,7 @@ pub const HTMLScriptElement = struct {
908919
}
909920

910921
pub fn set_integrity(self: *parser.Script, v: []const u8) !void {
911-
return try parser.elementSetAttribute(
922+
try parser.elementSetAttribute(
912923
parser.scriptToElt(self),
913924
"integrity",
914925
v,

src/browser/page.zig

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1103,13 +1103,22 @@ fn timestamp() u32 {
11031103
// so that's handled. And because we're only executing the inline <script> tags
11041104
// after the document is loaded, it's ok to execute any async and defer scripts
11051105
// immediately.
1106-
pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.C) void {
1106+
pub export fn scriptAddedCallback(ctx: ?*anyopaque, element: ?*parser.Element) callconv(.c) void {
11071107
const self: *Page = @alignCast(@ptrCast(ctx.?));
1108+
11081109
if (self.delayed_navigation) {
11091110
// if we're planning on navigating to another page, don't run this script
11101111
return;
11111112
}
11121113

1114+
// It's posisble for a script to be dynamically added without a src.
1115+
// const s = document.createElement('script');
1116+
// document.getElementsByTagName('body')[0].appendChild(s);
1117+
// The src can be set after. We handle that in HTMLScriptElement.set_src,
1118+
// but it's important we don't pass such elements to the script_manager
1119+
// here, else the script_manager will flag it as already-processed.
1120+
_ = parser.elementGetAttribute(element.?, "src") catch return orelse return;
1121+
11131122
self.script_manager.addFromElement(element.?) catch |err| {
11141123
log.warn(.browser, "dynamic script", .{ .err = err });
11151124
};

0 commit comments

Comments
 (0)