From dbe53d053a2d82c3e58755ece855496dc7389544 Mon Sep 17 00:00:00 2001 From: Jonathan Tron Date: Thu, 20 Mar 2025 16:43:49 +0100 Subject: [PATCH] Fix add/remove event listeners in `core:sys/wasm` There were multiple issues here: 1. listeners stored in the same key overwriting the previous one 2. missing `use_capture` parameter in `remove_event_listener`/`remove_window_event_listener` The key used to store the listener function in `listenerMap` was a javascript `Object`, when used as a key it was thus serialized to the string `"[object Object]"`, meaning all listeners where effectively set to the same key when calling `add_event_listener`/`add_window_event_listener`. Later on when calling `remove_event_listener`/`remove_window_event_listener`, it then tried to remove the incorrect one or none at all if there was a mix of the same event name registered on an element or the window. To fix I implemented a function `listener_key` in the javascript code which will generate a different key based on the event's: - `id`: dom element's id or 'window' (when event listener added to the window) - `name`: the event name (eg: `click`), each event handler should be removed for the event name it was register on. - `data`: we can register events with different data, each one generate a new listener which has to be removed. - `callback`: same as `data`, if you register two similar handler but with two different callback, each one should be removed. - `useCapture`: this one is a bit tricky, but when you register an event handler in javascript, if you don't pass `useCapture`, it defaults to `false`. When you remove an handler, you have to pass the exact same `useCapture` option you registered it with. In this case, we allowed to register an event with different `useCapture`, but didn't allow to pass the `useCapture` when removing it. We always called `removeEventListener` without the `useCapture` parameter which removed the handler properly only when it was registered with `useCapture=false`. I also switched the `WasmMemoryInterface.listenerMap` from `{}` (javascript object) to a `new Map()`, which is available everywhere nowadays. --- core/sys/wasm/js/events.odin | 30 ++++++++++++-------- core/sys/wasm/js/events_all_targets.odin | 8 +++--- core/sys/wasm/js/odin.js | 36 +++++++++++++++--------- 3 files changed, 45 insertions(+), 29 deletions(-) diff --git a/core/sys/wasm/js/events.odin b/core/sys/wasm/js/events.odin index f55ee8b90..d217f57e6 100644 --- a/core/sys/wasm/js/events.odin +++ b/core/sys/wasm/js/events.odin @@ -346,13 +346,13 @@ add_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, call return _add_event_listener(id, event_kind_string[kind], kind, user_data, callback, use_capture) } -remove_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { @(default_calling_convention="contextless") foreign dom_lib { @(link_name="remove_event_listener") - _remove_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc "odin" (Event)) -> bool --- + _remove_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc "odin" (Event), use_capture: bool) -> bool --- } - return _remove_event_listener(id, event_kind_string[kind], user_data, callback) + return _remove_event_listener(id, event_kind_string[kind], user_data, callback, use_capture) } add_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { @@ -364,20 +364,26 @@ add_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: return _add_window_event_listener(event_kind_string[kind], kind, user_data, callback, use_capture) } -remove_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { @(default_calling_convention="contextless") foreign dom_lib { @(link_name="remove_window_event_listener") - _remove_window_event_listener :: proc(name: string, user_data: rawptr, callback: proc "odin" (Event)) -> bool --- + _remove_window_event_listener :: proc(name: string, user_data: rawptr, callback: proc "odin" (Event), use_capture: bool) -> bool --- } - return _remove_window_event_listener(event_kind_string[kind], user_data, callback) + return _remove_window_event_listener(event_kind_string[kind], user_data, callback, use_capture) } remove_event_listener_from_event :: proc(e: Event) -> bool { + from_use_capture_false: bool + from_use_capture_true: bool if e.id == "" { - return remove_window_event_listener(e.kind, e.user_data, e.callback) - } - return remove_event_listener(e.id, e.kind, e.user_data, e.callback) + from_use_capture_false = remove_window_event_listener(e.kind, e.user_data, e.callback, false) + from_use_capture_true = remove_window_event_listener(e.kind, e.user_data, e.callback, true) + } else { + from_use_capture_false = remove_event_listener(e.id, e.kind, e.user_data, e.callback, false) + from_use_capture_true = remove_event_listener(e.id, e.kind, e.user_data, e.callback, true) + } + return from_use_capture_false || from_use_capture_true } add_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { @@ -388,13 +394,13 @@ add_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, c } return _add_event_listener(id, name, .Custom, user_data, callback, use_capture) } -remove_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { @(default_calling_convention="contextless") foreign dom_lib { @(link_name="remove_event_listener") - _remove_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc "odin" (Event)) -> bool --- + _remove_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc "odin" (Event), use_capture: bool) -> bool --- } - return _remove_event_listener(id, name, user_data, callback) + return _remove_event_listener(id, name, user_data, callback, use_capture) } get_gamepad_state :: proc "contextless" (index: int, s: ^Gamepad_State) -> bool { diff --git a/core/sys/wasm/js/events_all_targets.odin b/core/sys/wasm/js/events_all_targets.odin index b7e01ca10..6439396c5 100644 --- a/core/sys/wasm/js/events_all_targets.odin +++ b/core/sys/wasm/js/events_all_targets.odin @@ -263,7 +263,7 @@ add_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, call panic("vendor:wasm/js not supported on non JS targets") } -remove_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_event_listener :: proc(id: string, kind: Event_Kind, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { panic("vendor:wasm/js not supported on non JS targets") } @@ -271,7 +271,7 @@ add_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: panic("vendor:wasm/js not supported on non JS targets") } -remove_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_window_event_listener :: proc(kind: Event_Kind, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { panic("vendor:wasm/js not supported on non JS targets") } @@ -282,6 +282,6 @@ remove_event_listener_from_event :: proc(e: Event) -> bool { add_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { panic("vendor:wasm/js not supported on non JS targets") } -remove_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event)) -> bool { +remove_custom_event_listener :: proc(id: string, name: string, user_data: rawptr, callback: proc(e: Event), use_capture := false) -> bool { panic("vendor:wasm/js not supported on non JS targets") -} \ No newline at end of file +} diff --git a/core/sys/wasm/js/odin.js b/core/sys/wasm/js/odin.js index 4e3bb3c22..1599aa4fe 100644 --- a/core/sys/wasm/js/odin.js +++ b/core/sys/wasm/js/odin.js @@ -17,7 +17,7 @@ class WasmMemoryInterface { constructor() { this.memory = null; this.exports = null; - this.listenerMap = {}; + this.listenerMap = new Map(); // Size (in bytes) of the integer type, should be 4 on `js_wasm32` and 8 on `js_wasm64p32` this.intSize = 4; @@ -1403,6 +1403,10 @@ function odinSetupDefaultImports(wasmMemoryInterface, consoleElement, memory) { info.scrollTop = info.scrollHeight; }; + const listener_key = (id, name, data, callback, useCapture) => { + return `${id}-${name}-data:${data}-callback:${callback}-useCapture:${useCapture}`; + }; + let webglContext = new WebGLInterface(wasmMemoryInterface); const env = {}; @@ -1668,7 +1672,8 @@ function odinSetupDefaultImports(wasmMemoryInterface, consoleElement, memory) { onEventReceived(event_data, data, callback); }; - wasmMemoryInterface.listenerMap[{data: data, callback: callback}] = listener; + let key = listener_key(id, name, data, callback, !!use_capture); + wasmMemoryInterface.listenerMap.set(key, listener); element.addEventListener(name, listener, !!use_capture); return true; }, @@ -1685,12 +1690,13 @@ function odinSetupDefaultImports(wasmMemoryInterface, consoleElement, memory) { onEventReceived(event_data, data, callback); }; - wasmMemoryInterface.listenerMap[{data: data, callback: callback}] = listener; + let key = listener_key('window', name, data, callback, !!use_capture); + wasmMemoryInterface.listenerMap.set(key, listener); element.addEventListener(name, listener, !!use_capture); return true; }, - remove_event_listener: (id_ptr, id_len, name_ptr, name_len, data, callback) => { + remove_event_listener: (id_ptr, id_len, name_ptr, name_len, data, callback, use_capture) => { let id = wasmMemoryInterface.loadString(id_ptr, id_len); let name = wasmMemoryInterface.loadString(name_ptr, name_len); let element = getElement(id); @@ -1698,24 +1704,28 @@ function odinSetupDefaultImports(wasmMemoryInterface, consoleElement, memory) { return false; } - let listener = wasmMemoryInterface.listenerMap[{data: data, callback: callback}]; - if (listener == undefined) { + let key = listener_key(id, name, data, callback, !!use_capture); + let listener = wasmMemoryInterface.listenerMap.get(key); + if (listener === undefined) { return false; } - element.removeEventListener(name, listener); + wasmMemoryInterface.listenerMap.delete(key); + + element.removeEventListener(name, listener, !!use_capture); return true; }, - remove_window_event_listener: (name_ptr, name_len, data, callback) => { + remove_window_event_listener: (name_ptr, name_len, data, callback, use_capture) => { let name = wasmMemoryInterface.loadString(name_ptr, name_len); let element = window; - let key = {data: data, callback: callback}; - let listener = wasmMemoryInterface.listenerMap[key]; - if (!listener) { + + let key = listener_key('window', name, data, callback, !!use_capture); + let listener = wasmMemoryInterface.listenerMap.get(key); + if (listener === undefined) { return false; } - wasmMemoryInterface.listenerMap[key] = undefined; + wasmMemoryInterface.listenerMap.delete(key); - element.removeEventListener(name, listener); + element.removeEventListener(name, listener, !!use_capture); return true; },