hotkey: Remove the usage of deprecated keypress event.

This commit removes the deprecated `keypress` event.

Now, we use the `keydown` event to handle all the cases.

Earlier, we used `keypress` because it allowed us to distinguish
between lowercase and uppercase characters using `event.which`.

For example:
* For 'r':
  * keypress.which = 114
  * keydown.which = 82
* For 'R':
  * keypress.which = 82
  * keydown.which = 82

As shown, `keydown.which` cannot distinguish between 'r' and 'R',
which is why `keypress` was helpful.

Now, modern browsers support `event.key` on `keydown`, which directly
gives 'r' or 'R' as needed. This makes `keypress` unnecessary, and
we can safely rely on `keydown` with `event.key` to get the exact
character.

An earlier commit in this PR #34570, in which we replaced
the `event.which` with `event.key` plays a major role as a prep
commit to help removing `keypress` event in this commit.

web/src/hotkey.js

@@ -110,7 +110,10 @@ const keydown_shift_mappings = {
     ArrowDown: {name: "down_arrow", message_view_only: false},
     H: {name: "view_edit_history", message_view_only: true},
     N: {name: "narrow_to_next_unread_followed_topic", message_view_only: false},
-    V: {name: "toggle_read_receipts", message_view_only: true},
+    V: [
+        {name: "view_selected_stream", message_view_only: false},
+        {name: "toggle_read_receipts", message_view_only: true},
+    ],
 };
 
 const keydown_unshift_mappings = {
@@ -162,7 +165,7 @@ const keydown_either_mappings = {
     Delete: {name: "delete", message_view_only: false},
 };
 
-const keypress_mappings = {
+const character_mappings = {
     "*": {name: "open_starred_message_view", message_view_only: true},
     "+": {name: "thumbs_up_emoji", message_view_only: true},
     "=": {name: "upvote_first_emoji", message_view_only: true},
@@ -185,7 +188,6 @@ const keypress_mappings = {
     R: {name: "respond_to_author", message_view_only: true},
     S: {name: "toggle_stream_subscription", message_view_only: true},
     U: {name: "mark_unread", message_view_only: true},
-    V: {name: "view_selected_stream", message_view_only: false},
     // The shortcut "a" dates from when this was called "All messages".
     a: {name: "open_combined_feed", message_view_only: true},
     c: {name: "compose", message_view_only: true},
@@ -256,15 +258,12 @@ export function get_keydown_hotkey(e) {
         }
     }
 
-    return keydown_either_mappings[e.key];
-}
-
-export function get_keypress_hotkey(e) {
-    if (e.metaKey || e.ctrlKey || e.altKey) {
-        return undefined;
+    hotkey = keydown_either_mappings[e.key];
+    if (hotkey) {
+        return hotkey;
     }
 
-    return keypress_mappings[e.key];
+    return character_mappings[e.key];
 }
 
 export let processing_text = () => {
@@ -681,7 +680,7 @@ export function process_shift_tab_key() {
     return false;
 }
 
-// Process a keydown or keypress event.
+// Process a keydown event.
 //
 // Returns true if we handled it, false if the browser should.
 export function process_hotkey(e, hotkey) {
@@ -1318,31 +1317,17 @@ export function process_hotkey(e, hotkey) {
     return false;
 }
 
-/* We register both a keydown and a keypress function because
-   we want to intercept PgUp/PgDn, Esc, etc, and process them
-   as they happen on the keyboard. However, if we processed
-   letters/numbers in keydown, we wouldn't know what the case of
-   the letters were.
-
-   We want case-sensitive hotkeys (such as in the case of r vs R)
-   so we bail in .keydown if the event is a letter or number and
-   instead just let keypress go for it. */
-
 export function process_keydown(e) {
     activity.set_new_user_input(true);
-    const hotkey = get_keydown_hotkey(e);
-    if (!hotkey) {
+    const result = get_keydown_hotkey(e);
+    if (!result) {
         return false;
     }
-    return process_hotkey(e, hotkey);
-}
 
-export function process_keypress(e) {
-    const hotkey = get_keypress_hotkey(e);
-    if (!hotkey) {
-        return false;
+    if (Array.isArray(result)) {
+        return result.some((hotkey) => process_hotkey(e, hotkey));
     }
-    return process_hotkey(e, hotkey);
+    return process_hotkey(e, result);
 }
 
 export function initialize() {
@@ -1351,10 +1336,4 @@ export function initialize() {
             e.preventDefault();
         }
     });
-
-    $(document).on("keypress", (e) => {
-        if (process_keypress(e)) {
-            e.preventDefault();
-        }
-    });
 }

web/tests/hotkey.test.cjs

@@ -153,13 +153,6 @@ function test_while_not_editing_text(label, f) {
 }
 
 run_test("mappings", () => {
-    function map_press(key, shiftKey) {
-        return hotkey.get_keypress_hotkey({
-            key,
-            shiftKey,
-        });
-    }
-
     function map_down(key, shiftKey, ctrlKey, metaKey, altKey) {
         return hotkey.get_keydown_hotkey({
             key,
@@ -172,7 +165,7 @@ run_test("mappings", () => {
 
     // The next assertion protects against an iOS bug where we
     // treat "!" as a hotkey, because iOS sends the wrong code.
-    assert.equal(map_press("!"), undefined);
+    assert.equal(map_down("!"), undefined);
 
     // Test page-up does work.
     assert.equal(map_down("PageUp").name, "page_up");
@@ -187,10 +180,13 @@ run_test("mappings", () => {
     assert.equal(map_down("Enter", true).name, "enter");
     assert.equal(map_down("H", true).name, "view_edit_history");
     assert.equal(map_down("N", true).name, "narrow_to_next_unread_followed_topic");
-    assert.equal(map_down("V", true).name, "toggle_read_receipts");
+    assert.deepEqual(
+        map_down("V", true).map((item) => item.name),
+        ["view_selected_stream", "toggle_read_receipts"],
+    );
 
-    assert.equal(map_press("/").name, "search");
-    assert.equal(map_press("j").name, "vim_down");
+    assert.equal(map_down("/").name, "search");
+    assert.equal(map_down("j").name, "vim_down");
 
     assert.equal(map_down("[", false, true).name, "escape");
     assert.equal(map_down("c", false, true).name, "copy_with_c");
@@ -201,8 +197,6 @@ run_test("mappings", () => {
     assert.equal(map_down("p", false, false, false, true).name, "toggle_compose_preview"); // Alt + P
 
     // More negative tests.
-    assert.equal(map_down("/"), undefined);
-    assert.equal(map_press("Escape"), undefined);
     assert.equal(map_down("Escape", true), undefined);
     assert.equal(map_down("v", false, true), undefined);
     assert.equal(map_down("z", false, true), undefined);
@@ -240,16 +234,13 @@ run_test("mappings", () => {
     navigator.platform = "";
 });
 
-function process(s, shiftKey, keydown = false) {
+function process(s, shiftKey) {
     const e = {
         key: s,
         shiftKey,
     };
     try {
-        if (keydown) {
-            return hotkey.process_keydown(e);
-        }
-        return hotkey.process_keypress(e);
+        return hotkey.process_keydown(e);
     } catch (error) /* istanbul ignore next */ {
         // An exception will be thrown here if a different
         // function is called than the one declared.  Try to
@@ -260,9 +251,9 @@ function process(s, shiftKey, keydown = false) {
     }
 }
 
-function assert_mapping(c, module, func_name, shiftKey, keydown) {
+function assert_mapping(c, module, func_name, shiftKey) {
     stubbing(module, func_name, (stub) => {
-        assert.ok(process(c, shiftKey, keydown));
+        assert.ok(process(c, shiftKey));
         assert.equal(stub.num_calls, 1);
     });
 }
@@ -318,7 +309,7 @@ test_while_not_editing_text("streams", ({override}) => {
     override(overlays, "streams_open", () => true);
     override(overlays, "any_active", () => true);
     assert_mapping("S", stream_settings_ui, "keyboard_sub");
-    assert_mapping("V", stream_settings_ui, "view_stream");
+    assert_mapping("V", stream_settings_ui, "view_stream", true);
     assert_mapping("n", stream_settings_ui, "open_create_stream");
     settings_data.user_can_create_private_streams = () => false;
     settings_data.user_can_create_public_streams = () => false;
@@ -424,11 +415,11 @@ test_while_not_editing_text("misc", ({override}) => {
     override(message_edit, "can_move_message", () => false);
     assert_unmapped("m");
 
-    assert_mapping("V", read_receipts, "show_user_list", true, true);
+    assert_mapping("V", read_receipts, "show_user_list", true);
 
     override(modals, "any_active", () => true);
     override(modals, "active_modal", () => "#read_receipts_modal");
-    assert_mapping("V", read_receipts, "hide_user_list", true, true);
+    assert_mapping("V", read_receipts, "hide_user_list", true);
 });
 
 test_while_not_editing_text("lightbox overlay open", ({override}) => {
@@ -468,7 +459,7 @@ test_while_not_editing_text("n/p keys", () => {
 });
 
 test_while_not_editing_text("narrow next unread followed topic", () => {
-    assert_mapping("N", message_view, "narrow_to_next_topic", true, true);
+    assert_mapping("N", message_view, "narrow_to_next_topic", true);
 });
 
 test_while_not_editing_text("motion_keys", () => {
@@ -581,7 +572,10 @@ run_test("test new user input hook called", () => {
         hook_called = true;
     });
 
-    hotkey.process_keydown({key: "S"});
+    // Currently, "b" is not a valid hotkey.
+    // But it serves our purpose here to verify
+    // `hook_called` on keydown.
+    hotkey.process_keydown({key: "b"});
     assert.ok(hook_called);
 });
 
@@ -598,7 +592,7 @@ test_while_not_editing_text("e shortcut works for anonymous users", ({override_r
     };
 
     stubbing(message_edit, "start", (stub) => {
-        hotkey.process_keypress(e);
+        hotkey.process_keydown(e);
         assert.equal(stub.num_calls, 1);
     });
     assert.equal(stub.num_calls, 0, "login_to_access should not be called for 'e' shortcut");