From 0ab7187c8f88bc92d94d6a4a14983126f7687625 Mon Sep 17 00:00:00 2001
From: Sam Clegg <sbc@chromium.org>
Date: Thu, 7 Nov 2024 13:22:08 -0800
Subject: [PATCH] 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 #22874.
---
 src/library_glfw.js                        |  2 +-
 src/library_sdl.js                         |  6 ++++--
 src/proxyClient.js                         |  2 +-
 test/interactive/test_glfw_get_key_stuck.c |  8 ++++----
 test/test_browser.py                       | 12 ++++++------
 test/test_interactive.py                   |  3 +++
 6 files changed, 19 insertions(+), 14 deletions(-)

diff --git a/src/library_glfw.js b/src/library_glfw.js
index a8815d838af9d..7a07e3633e98d 100644
--- a/src/library_glfw.js
+++ b/src/library_glfw.js
@@ -419,7 +419,7 @@ var LibraryGLFW = {
       // This logic comes directly from the sdl implementation. We cannot
       // call preventDefault on all keydown events otherwise onKeyPress will
       // not get called
-      if (event.keyCode === 8 /* backspace */ || event.keyCode === 9 /* tab */) {
+      if (event.key == 'Backspace' || event.key == 'Tab') {
         event.preventDefault();
       }
     },
diff --git a/src/library_sdl.js b/src/library_sdl.js
index 9ab538381b403..27a739a2fff99 100644
--- a/src/library_sdl.js
+++ b/src/library_sdl.js
@@ -696,7 +696,7 @@ var LibrarySDL = {
           // won't fire. However, it's fine (and in some cases necessary) to
           // preventDefault for keys that don't generate a character. Otherwise,
           // preventDefault is the right thing to do in general.
-          if (event.type !== 'keydown' || (!SDL.unicode && !SDL.textInput) || (event.keyCode === 8 /* backspace */ || event.keyCode === 9 /* tab */)) {
+          if (event.type !== 'keydown' || (!SDL.unicode && !SDL.textInput) || (event.key == 'Backspace' || event.key == 'Tab')) {
             event.preventDefault();
           }
 
@@ -930,7 +930,9 @@ var LibrarySDL = {
       switch (event.type) {
         case 'keydown': case 'keyup': {
           var down = event.type === 'keydown';
-          //dbg('Received key event: ' + event.keyCode);
+#if RUNTIME_DEBUG
+          dbg('Received key event: ' + event.keyCode);
+#endif
           var key = SDL.lookupKeyCodeForEvent(event);
           var scan;
           if (key >= 1024) {
diff --git a/src/proxyClient.js b/src/proxyClient.js
index a2dc26d56df67..de17f2e549916 100644
--- a/src/proxyClient.js
+++ b/src/proxyClient.js
@@ -313,7 +313,7 @@ if (!ENVIRONMENT_IS_NODE) {
 // Only prevent default on backspace/tab because we don't want unexpected navigation.
 // Do not prevent default on the rest as we need the keypress event.
 function shouldPreventDefault(event) {
-  if (event.type === 'keydown' && event.keyCode !== 8 /* backspace */ && event.keyCode !== 9 /* tab */) {
+  if (event.type === 'keydown' && event.key != 'Backspace' && event.key != 'Tab') {
     return false; // keypress, back navigation
   } else {
     return true; // NO keypress, NO back navigation
diff --git a/test/interactive/test_glfw_get_key_stuck.c b/test/interactive/test_glfw_get_key_stuck.c
index 878778fb324b0..409df75777bd7 100644
--- a/test/interactive/test_glfw_get_key_stuck.c
+++ b/test/interactive/test_glfw_get_key_stuck.c
@@ -98,10 +98,10 @@ int main() {
     printf("%d. Press and hold spacebar\n", step);
 
 #ifdef __EMSCRIPTEN__
-    emscripten_set_blur_callback(NULL, NULL, true, on_focuspocus);
-    emscripten_set_focus_callback(NULL, NULL, true, on_focuspocus);
-    emscripten_set_focusin_callback(NULL, NULL, true, on_focuspocus);
-    emscripten_set_focusout_callback(NULL, NULL, true, on_focuspocus);
+    emscripten_set_blur_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
+    emscripten_set_focus_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
+    emscripten_set_focusin_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
+    emscripten_set_focusout_callback(EMSCRIPTEN_EVENT_TARGET_WINDOW, NULL, true, on_focuspocus);
 
     emscripten_set_main_loop(render, 0, 1);
     __builtin_trap();
diff --git a/test/test_browser.py b/test/test_browser.py
index 8c8062e1b02ce..950dc3c62f602 100644
--- a/test/test_browser.py
+++ b/test/test_browser.py
@@ -1013,15 +1013,15 @@ def post():
 <script src='fake_events.js'></script>
 <script>
 // Send 'A'.  The corresonding keypress event will not be prevented.
-simulateKeyDown(65);
-simulateKeyUp(65);
+simulateKeyDown(65, 'KeyA);
+simulateKeyUp(65, 'KeyA');
 
 // Send backspace.  The corresonding keypress event *will* be prevented due to proxyClient.js.
-simulateKeyDown(8);
-simulateKeyUp(8);
+simulateKeyDown(8, 'Backspace');
+simulateKeyUp(8, 'Backspace');
 
-simulateKeyDown(100);
-simulateKeyUp(100);
+simulateKeyDown(100, 'Numpad4');
+simulateKeyUp(100, 'Numpad4');
 </script>
 </body>''')
 
diff --git a/test/test_interactive.py b/test/test_interactive.py
index 1d399b60bfc63..bc7f037e5d56d 100644
--- a/test/test_interactive.py
+++ b/test/test_interactive.py
@@ -48,6 +48,9 @@ def test_sdl_touch(self):
   def test_sdl_wm_togglefullscreen(self):
     self.btest_exit('test_sdl_wm_togglefullscreen.c')
 
+  def test_sdl_key_test(self):
+    self.btest_exit('test_sdl_key_test.c')
+
   def test_sdl_fullscreen_samecanvassize(self):
     self.btest_exit('test_sdl_fullscreen_samecanvassize.c')