Add a way to remove a single event listener (#20983) #25535
+207
−0
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| /* | ||
| * Copyright 2025 The Emscripten Authors. All rights reserved. | ||
| * Emscripten is available under two separate licenses, the MIT license and the | ||
| * University of Illinois/NCSA Open Source License. Both these licenses can be | ||
| * found in the LICENSE file. | ||
| */ | ||
|
|
||
| #include <assert.h> | ||
| #include <stdio.h> | ||
| #include <emscripten.h> | ||
| #include <string.h> | ||
| #include <emscripten/html5.h> | ||
|
|
||
| const char *emscripten_result_to_string(EMSCRIPTEN_RESULT result) { | ||
| if (result == EMSCRIPTEN_RESULT_SUCCESS) return "EMSCRIPTEN_RESULT_SUCCESS"; | ||
| if (result == EMSCRIPTEN_RESULT_DEFERRED) return "EMSCRIPTEN_RESULT_DEFERRED"; | ||
| if (result == EMSCRIPTEN_RESULT_NOT_SUPPORTED) return "EMSCRIPTEN_RESULT_NOT_SUPPORTED"; | ||
| if (result == EMSCRIPTEN_RESULT_FAILED_NOT_DEFERRED) return "EMSCRIPTEN_RESULT_FAILED_NOT_DEFERRED"; | ||
| if (result == EMSCRIPTEN_RESULT_INVALID_TARGET) return "EMSCRIPTEN_RESULT_INVALID_TARGET"; | ||
| if (result == EMSCRIPTEN_RESULT_UNKNOWN_TARGET) return "EMSCRIPTEN_RESULT_UNKNOWN_TARGET"; | ||
| if (result == EMSCRIPTEN_RESULT_INVALID_PARAM) return "EMSCRIPTEN_RESULT_INVALID_PARAM"; | ||
| if (result == EMSCRIPTEN_RESULT_FAILED) return "EMSCRIPTEN_RESULT_FAILED"; | ||
| if (result == EMSCRIPTEN_RESULT_NO_DATA) return "EMSCRIPTEN_RESULT_NO_DATA"; | ||
| return "Unknown EMSCRIPTEN_RESULT!"; | ||
| } | ||
|
|
||
| // Report API failure | ||
| #define TEST_RESULT(x) if (ret != EMSCRIPTEN_RESULT_SUCCESS) printf("%s returned %s.\n", #x, emscripten_result_to_string(ret)); | ||
|
|
||
| // Like above above but also assert API success | ||
| #define ASSERT_RESULT(x) TEST_RESULT(x); assert(ret == EMSCRIPTEN_RESULT_SUCCESS); | ||
|
|
||
| char const *userDataToString(void *userData) { | ||
| return userData ? (char const *) userData : "nullptr"; | ||
| } | ||
|
|
||
| bool key_callback_1(int eventType, const EmscriptenKeyboardEvent *e, void *userData) { | ||
| printf("key_callback_1: eventType=%d, userData=%s\n", eventType, userDataToString(userData)); | ||
| return 0; | ||
| } | ||
|
|
||
| bool key_callback_2(int eventType, const EmscriptenKeyboardEvent *e, void *userData) { | ||
| printf("key_callback_2: eventType=%d, userData=%s\n", eventType, userDataToString(userData)); | ||
| return 0; | ||
| } | ||
|
|
||
| bool mouse_callback_1(int eventType, const EmscriptenMouseEvent *e, void *userData) { | ||
| printf("mouse_callback_1: eventType=%d, userData=%s\n", eventType, userDataToString(userData)); | ||
| return 0; | ||
| } | ||
|
|
||
| void checkCount(int count) | ||
| { | ||
| int eventHandlersCount = EM_ASM_INT({ return JSEvents.eventHandlers.length; }); | ||
| printf("Detected [%d] handlers\n", eventHandlersCount); | ||
| assert(count == eventHandlersCount); | ||
| } | ||
|
|
||
| void test_done() {} | ||
|
|
||
| int main() { | ||
| bool useCapture = true; | ||
| void *userData3 = "3"; | ||
|
|
||
| // first we check for invalid parameters | ||
| assert(emscripten_remove_callback("this_dom_element_does_not_exist", NULL, 0, key_callback_1) == EMSCRIPTEN_RESULT_UNKNOWN_TARGET); | ||
|
|
||
| checkCount(0); | ||
|
|
||
| EMSCRIPTEN_RESULT ret = emscripten_set_keypress_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, useCapture, key_callback_1); | ||
| ASSERT_RESULT(emscripten_set_keypress_callback); | ||
| ret = emscripten_set_keydown_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, useCapture, key_callback_1); | ||
| ASSERT_RESULT(emscripten_set_keydown_callback); | ||
| ret = emscripten_set_keyup_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, useCapture, key_callback_1); | ||
| ASSERT_RESULT(emscripten_set_keyup_callback); | ||
|
|
||
| checkCount(3); | ||
|
|
||
| // removing keydown event | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, EMSCRIPTEN_EVENT_KEYDOWN, key_callback_1); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(2); | ||
|
|
||
| // adding another keypress callback on the same target | ||
| ret = emscripten_set_keypress_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, useCapture, key_callback_2); | ||
| ASSERT_RESULT(emscripten_set_keypress_callback); | ||
|
|
||
| checkCount(3); | ||
|
|
||
| // adding another keypress callback on the same target with different user data | ||
| ret = emscripten_set_keypress_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, userData3, useCapture, key_callback_2); | ||
| ASSERT_RESULT(emscripten_set_keypress_callback); | ||
|
|
||
| checkCount(4); | ||
|
|
||
| // removing a combination that does not exist (no mouse_callback_1 registered) | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, EMSCRIPTEN_EVENT_KEYPRESS, mouse_callback_1); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(4); | ||
|
|
||
| // removing keypress / userData=NULL / key_callback_2 | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, EMSCRIPTEN_EVENT_KEYPRESS, key_callback_2); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(3); | ||
|
|
||
| // removing keypress / userData=NULL / key_callback_1 | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, EMSCRIPTEN_EVENT_KEYPRESS, key_callback_1); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(2); | ||
|
|
||
| // removing keypress / userData=3 / key_callback_2 | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, userData3, EMSCRIPTEN_EVENT_KEYPRESS, key_callback_2); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(1); | ||
|
|
||
| // adding the same mouse down callback to 2 different targets | ||
| ret = emscripten_set_mousedown_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, useCapture, mouse_callback_1); | ||
| ASSERT_RESULT(emscripten_set_mousedown_callback); | ||
| ret = emscripten_set_mousedown_callback("#canvas", NULL, useCapture, mouse_callback_1); | ||
| ASSERT_RESULT(emscripten_set_mousedown_callback); | ||
|
|
||
| checkCount(3); | ||
|
|
||
| // removing mousedown / userData=NULL / mouse_callback_1 on the window target | ||
| ret = emscripten_remove_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, EMSCRIPTEN_EVENT_MOUSEDOWN, mouse_callback_1); | ||
| ASSERT_RESULT(emscripten_remove_callback); | ||
|
|
||
| checkCount(2); | ||
|
|
||
| return 0; | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, should we key the
userDatapointer into the removal? I.e. it would be better for this API to beThe rationale is that the
userDatapointer is generally just data to the function call, and not identifying. I.e. it is more of the 'value' part in a key->value association.It might be some function local value in the caller (e.g. a boolean for some behavior), and might result in user having to store the
userDatafield in some global to be able to unregister. So requiring users to know to match theuserDatavalue could be a bit tedious.(If users did specifically want to differentiate unregistrations based on what was seemingly a different userData value, they could do that by separating to a different function with a trampoline)
Also, this function would be good to follow the same
_on_threadmodel, so that it can be called from pthreads symmetric to how registering on pthreads works.The semantics of function
emscripten_remove_callbackare to remove all copies of callback registrations of the given function. I wonder if the function naming should somehow reflect that?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this makes sense to me, but in that case I think it would also be good it if we make it impossible to register the same callback function for the same event with different userData.
If we didn't error in the second case, and allowed the same callback to be registred twice, there would be no way to unregister just one of them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what @juj suggested was to indeed un-register all callbacks that were registered with different data but with the same target/event_id/callback combination. That would have eliminated having to store the user data.
It sounds like you don't like this because then "there would be no way to unregister just one of them".
I am definitely not in favor of erroring when registering 2 identical callbacks with different user data, because from a developer point of view, this is absolutely not an error and I can imagine scenarios where that could be useful.
So I guess the solution is to leave the API and changes as implemented in this PR. That way we can un-register a single callback (which IS the point of this PR). So I am personally in favor of this.
I will now work on updating the documentation and fixing the "Code Size" failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you give an example of when this might be useful?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This a theoretical example, because I am not implementing it this way at the moment, but imagine for GLFW, you have 2 windows (represented in C by GLFWwindow pointers and canvases in HTML) and you need to set mouse move callbacks to track the mouse movements.
In order to detect moving the mouse outside the canvas while the button is clicked (something my library supports where others don't) you cannot set the callback on each canvas, it has to be set on the window... so the callbacks would be:
Again I am not doing it this way, but that could be a way to implement it...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair enough. If we do want to officially support that then we probably also want to officially support unregistering them individually (sadly)