hotkey: Replace deprecated e.which with e.key.

The `keydown` and `keypress` event handlers defined in `hotkey.js`
were using `event.which`, which is deprecated.

This commit makes changes to use `event.key` instead.

As a side effect, it fixes a small bug where both `Alt+P` and
`Alt+Shift+P` could be used to toggle preview mode. Only `Alt+P`
is the defined shortcut for that.
This commit is contained in:
Prakhar Pratyush
2025-05-06 15:11:59 +05:30
committed by Tim Abbott
parent 104b5c9731
commit 075e398307
2 changed files with 157 additions and 154 deletions

View File

@@ -102,47 +102,47 @@ const color_picker_hotkeys = new Set([
const keydown_shift_mappings = {
// these can be triggered by Shift + key only
9: {name: "shift_tab", message_view_only: false}, // Tab
32: {name: "shift_spacebar", message_view_only: true}, // space bar
37: {name: "left_arrow", message_view_only: false}, // left arrow
39: {name: "right_arrow", message_view_only: false}, // right arrow
38: {name: "up_arrow", message_view_only: false}, // up arrow
40: {name: "down_arrow", message_view_only: false}, // down arrow
72: {name: "view_edit_history", message_view_only: true}, // 'H'
78: {name: "narrow_to_next_unread_followed_topic", message_view_only: false}, // 'N'
86: {name: "toggle_read_receipts", message_view_only: true}, // 'V'
Tab: {name: "shift_tab", message_view_only: false},
" ": {name: "shift_spacebar", message_view_only: true},
ArrowLeft: {name: "left_arrow", message_view_only: false},
ArrowRight: {name: "right_arrow", message_view_only: false},
ArrowUp: {name: "up_arrow", message_view_only: false},
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},
};
const keydown_unshift_mappings = {
// these can be triggered by key only (without Shift)
9: {name: "tab", message_view_only: false}, // Tab
27: {name: "escape", message_view_only: false}, // Esc
32: {name: "spacebar", message_view_only: true}, // space bar
33: {name: "page_up", message_view_only: true}, // PgUp
34: {name: "page_down", message_view_only: true}, // PgDn
35: {name: "end", message_view_only: true}, // End
36: {name: "home", message_view_only: true}, // Home
37: {name: "left_arrow", message_view_only: false}, // left arrow
39: {name: "right_arrow", message_view_only: false}, // right arrow
38: {name: "up_arrow", message_view_only: false}, // up arrow
40: {name: "down_arrow", message_view_only: false}, // down arrow
Tab: {name: "tab", message_view_only: false},
Escape: {name: "escape", message_view_only: false},
" ": {name: "spacebar", message_view_only: true},
PageUp: {name: "page_up", message_view_only: true},
PageDown: {name: "page_down", message_view_only: true},
End: {name: "end", message_view_only: true},
Home: {name: "home", message_view_only: true},
ArrowLeft: {name: "left_arrow", message_view_only: false},
ArrowRight: {name: "right_arrow", message_view_only: false},
ArrowUp: {name: "up_arrow", message_view_only: false},
ArrowDown: {name: "down_arrow", message_view_only: false},
};
const keydown_ctrl_mappings = {
219: {name: "escape", message_view_only: false}, // '['
"[": {name: "escape", message_view_only: false},
};
const keydown_cmd_or_ctrl_mappings = {
13: {name: "action_with_enter", message_view_only: true}, // 'Enter'
67: {name: "copy_with_c", message_view_only: false}, // 'C'
75: {name: "search_with_k", message_view_only: false}, // 'K'
83: {name: "star_message", message_view_only: true}, // 'S'
190: {name: "narrow_to_compose_target", message_view_only: true}, // '.'
222: {name: "open_saved_snippet_dropdown", message_view_only: true}, // '''
Enter: {name: "action_with_enter", message_view_only: true},
c: {name: "copy_with_c", message_view_only: false},
k: {name: "search_with_k", message_view_only: false},
s: {name: "star_message", message_view_only: true},
".": {name: "narrow_to_compose_target", message_view_only: true},
"'": {name: "open_saved_snippet_dropdown", message_view_only: true},
};
const keydown_alt_mappings = {
80: {name: "toggle_compose_preview", message_view_only: true}, // 'P'
p: {name: "toggle_compose_preview", message_view_only: true},
};
const keydown_either_mappings = {
@@ -157,65 +157,67 @@ const keydown_either_mappings = {
// For Enter, there is some possibly that Shift-Enter is intended to
// have special behavior for folks that are used to Shift-Enter behavior
// in other apps, but that's also slightly dubious.
8: {name: "backspace", message_view_only: true}, // Backspace
13: {name: "enter", message_view_only: false}, // Enter
46: {name: "delete", message_view_only: false}, // Delete
Backspace: {name: "backspace", message_view_only: true},
Enter: {name: "enter", message_view_only: false},
Delete: {name: "delete", message_view_only: false},
};
const keypress_mappings = {
42: {name: "open_starred_message_view", message_view_only: true}, // '*'
43: {name: "thumbs_up_emoji", message_view_only: true}, // '+'
61: {name: "upvote_first_emoji", message_view_only: true}, // '='
45: {name: "toggle_message_collapse", message_view_only: true}, // '-'
47: {name: "search", message_view_only: false}, // '/'
58: {name: "toggle_reactions_popover", message_view_only: true}, // ':'
60: {name: "compose_forward_message", message_view_only: true}, // '<'
62: {name: "compose_quote_message", message_view_only: true}, // '>'
63: {name: "show_shortcuts", message_view_only: false}, // '?'
64: {name: "compose_reply_with_mention", message_view_only: true}, // '@'
65: {name: "stream_cycle_backward", message_view_only: true}, // 'A'
67: {name: "C_deprecated", message_view_only: true}, // 'C'
68: {name: "stream_cycle_forward", message_view_only: true}, // 'D'
71: {name: "G_end", message_view_only: true}, // 'G'
73: {name: "open_inbox", message_view_only: true}, // 'I'
74: {name: "vim_page_down", message_view_only: true}, // 'J'
75: {name: "vim_page_up", message_view_only: true}, // 'K'
77: {name: "toggle_topic_visibility_policy", message_view_only: true}, // 'M'
80: {name: "narrow_private", message_view_only: true}, // 'P'
82: {name: "respond_to_author", message_view_only: true}, // 'R'
83: {name: "toggle_stream_subscription", message_view_only: true}, // 'S'
85: {name: "mark_unread", message_view_only: true}, // 'U'
86: {name: "view_selected_stream", message_view_only: false}, // 'V'
"*": {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},
"-": {name: "toggle_message_collapse", message_view_only: true},
"/": {name: "search", message_view_only: false},
":": {name: "toggle_reactions_popover", message_view_only: true},
"<": {name: "compose_forward_message", message_view_only: true},
">": {name: "compose_quote_message", message_view_only: true},
"?": {name: "show_shortcuts", message_view_only: false},
"@": {name: "compose_reply_with_mention", message_view_only: true},
A: {name: "stream_cycle_backward", message_view_only: true},
C: {name: "C_deprecated", message_view_only: true},
D: {name: "stream_cycle_forward", message_view_only: true},
G: {name: "G_end", message_view_only: true},
I: {name: "open_inbox", message_view_only: true},
J: {name: "vim_page_down", message_view_only: true},
K: {name: "vim_page_up", message_view_only: true},
M: {name: "toggle_topic_visibility_policy", message_view_only: true},
P: {name: "narrow_private", message_view_only: true},
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".
97: {name: "open_combined_feed", message_view_only: true}, // 'a'
99: {name: "compose", message_view_only: true}, // 'c'
100: {name: "open_drafts", message_view_only: true}, // 'd'
101: {name: "edit_message", message_view_only: true}, // 'e'
103: {name: "gear_menu", message_view_only: true}, // 'g'
104: {name: "vim_left", message_view_only: true}, // 'h'
105: {name: "message_actions", message_view_only: true}, // 'i'
106: {name: "vim_down", message_view_only: true}, // 'j'
107: {name: "vim_up", message_view_only: true}, // 'k'
108: {name: "vim_right", message_view_only: true}, // 'l'
109: {name: "move_message", message_view_only: true}, // 'm'
110: {name: "n_key", message_view_only: false}, // 'n'
112: {name: "p_key", message_view_only: false}, // 'p'
113: {name: "query_streams", message_view_only: true}, // 'q'
114: {name: "reply_message", message_view_only: true}, // 'r'
115: {name: "toggle_conversation_view", message_view_only: true}, // 's'
116: {name: "open_recent_view", message_view_only: true}, // 't'
117: {name: "toggle_sender_info", message_view_only: true}, // 'u'
118: {name: "show_lightbox", message_view_only: true}, // 'v'
119: {name: "query_users", message_view_only: true}, // 'w'
120: {name: "compose_private_message", message_view_only: true}, // 'x'
122: {name: "zoom_to_message_near", message_view_only: true}, // 'z'
a: {name: "open_combined_feed", message_view_only: true},
c: {name: "compose", message_view_only: true},
d: {name: "open_drafts", message_view_only: true},
e: {name: "edit_message", message_view_only: true},
g: {name: "gear_menu", message_view_only: true},
h: {name: "vim_left", message_view_only: true},
i: {name: "message_actions", message_view_only: true},
j: {name: "vim_down", message_view_only: true},
k: {name: "vim_up", message_view_only: true},
l: {name: "vim_right", message_view_only: true},
m: {name: "move_message", message_view_only: true},
n: {name: "n_key", message_view_only: false},
p: {name: "p_key", message_view_only: false},
q: {name: "query_streams", message_view_only: true},
r: {name: "reply_message", message_view_only: true},
s: {name: "toggle_conversation_view", message_view_only: true},
t: {name: "open_recent_view", message_view_only: true},
u: {name: "toggle_sender_info", message_view_only: true},
v: {name: "show_lightbox", message_view_only: true},
w: {name: "query_users", message_view_only: true},
x: {name: "compose_private_message", message_view_only: true},
z: {name: "zoom_to_message_near", message_view_only: true},
};
// Keyboard Event Viewer tool:
// https://w3c.github.io/uievents/tools/key-event-viewer.html
export function get_keydown_hotkey(e) {
let hotkey;
if (e.altKey) {
hotkey = keydown_alt_mappings[e.which];
hotkey = keydown_alt_mappings[e.key];
if (hotkey) {
return hotkey;
}
@@ -223,7 +225,7 @@ export function get_keydown_hotkey(e) {
}
if (e.ctrlKey && !e.shiftKey) {
hotkey = keydown_ctrl_mappings[e.which];
hotkey = keydown_ctrl_mappings[e.key];
if (hotkey) {
return hotkey;
}
@@ -231,7 +233,7 @@ export function get_keydown_hotkey(e) {
const isCmdOrCtrl = common.has_mac_keyboard() ? e.metaKey : e.ctrlKey;
if (isCmdOrCtrl && !e.shiftKey) {
hotkey = keydown_cmd_or_ctrl_mappings[e.which];
hotkey = keydown_cmd_or_ctrl_mappings[e.key];
if (hotkey) {
return hotkey;
}
@@ -241,20 +243,20 @@ export function get_keydown_hotkey(e) {
}
if (e.shiftKey) {
hotkey = keydown_shift_mappings[e.which];
hotkey = keydown_shift_mappings[e.key];
if (hotkey) {
return hotkey;
}
}
if (!e.shiftKey) {
hotkey = keydown_unshift_mappings[e.which];
hotkey = keydown_unshift_mappings[e.key];
if (hotkey) {
return hotkey;
}
}
return keydown_either_mappings[e.which];
return keydown_either_mappings[e.key];
}
export function get_keypress_hotkey(e) {
@@ -262,7 +264,7 @@ export function get_keypress_hotkey(e) {
return undefined;
}
return keypress_mappings[e.which];
return keypress_mappings[e.key];
}
export let processing_text = () => {
@@ -407,7 +409,7 @@ export function process_escape_key(e) {
/* The Ctrl+[ hotkey navigates to the home view
* unconditionally; Esc's behavior depends on a setting. */
if (user_settings.web_escape_navigates_to_home_view || e.which === 219) {
if (user_settings.web_escape_navigates_to_home_view || e.key === "[") {
const triggered_by_escape_key = true;
hashchange.set_hash_to_home_view(triggered_by_escape_key);
return true;

View File

@@ -147,16 +147,16 @@ function test_while_not_editing_text(label, f) {
}
run_test("mappings", () => {
function map_press(which, shiftKey) {
function map_press(key, shiftKey) {
return hotkey.get_keypress_hotkey({
which,
key,
shiftKey,
});
}
function map_down(which, shiftKey, ctrlKey, metaKey, altKey) {
function map_down(key, shiftKey, ctrlKey, metaKey, altKey) {
return hotkey.get_keydown_hotkey({
which,
key,
shiftKey,
ctrlKey,
metaKey,
@@ -166,75 +166,76 @@ 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(33), undefined);
assert.equal(map_press("!"), undefined);
// Test page-up does work.
assert.equal(map_down(33).name, "page_up");
assert.equal(map_down("PageUp").name, "page_up");
// Test other mappings.
assert.equal(map_down(9).name, "tab");
assert.equal(map_down(9, true).name, "shift_tab");
assert.equal(map_down(27).name, "escape");
assert.equal(map_down(37).name, "left_arrow");
assert.equal(map_down(13).name, "enter");
assert.equal(map_down(46).name, "delete");
assert.equal(map_down(13, true).name, "enter");
assert.equal(map_down(78, true).name, "narrow_to_next_unread_followed_topic");
assert.equal(map_down(86, true).name, "toggle_read_receipts"); // Shift + V
assert.equal(map_down("Tab").name, "tab");
assert.equal(map_down("Tab", true).name, "shift_tab");
assert.equal(map_down("Escape").name, "escape");
assert.equal(map_down("ArrowLeft").name, "left_arrow");
assert.equal(map_down("Enter").name, "enter");
assert.equal(map_down("Delete").name, "delete");
assert.equal(map_down("Enter", true).name, "enter");
assert.equal(map_down("N", true).name, "narrow_to_next_unread_followed_topic");
assert.equal(map_down("V", true).name, "toggle_read_receipts");
assert.equal(map_press(47).name, "search"); // slash
assert.equal(map_press(106).name, "vim_down"); // j
assert.equal(map_press("/").name, "search");
assert.equal(map_press("j").name, "vim_down");
assert.equal(map_down(219, false, true).name, "escape"); // Ctrl + [
assert.equal(map_down(67, false, true).name, "copy_with_c"); // Ctrl + C
assert.equal(map_down(75, false, true).name, "search_with_k"); // Ctrl + K
assert.equal(map_down(83, false, true).name, "star_message"); // Ctrl + S
assert.equal(map_down(190, false, true).name, "narrow_to_compose_target"); // Ctrl + .
assert.equal(map_down("[", false, true).name, "escape");
assert.equal(map_down("c", false, true).name, "copy_with_c");
assert.equal(map_down("k", false, true).name, "search_with_k");
assert.equal(map_down("s", false, true).name, "star_message");
assert.equal(map_down(".", false, true).name, "narrow_to_compose_target");
assert.equal(map_down(80, false, false, false, true).name, "toggle_compose_preview"); // Alt + P
assert.equal(map_down("p", false, false, false, true).name, "toggle_compose_preview"); // Alt + P
// More negative tests.
assert.equal(map_down(47), undefined);
assert.equal(map_press(27), undefined);
assert.equal(map_down(27, true), undefined);
assert.equal(map_down(86, false, true), undefined); // Ctrl + V
assert.equal(map_down(90, false, true), undefined); // Ctrl + Z
assert.equal(map_down(84, false, true), undefined); // Ctrl + T
assert.equal(map_down(82, false, true), undefined); // Ctrl + R
assert.equal(map_down(79, false, true), undefined); // Ctrl + O
assert.equal(map_down(80, false, true), undefined); // Ctrl + P
assert.equal(map_down(65, false, true), undefined); // Ctrl + A
assert.equal(map_down(70, false, true), undefined); // Ctrl + F
assert.equal(map_down(72, false, true), undefined); // Ctrl + H
assert.equal(map_down(88, false, true), undefined); // Ctrl + X
assert.equal(map_down(78, false, true), undefined); // Ctrl + N
assert.equal(map_down(77, false, true), undefined); // Ctrl + M
assert.equal(map_down(67, false, false, true), undefined); // Cmd + C
assert.equal(map_down(75, false, false, true), undefined); // Cmd + K
assert.equal(map_down(83, false, false, true), undefined); // Cmd + S
assert.equal(map_down(75, true, true), undefined); // Shift + Ctrl + K
assert.equal(map_down(83, true, true), undefined); // Shift + Ctrl + S
assert.equal(map_down(219, true, true, false), undefined); // Shift + Ctrl + [
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);
assert.equal(map_down("t", false, true), undefined);
assert.equal(map_down("r", false, true), undefined);
assert.equal(map_down("o", false, true), undefined);
assert.equal(map_down("p", false, true), undefined);
assert.equal(map_down("a", false, true), undefined);
assert.equal(map_down("f", false, true), undefined);
assert.equal(map_down("h", false, true), undefined);
assert.equal(map_down("x", false, true), undefined);
assert.equal(map_down("n", false, true), undefined);
assert.equal(map_down("m", false, true), undefined);
assert.equal(map_down("c", false, false, true), undefined);
assert.equal(map_down("k", false, false, true), undefined);
assert.equal(map_down("s", false, false, true), undefined);
assert.equal(map_down("K", true, true), undefined);
assert.equal(map_down("S", true, true), undefined);
assert.equal(map_down("[", true, true, false), undefined);
assert.equal(map_down("P", true, false, false, true), undefined);
// Cmd tests for MacOS
navigator.platform = "MacIntel";
assert.equal(map_down(219, false, true, false).name, "escape"); // Ctrl + [
assert.equal(map_down(219, false, false, true), undefined); // Cmd + [
assert.equal(map_down(67, false, true, true).name, "copy_with_c"); // Ctrl + C
assert.equal(map_down(67, false, true, false), undefined); // Cmd + C
assert.equal(map_down(75, false, false, true).name, "search_with_k"); // Cmd + K
assert.equal(map_down(75, false, true, false), undefined); // Ctrl + K
assert.equal(map_down(83, false, false, true).name, "star_message"); // Cmd + S
assert.equal(map_down(83, false, true, false), undefined); // Ctrl + S
assert.equal(map_down(190, false, false, true).name, "narrow_to_compose_target"); // Cmd + .
assert.equal(map_down(190, false, true, false), undefined); // Ctrl + .
assert.equal(map_down("[", false, true, false).name, "escape");
assert.equal(map_down("[", false, false, true), undefined);
assert.equal(map_down("c", false, true, true).name, "copy_with_c");
assert.equal(map_down("c", false, true, false), undefined);
assert.equal(map_down("k", false, false, true).name, "search_with_k");
assert.equal(map_down("k", false, true, false), undefined);
assert.equal(map_down("s", false, false, true).name, "star_message");
assert.equal(map_down("s", false, true, false), undefined);
assert.equal(map_down(".", false, false, true).name, "narrow_to_compose_target");
assert.equal(map_down(".", false, true, false), undefined);
// Reset platform
navigator.platform = "";
});
function process(s, shiftKey, keydown = false) {
const e = {
which: s.codePointAt(0),
key: s,
shiftKey,
};
try {
@@ -247,7 +248,7 @@ function process(s, shiftKey, keydown = false) {
// function is called than the one declared. Try to
// provide a useful error message.
// add a newline to separate from other console output.
console.log('\nERROR: Mapping for character "' + e.which + '" does not match tests.');
console.log('\nERROR: Mapping for character "' + e.key + '" does not match tests.');
throw error;
}
}
@@ -459,21 +460,21 @@ test_while_not_editing_text("narrow next unread followed topic", () => {
test_while_not_editing_text("motion_keys", () => {
$.create(".navbar-item:focus", {children: []});
const codes = {
down_arrow: 40,
end: 35,
home: 36,
left_arrow: 37,
right_arrow: 39,
page_up: 33,
page_down: 34,
spacebar: 32,
up_arrow: 38,
const keys = {
down_arrow: "ArrowDown",
end: "End",
home: "Home",
left_arrow: "ArrowLeft",
right_arrow: "ArrowRight",
page_up: "PageUp",
page_down: "PageDown",
spacebar: " ",
up_arrow: "ArrowUp",
};
function process(name) {
const e = {
which: codes[name],
key: keys[name],
};
try {
@@ -483,7 +484,7 @@ test_while_not_editing_text("motion_keys", () => {
// function is called than the one declared. Try to
// provide a useful error message.
// add a newline to separate from other console output.
console.log('\nERROR: Mapping for character "' + e.which + '" does not match tests.');
console.log('\nERROR: Mapping for character "' + e.key + '" does not match tests.');
throw error;
}
}
@@ -566,7 +567,7 @@ run_test("test new user input hook called", () => {
hook_called = true;
});
hotkey.process_keydown({which: "S".codePointAt(0)});
hotkey.process_keydown({key: "S"});
assert.ok(hook_called);
});
@@ -579,7 +580,7 @@ test_while_not_editing_text("e shortcut works for anonymous users", ({override_r
overlays.settings_open = () => false;
const e = {
which: "e".codePointAt(0),
key: "e",
};
stubbing(message_edit, "start", (stub) => {