Improve events

1 - Expose various event types for Workers
2 - Listen to the removed listener flag in more places. We delay removing the
    listener (to keep the list intact) via a flag, but need to consider that
    flag in all places, e.g. when checking for duplicates when adding a listener
3 - Enforce passive flag. We have this flag, but weren't using it to block
    calls to preventDefault returnValue (which a passive listener should not
    call)
This commit is contained in:
Karl Seguin
2026-05-11 12:34:52 +08:00
parent efbf1db87c
commit 8d5eef44c8
5 changed files with 89 additions and 19 deletions

View File

@@ -61,10 +61,7 @@ pub fn init(arena: Allocator, frame: *Frame) EventManager {
}
pub fn register(self: *EventManager, target: *EventTarget, typ: []const u8, callback: Callback, opts: RegisterOptions) !void {
const listener = self.base.register(target, typ, callback, opts) catch |err| switch (err) {
error.SignalAborted, error.DuplicateListener => return,
else => return err,
};
const listener = (try self.base.register(target, typ, callback, opts)) orelse return;
if (listener.typ.eql(comptime .wrap("load"))) {
if (target._type == .node) {
@@ -321,16 +318,14 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
// Track dispatch depth for deferred removal
base.dispatch_depth += 1;
defer {
const dispatch_depth = base.dispatch_depth;
base.dispatch_depth -= 1;
// Only destroy deferred listeners when we exit the outermost dispatch
if (dispatch_depth == 1) {
if (base.dispatch_depth == 0) {
for (base.deferred_removals.items) |removal| {
removal.list.remove(&removal.listener.node);
base.listener_pool.destroy(removal.listener);
}
base.deferred_removals.clearRetainingCapacity();
} else {
base.dispatch_depth = dispatch_depth - 1;
}
}
@@ -385,6 +380,7 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
was_handled.* = true;
event._current_target = current_target;
event._in_passive_listener = listener.passive;
// Compute adjusted target for shadow DOM retargeting (only if needed)
const original_target = event._target;
@@ -394,6 +390,8 @@ fn dispatchPhase(self: *EventManager, list: *std.DoublyLinkedList, current_targe
try listener.run(frame.call_arena, local, event, "listener");
event._in_passive_listener = false;
// Restore original target (only if we changed it)
if (event._needs_retargeting) {
event._target = original_target;

View File

@@ -89,7 +89,13 @@ pub const Callback = union(enum) {
object: js.Object,
};
pub fn register(self: *EventManagerBase, target: *EventTarget, typ: []const u8, callback: Callback, opts: RegisterOptions) !*Listener {
// Returns null when the listener is a no-op per spec: signal already
// aborted, or a duplicate (same type + callback + capture) of an
// already-registered listener. Real errors (OOM, StringTooLarge) still
// propagate. Callers don't have to distinguish "skipped" from "registered"
// unless they need the resulting *Listener (e.g. Frame's load-listener
// tracking).
pub fn register(self: *EventManagerBase, target: *EventTarget, typ: []const u8, callback: Callback, opts: RegisterOptions) !?*Listener {
if (comptime IS_DEBUG) {
log.debug(.event, "EventManager.register", .{
.type = typ,
@@ -102,7 +108,7 @@ pub fn register(self: *EventManagerBase, target: *EventTarget, typ: []const u8,
// If a signal is provided and already aborted, don't register the listener
if (opts.signal) |signal| {
if (signal.getAborted()) {
return error.SignalAborted;
return null;
}
}
@@ -114,18 +120,22 @@ pub fn register(self: *EventManagerBase, target: *EventTarget, typ: []const u8,
.event_target = @intFromPtr(target),
});
if (gop.found_existing) {
// check for duplicate callbacks already registered
// check for duplicate callbacks already registered. Listeners that
// have been removed (e.g. a `once` listener that fired mid-dispatch
// and is awaiting destruction in deferred_removals) are not "in"
// the listener list per spec — skip them.
var node = gop.value_ptr.*.first;
while (node) |n| {
const listener: *Listener = @alignCast(@fieldParentPtr("node", n));
node = n.next;
if (listener.removed) continue;
const is_duplicate = switch (callback) {
.object => |obj| listener.function.eqlObject(obj),
.function => |func| listener.function.eqlFunction(func),
};
if (is_duplicate and listener.capture == opts.capture) {
return error.DuplicateListener;
return null;
}
node = n.next;
}
} else {
gop.value_ptr.* = try self.list_pool.create();
@@ -164,6 +174,11 @@ pub fn remove(self: *EventManagerBase, target: *EventTarget, typ: []const u8, ca
}
pub fn removeListener(self: *EventManagerBase, list: *std.DoublyLinkedList, listener: *Listener) void {
// Already removed (or queued for removal). Avoids double-pushing the
// same listener into deferred_removals — which would double-free at
// the outer-dispatch cleanup — if e.g. a `once` listener also calls
// removeEventListener on itself.
if (listener.removed) return;
// If we're in a dispatch, defer removal to avoid invalidating iteration
if (self.dispatch_depth > 0) {
listener.removed = true;
@@ -256,16 +271,14 @@ pub fn dispatchDirect(
// Track dispatch depth for deferred removal
self.dispatch_depth += 1;
defer {
const dispatch_depth = self.dispatch_depth;
self.dispatch_depth -= 1;
// Only destroy deferred listeners when we exit the outermost dispatch
if (dispatch_depth == 1) {
if (self.dispatch_depth == 0) {
for (self.deferred_removals.items) |removal| {
removal.list.remove(&removal.listener.node);
self.listener_pool.destroy(removal.listener);
}
self.deferred_removals.clearRetainingCapacity();
} else {
self.dispatch_depth = dispatch_depth - 1;
}
}
@@ -304,9 +317,12 @@ pub fn dispatchDirect(
}
event._current_target = target;
event._in_passive_listener = listener.passive;
try listener.run(arena, &ls.local, event, opts.context);
event._in_passive_listener = false;
if (event._stop_immediate_propagation) {
return;
}
@@ -356,6 +372,10 @@ fn findListener(list: *const std.DoublyLinkedList, callback: Callback, capture:
while (node) |n| {
node = n.next;
const listener: *Listener = @alignCast(@fieldParentPtr("node", n));
// Per spec, a removed listener isn't "in" the list anymore; skip
// entries still present only because their deferred removal hasn't
// been flushed yet.
if (listener.removed) continue;
const matches = switch (callback) {
.object => |obj| listener.function.eqlObject(obj),
.function => |func| listener.function.eqlFunction(func),

View File

@@ -944,6 +944,11 @@ pub const WorkerJsApis = flattenTypes(&.{
@import("../webapi/WorkerGlobalScope.zig"),
@import("../webapi/WorkerLocation.zig"),
@import("../webapi/EventTarget.zig"),
@import("../webapi/Event.zig"),
@import("../webapi/event/MessageEvent.zig"),
@import("../webapi/event/ErrorEvent.zig"),
@import("../webapi/event/PromiseRejectionEvent.zig"),
@import("../webapi/event/CloseEvent.zig"),
@import("../webapi/DOMException.zig"),
@import("../webapi/net/URLSearchParams.zig"),
@import("../webapi/encoding/TextEncoder.zig"),

View File

@@ -613,6 +613,52 @@
}
</script>
<div id=once_readd_test></div>
<script id=onceListenerReaddedDuringNestedDispatch>
// Regression: a `once: true` listener that re-adds itself inside its
// callback and triggers a nested dispatch must see the *new* registration
// fire. The original `once` instance is `removed=true` but still parked
// in the listener list until the outer dispatch flushes deferred
// removals, so `register`'s duplicate check has to skip removed entries
// — otherwise the re-add is silently swallowed as a "duplicate".
{
const el = $('#once_readd_test');
let invoked = 0;
function h() {
invoked++;
if (invoked == 1) el.addEventListener('reonce', h, {once: true});
if (invoked <= 2) el.dispatchEvent(new Event('reonce'));
}
el.addEventListener('reonce', h, {once: true});
el.dispatchEvent(new Event('reonce'));
testing.expectEqual(2, invoked);
// Third dispatch shouldn't fire anything: every prior registration was
// `once` and has been consumed.
el.dispatchEvent(new Event('reonce'));
testing.expectEqual(2, invoked);
}
</script>
<div id=once_self_remove_test></div>
<script id=onceListenerExplicitlyRemovesItself>
// Regression: a `once: true` listener that also calls removeEventListener
// on itself must not double-defer the same node (which would double-free
// on outer-dispatch cleanup).
{
const el = $('#once_self_remove_test');
let calls = 0;
const fn = () => {
calls++;
el.removeEventListener('osr', fn);
};
el.addEventListener('osr', fn, {once: true});
el.dispatchEvent(new Event('osr'));
el.dispatchEvent(new Event('osr'));
testing.expectEqual(1, calls);
}
</script>
<script id=isTrusted>
// Test isTrusted property on generic Event
let untrustedEvent = new Event('test');

View File

@@ -48,6 +48,7 @@ _event_phase: EventPhase = .none,
_time_stamp: u64,
_needs_retargeting: bool = false,
_is_trusted: bool = false,
_in_passive_listener: bool = false,
// There's a period of time between creating an event and handing it off to v8
// where things can fail. If it does fail, we need to deinit the event. The timing
@@ -204,7 +205,7 @@ pub fn getCurrentTarget(self: *const Event) ?*EventTarget {
}
pub fn preventDefault(self: *Event) void {
if (self._cancelable) {
if (self._cancelable and !self._in_passive_listener) {
self._prevent_default = true;
}
}
@@ -229,7 +230,7 @@ pub fn getReturnValue(self: *const Event) bool {
pub fn setReturnValue(self: *Event, v: bool) void {
if (!v) {
// Setting returnValue=false is equivalent to preventDefault()
if (self._cancelable) {
if (self._cancelable and !self._in_passive_listener) {
self._prevent_default = true;
}
}