Skip to content

Commit e97f2e2

Browse files
fix(web): ignore keyboard events with a missing key
Chrome reports TypeError when shortcut listeners call .length or .toLowerCase on KeyboardEvent.key that some browsers/extensions leave unset. Treat a missing key as empty so those listeners no-op. Co-authored-by: Tyler Dane <tyler-dane@users.noreply.github.com>
1 parent fc0112d commit e97f2e2

13 files changed

Lines changed: 171 additions & 28 deletions

File tree

packages/web/src/__tests__/utils/keyboard.test.util.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,3 +26,18 @@ export function pressKey(
2626
}),
2727
);
2828
}
29+
30+
/** Some browsers fire KeyboardEvents with `key` unset. */
31+
export function dispatchMissingKey(
32+
type: "keydown" | "keyup",
33+
target: Element | Node | Window | Document = document,
34+
) {
35+
const event = new KeyboardEvent(type, {
36+
bubbles: true,
37+
cancelable: true,
38+
composed: true,
39+
});
40+
Object.defineProperty(event, "key", { get: () => undefined });
41+
target.dispatchEvent(event);
42+
return event;
43+
}

packages/web/src/components/OnboardingChecklist/useChecklistDetection.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { isEditableKeyboardTarget } from "@web/common/utils/form/form.util";
55
import { checklistActions } from "@web/components/OnboardingChecklist/checklist.store";
66
import { useDraftStore } from "@web/events/stores/draft.store";
77
import { useEdgeFocusStore } from "@web/grid/shortcuts/edge-focus.store";
8+
import { keyboardKey } from "@web/shortcuts/is-bare-letter-key";
89
import { useEventJumpStore } from "@web/shortcuts/shift-hint/event-jump.store";
910

1011
const ARROW_KEYS = new Set(["ArrowUp", "ArrowDown", "ArrowLeft", "ArrowRight"]);
@@ -42,7 +43,7 @@ export function useChecklistDetection(enabled: boolean) {
4243
if (
4344
(event.metaKey || event.ctrlKey) &&
4445
!event.shiftKey &&
45-
event.key.toLowerCase() === "z"
46+
keyboardKey(event).toLowerCase() === "z"
4647
) {
4748
checklistActions.completeItem("undo");
4849
return;

packages/web/src/components/ShortcutShowcase/ShortcutShowcase.tsx

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,10 @@ import {
4848
} from "@web/components/ShortcutShowcase/showcase.store";
4949
import { ShortcutKeys } from "@web/components/Shortcuts/ShortcutKeys";
5050
import { useAppLockReason } from "@web/shortcuts/app-lock";
51-
import { isBareLetterKey } from "@web/shortcuts/is-bare-letter-key";
51+
import {
52+
isBareLetterKey,
53+
keyboardKey,
54+
} from "@web/shortcuts/is-bare-letter-key";
5255
import { KEYMAP } from "@web/shortcuts/keymap";
5356
import { ShortcutTipParts } from "@web/shortcuts/tips/ShortcutTipParts";
5457
import { ARM_WINDOW_MS } from "@web/shortcuts/useEditSequenceShortcut";
@@ -272,7 +275,7 @@ const ShowcaseTakeover: FC = () => {
272275
) {
273276
event.preventDefault();
274277
const hints = Object.values(practiceRef.current.jumpChips);
275-
const typed = jumpBufferRef.current + event.key.toLowerCase();
278+
const typed = jumpBufferRef.current + keyboardKey(event).toLowerCase();
276279
if (hints.includes(typed)) {
277280
jumpBufferRef.current = "";
278281
apply((state) => jumpToChipHint(state, typed));
@@ -306,7 +309,10 @@ const ShowcaseTakeover: FC = () => {
306309
return;
307310
}
308311

309-
if ((event.metaKey || event.ctrlKey) && event.key.toLowerCase() === "z") {
312+
if (
313+
(event.metaKey || event.ctrlKey) &&
314+
keyboardKey(event).toLowerCase() === "z"
315+
) {
310316
event.preventDefault();
311317
const before = practiceRef.current;
312318
if (event.shiftKey) {

packages/web/src/components/WelcomeModal/WelcomeModal.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { useAuthModal } from "@web/components/AuthModal/hooks/useAuthModal";
1313
import { OverlayPanel } from "@web/components/OverlayPanel/OverlayPanel";
1414
import { shortcutShowcaseActions } from "@web/components/ShortcutShowcase/showcase.store";
1515
import { ShortcutHint } from "@web/components/Shortcuts/ShortcutHint";
16+
import { keyboardKey } from "@web/shortcuts/is-bare-letter-key";
1617
import { PixelPirate } from "./PixelPirate";
1718
import { WelcomeGuideBody } from "./WelcomeGuideBody";
1819
import { hasSeenWelcome, markWelcomeSeen } from "./welcome.modal.util";
@@ -66,7 +67,7 @@ export function WelcomeModal() {
6667

6768
const handleShortcutKey = (e: React.KeyboardEvent) => {
6869
if (e.metaKey || e.ctrlKey || e.altKey) return;
69-
const key = e.key.toLowerCase();
70+
const key = keyboardKey(e).toLowerCase();
7071
if (key === "u") {
7172
e.preventDefault();
7273
handOffToAuth("sign_up");
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
import {
2+
isBareLetterKey,
3+
keyboardKey,
4+
normalizedKeyboardKey,
5+
} from "@web/shortcuts/is-bare-letter-key";
6+
import { describe, expect, it } from "bun:test";
7+
8+
const eventWithKey = (key: unknown): KeyboardEvent => {
9+
const event = new KeyboardEvent("keydown", {
10+
bubbles: true,
11+
cancelable: true,
12+
});
13+
Object.defineProperty(event, "key", { get: () => key });
14+
return event;
15+
};
16+
17+
describe("keyboardKey", () => {
18+
it("returns the key when it is a string", () => {
19+
expect(keyboardKey(eventWithKey("e"))).toBe("e");
20+
});
21+
22+
it("returns empty string when key is missing", () => {
23+
expect(keyboardKey(eventWithKey(undefined))).toBe("");
24+
expect(keyboardKey(eventWithKey(null))).toBe("");
25+
});
26+
});
27+
28+
describe("normalizedKeyboardKey", () => {
29+
it("lowercases single-character keys", () => {
30+
expect(normalizedKeyboardKey(eventWithKey("E"))).toBe("e");
31+
});
32+
33+
it("leaves named keys unchanged", () => {
34+
expect(normalizedKeyboardKey(eventWithKey("Escape"))).toBe("Escape");
35+
});
36+
37+
it("does not throw when key is missing", () => {
38+
expect(normalizedKeyboardKey(eventWithKey(undefined))).toBe("");
39+
});
40+
});
41+
42+
describe("isBareLetterKey", () => {
43+
it("matches an unmodified letter", () => {
44+
expect(isBareLetterKey(eventWithKey("s"), "s")).toBe(true);
45+
expect(isBareLetterKey(eventWithKey("S"), "s")).toBe(true);
46+
});
47+
48+
it("returns false instead of throwing when key is missing", () => {
49+
expect(isBareLetterKey(eventWithKey(undefined), "s")).toBe(false);
50+
});
51+
});
Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,29 @@
1+
/**
2+
* Some browsers, extensions, and IME/autofill paths fire KeyboardEvents with
3+
* `key` unset. Calling `.length` / `.toLowerCase()` on that value is a
4+
* TypeError and is what production error tracking reports as
5+
* "Cannot read properties of undefined (reading 'length'|'toLowerCase')".
6+
*/
7+
export const keyboardKey = (event: Pick<KeyboardEvent, "key">): string =>
8+
typeof event.key === "string" ? event.key : "";
9+
10+
/** Lowercase a single-character key; leave named keys (Escape, ArrowUp) as-is. */
11+
export const normalizedKeyboardKey = (
12+
event: Pick<KeyboardEvent, "key">,
13+
): string => {
14+
const key = keyboardKey(event);
15+
return key.length === 1 ? key.toLowerCase() : key;
16+
};
17+
118
/** True for an unmodified single-letter key matching `letter` (case-insensitive). */
2-
export const isBareLetterKey = (event: KeyboardEvent, letter: string) =>
3-
event.key.length === 1 &&
4-
event.key.toLowerCase() === letter &&
5-
!event.metaKey &&
6-
!event.ctrlKey &&
7-
!event.altKey &&
8-
!event.shiftKey;
19+
export const isBareLetterKey = (event: KeyboardEvent, letter: string) => {
20+
const key = keyboardKey(event);
21+
return (
22+
key.length === 1 &&
23+
key.toLowerCase() === letter &&
24+
!event.metaKey &&
25+
!event.ctrlKey &&
26+
!event.altKey &&
27+
!event.shiftKey
28+
);
29+
};

packages/web/src/shortcuts/keyboard-only/useKeyboardOnlyMode.test.tsx

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { act, renderHook } from "@testing-library/react";
2+
import { dispatchMissingKey } from "@web/__tests__/utils/keyboard.test.util";
23
import { clearAppLockReasons, setAppLockReason } from "@web/shortcuts/app-lock";
34
import {
45
initialKeyboardOnlyState,
@@ -198,4 +199,15 @@ describe("useKeyboardOnlyMode", () => {
198199
expect(useKeyboardOnlyStore.getState().isActive).toBe(true);
199200
expect(useEventJumpStore.getState().isActive).toBe(false);
200201
});
202+
203+
it("ignores KeyboardEvents with no key instead of throwing", () => {
204+
renderHook(() => useKeyboardOnlyMode());
205+
206+
expect(() => {
207+
dispatchMissingKey("keydown");
208+
dispatchMissingKey("keyup");
209+
}).not.toThrow();
210+
211+
expect(useKeyboardOnlyStore.getState().isActive).toBe(false);
212+
});
201213
});

packages/web/src/shortcuts/shift-hint/useShiftHoldEventHints.test.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { act, cleanup, renderHook } from "@testing-library/react";
22
import { EventIdSchema } from "@core/types/domain-primitives";
3+
import { dispatchMissingKey } from "@web/__tests__/utils/keyboard.test.util";
34
import { type GridEvent } from "@web/common/types/web.event.types";
45
import { clearAppLockReasons, setAppLockReason } from "@web/shortcuts/app-lock";
56
import {
@@ -262,4 +263,16 @@ describe("useShiftHoldEventHints", () => {
262263
expect(useEventJumpStore.getState().isActive).toBe(false);
263264
expect(result.current.hints).toEqual([]);
264265
});
266+
267+
it("ignores KeyboardEvents with no key instead of throwing", () => {
268+
const { result } = mountHints();
269+
270+
expect(() => {
271+
dispatchMissingKey("keydown");
272+
dispatchMissingKey("keyup");
273+
}).not.toThrow();
274+
275+
expect(useEventJumpStore.getState().isActive).toBe(false);
276+
expect(result.current.hints).toEqual([]);
277+
});
265278
});

packages/web/src/shortcuts/shift-hint/useShiftHoldEventHints.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { type GridEvent } from "@web/common/types/web.event.types";
55
import { isEditableKeyboardTarget } from "@web/common/utils/form/form.util";
66
import { isAppLocked } from "@web/shortcuts/app-lock";
77
import { isHigherEscapeOwner } from "@web/shortcuts/escape-ownership";
8-
import { isBareLetterKey } from "@web/shortcuts/is-bare-letter-key";
8+
import {
9+
isBareLetterKey,
10+
keyboardKey,
11+
normalizedKeyboardKey,
12+
} from "@web/shortcuts/is-bare-letter-key";
913
import { KEYMAP } from "@web/shortcuts/keymap";
1014
import {
1115
assignDayJumpKeys,
@@ -280,7 +284,7 @@ export function useShiftHoldEventHints({
280284
}
281285

282286
// Arrows keep mode on so letter-then-arrows can move focus.
283-
if (event.key.startsWith("Arrow")) {
287+
if (keyboardKey(event).startsWith("Arrow")) {
284288
clearAmbiguousCommitTimer();
285289
stripDigitBuffer();
286290
return;
@@ -290,7 +294,7 @@ export function useShiftHoldEventHints({
290294
return;
291295
}
292296

293-
const key = event.key.length === 1 ? event.key.toLowerCase() : event.key;
297+
const key = normalizedKeyboardKey(event);
294298
if (key.length !== 1) return;
295299

296300
// Swallow j/k and other unmatched printable shortcuts while jump is on.
@@ -359,7 +363,7 @@ export function useShiftHoldEventHints({
359363
};
360364

361365
const onKeyUp = (event: KeyboardEvent) => {
362-
const key = event.key.length === 1 ? event.key.toLowerCase() : event.key;
366+
const key = normalizedKeyboardKey(event);
363367
if (!suppressKeyUpRef.current.has(key)) return;
364368
suppressKeyUpRef.current.delete(key);
365369
event.preventDefault();

packages/web/src/shortcuts/tips/useShortcutTipTrigger.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
selectIsEventFormOpen,
55
useDraftStore,
66
} from "@web/events/stores/draft.store";
7+
import { keyboardKey } from "@web/shortcuts/is-bare-letter-key";
78
import {
89
selectActiveShortcutTipId,
910
shortcutTipsActions,
@@ -45,13 +46,13 @@ export function useShortcutTipTrigger() {
4546
activeTipId === "edit-sequence" &&
4647
!event.shiftKey &&
4748
!event.altKey &&
48-
event.key.toLowerCase() === "e"
49+
keyboardKey(event).toLowerCase() === "e"
4950
) {
5051
shortcutTipsActions.actedOn("edit-sequence");
5152
} else if (
5253
activeTipId === "nudge" &&
5354
event.shiftKey &&
54-
event.key.startsWith("Arrow")
55+
keyboardKey(event).startsWith("Arrow")
5556
) {
5657
shortcutTipsActions.actedOn("nudge");
5758
} else if (activeTipId === "target-event" && event.key === "Shift") {

0 commit comments

Comments
 (0)