Skip to content

Commit 0ab7187

Browse files
committed
Reduce usage of legacy event.keyCode. NFC
This change replaced the usage of the legacy `keyCode` attribute with the preferred `key` attribute, which also has the advantage of removing some hardcoded constant numbers. In order to test this change I fixed the `test_glfw_get_key_stuck` test and added `test_sdl_key_test` to the interactive tests. In doing so I noticed and fixed a crash bug that was introduced in emscripten-core#22874.
1 parent ffeb761 commit 0ab7187

File tree

6 files changed

+19
-14
lines changed

6 files changed

+19
-14
lines changed

src/library_glfw.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -419,7 +419,7 @@ var LibraryGLFW = {
419419
// This logic comes directly from the sdl implementation. We cannot
420420
// call preventDefault on all keydown events otherwise onKeyPress will
421421
// not get called
422-
if (event.keyCode === 8 /* backspace */ || event.keyCode === 9 /* tab */) {
422+
if (event.key == 'Backspace' || event.key == 'Tab') {
423423
event.preventDefault();
424424
}
425425
},

src/library_sdl.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -696,7 +696,7 @@ var LibrarySDL = {
696696
// won't fire. However, it's fine (and in some cases necessary) to
697697
// preventDefault for keys that don't generate a character. Otherwise,
698698
// preventDefault is the right thing to do in general.
699-
if (event.type !== 'keydown' || (!SDL.unicode && !SDL.textInput) || (event.keyCode === 8 /* backspace */ || event.keyCode === 9 /* tab */)) {
699+
if (event.type !== 'keydown' || (!SDL.unicode && !SDL.textInput) || (event.key == 'Backspace' || event.key == 'Tab')) {
700700
event.preventDefault();
701701
}
702702

@@ -930,7 +930,9 @@ var LibrarySDL = {
930930
switch (event.type) {
931931
case 'keydown': case 'keyup': {
932932
var down = event.type === 'keydown';
933-
//dbg('Received key event: ' + event.keyCode);
933+
#if RUNTIME_DEBUG
934+
dbg('Received key event: ' + event.keyCode);
935+
#endif
934936
var key = SDL.lookupKeyCodeForEvent(event);
935937
var scan;
936938
if (key >= 1024) {

src/proxyClient.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ if (!ENVIRONMENT_IS_NODE) {
313313
// Only prevent default on backspace/tab because we don't want unexpected navigation.
314314
// Do not prevent default on the rest as we need the keypress event.
315315
function shouldPreventDefault(event) {
316-
if (event.type === 'keydown' && event.keyCode !== 8 /* backspace */ && event.keyCode !== 9 /* tab */) {
316+
if (event.type === 'keydown' && event.key != 'Backspace' && event.key != 'Tab') {
317317
return false; // keypress, back navigation
318318
} else {
319319
return true; // NO keypress, NO back navigation

test/interactive/test_glfw_get_key_stuck.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,10 @@ int main() {
9898
printf("%d. Press and hold spacebar\n", step);
9999

100100
#ifdef __EMSCRIPTEN__
101-
emscripten_set_blur_callback(NULL, NULL, true, on_focuspocus);
102-
emscripten_set_focus_callback(NULL, NULL, true, on_focuspocus);
103-
emscripten_set_focusin_callback(NULL, NULL, true, on_focuspocus);
104-
emscripten_set_focusout_callback(NULL, NULL, true, on_focuspocus);
101+
emscripten_set_blur_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
102+
emscripten_set_focus_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
103+
emscripten_set_focusin_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
104+
emscripten_set_focusout_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
105105

106106
emscripten_set_main_loop(render, 0, 1);
107107
__builtin_trap();

test/test_browser.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,15 +1013,15 @@ def post():
10131013
<script src='fake_events.js'></script>
10141014
<script>
10151015
// Send 'A'. The corresonding keypress event will not be prevented.
1016-
simulateKeyDown(65);
1017-
simulateKeyUp(65);
1016+
simulateKeyDown(65, 'KeyA);
1017+
simulateKeyUp(65, 'KeyA');
10181018
10191019
// Send backspace. The corresonding keypress event *will* be prevented due to proxyClient.js.
1020-
simulateKeyDown(8);
1021-
simulateKeyUp(8);
1020+
simulateKeyDown(8, 'Backspace');
1021+
simulateKeyUp(8, 'Backspace');
10221022
1023-
simulateKeyDown(100);
1024-
simulateKeyUp(100);
1023+
simulateKeyDown(100, 'Numpad4');
1024+
simulateKeyUp(100, 'Numpad4');
10251025
</script>
10261026
</body>''')
10271027

test/test_interactive.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,9 @@ def test_sdl_touch(self):
4848
def test_sdl_wm_togglefullscreen(self):
4949
self.btest_exit('test_sdl_wm_togglefullscreen.c')
5050

51+
def test_sdl_key_test(self):
52+
self.btest_exit('test_sdl_key_test.c')
53+
5154
def test_sdl_fullscreen_samecanvassize(self):
5255
self.btest_exit('test_sdl_fullscreen_samecanvassize.c')
5356

0 commit comments

Comments
 (0)