Skip to content

Fix crash when event target is the window. #757

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 4, 2025
Merged

Fix crash when event target is the window. #757

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions src/browser/dom/event_target.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,15 @@ pub const EventTarget = struct {
pub const Self = parser.EventTarget;
pub const Exception = DOMException;

pub fn toInterface(et: *parser.EventTarget) !Union {
// NOTE: for now we state that all EventTarget are Nodes
// TODO: handle other types (eg. Window)
pub fn toInterface(et: *parser.EventTarget, page: *Page) !Union {
// Not all targets are *parser.Nodes. page.zig emits a "load" event
// where the target is a Window, which cannot be cast directly to a node.
// Ideally, we'd remove this duality. Failing that, we'll need to embed
// data into the *parser.EventTarget should we need this for other types.
// For now, for the Window, which is a singleton, we can do this:
if (@intFromPtr(et) == @intFromPtr(&page.window.base)) {
Copy link
Contributor

@sjorsdonkers sjorsdonkers Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I would make every struct a extern struct and ban struct.
Would also help with slowly taking over libDOM functionality.
IMO zig got the default wrong in this case.

Copy link
Collaborator Author

@karlseguin karlseguin Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm...I was going to say "I don't want to make that decision".

Then I thought, "well, we could at least do it on Window for this specific problem...try it out".

But ya..no. There's a bunch of restrictions on what you can put into an extern struct. You can't, for example, have slices. So target: []const u8 = "", isn't valid.

If this is the way we go, it's a bigger change / discussion.

Copy link
Contributor

@sjorsdonkers sjorsdonkers Jun 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my😱

return .{ .Window = &page.window };
}
return Nod.Node.toInterface(@as(*parser.Node, @ptrCast(et)));
}

Expand Down
1 change: 1 addition & 0 deletions src/browser/events/custom_event.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const JsObject = @import("../env.zig").JsObject;
// https://dom.spec.whatwg.org/#interface-customevent
pub const CustomEvent = struct {
pub const prototype = *Event;
pub const union_make_copy = true;

proto: parser.Event,
detail: ?JsObject,
Expand Down
9 changes: 5 additions & 4 deletions src/browser/events/event.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const log = @import("../../log.zig");
const parser = @import("../netsurf.zig");
const generate = @import("../../runtime/generate.zig");

const Page = @import("../page.zig").Page;
const DOMException = @import("../dom/exceptions.zig").DOMException;
const EventTarget = @import("../dom/event_target.zig").EventTarget;
const EventTargetUnion = @import("../dom/event_target.zig").Union;
Expand Down Expand Up @@ -76,16 +77,16 @@ pub const Event = struct {
return try parser.eventType(self);
}

pub fn get_target(self: *parser.Event) !?EventTargetUnion {
pub fn get_target(self: *parser.Event, page: *Page) !?EventTargetUnion {
const et = try parser.eventTarget(self);
if (et == null) return null;
return try EventTarget.toInterface(et.?);
return try EventTarget.toInterface(et.?, page);
}

pub fn get_currentTarget(self: *parser.Event) !?EventTargetUnion {
pub fn get_currentTarget(self: *parser.Event, page: *Page) !?EventTargetUnion {
const et = try parser.eventCurrentTarget(self);
if (et == null) return null;
return try EventTarget.toInterface(et.?);
return try EventTarget.toInterface(et.?, page);
}

pub fn get_eventPhase(self: *parser.Event) !u8 {
Expand Down
1 change: 1 addition & 0 deletions src/browser/xhr/progress_event.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const DOMException = @import("../dom/exceptions.zig").DOMException;
pub const ProgressEvent = struct {
pub const prototype = *Event;
pub const Exception = DOMException;
pub const union_make_copy = true;

pub const EventInit = struct {
lengthComputable: bool = false,
Expand Down
12 changes: 7 additions & 5 deletions src/runtime/generate.zig
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub fn Union(comptime interfaces: anytype) type {
var FT = @field(tuple, field.name);
if (@hasDecl(FT, "Self")) {
FT = *(@field(FT, "Self"));
} else if (!@hasDecl(FT, "union_make_copy")) {
FT = *FT;
}
union_fields[index] = .{
.type = FT,
Expand Down Expand Up @@ -171,7 +173,7 @@ fn filterMap(comptime count: usize, interfaces: [count]type) struct { usize, [co
return .{ unfiltered_count, map };
}

test "generate.Union" {
test "generate: Union" {
const Astruct = struct {
pub const Self = Other;
const Other = struct {};
Expand All @@ -188,15 +190,15 @@ test "generate.Union" {
const value = Union(.{ Astruct, Bstruct, .{Cstruct} });
const ti = @typeInfo(value).@"union";
try std.testing.expectEqual(3, ti.fields.len);
try std.testing.expectEqualStrings("*runtime.generate.test.generate.Union.Astruct.Other", @typeName(ti.fields[0].type));
try std.testing.expectEqualStrings("*runtime.generate.test.generate: Union.Astruct.Other", @typeName(ti.fields[0].type));
try std.testing.expectEqualStrings(ti.fields[0].name, "Astruct");
try std.testing.expectEqual(Bstruct, ti.fields[1].type);
try std.testing.expectEqual(*Bstruct, ti.fields[1].type);
try std.testing.expectEqualStrings(ti.fields[1].name, "Bstruct");
try std.testing.expectEqual(Cstruct, ti.fields[2].type);
try std.testing.expectEqual(*Cstruct, ti.fields[2].type);
try std.testing.expectEqualStrings(ti.fields[2].name, "Cstruct");
}

test "generate.Tuple" {
test "generate: Tuple" {
const Astruct = struct {};

const Bstruct = struct {
Expand Down