Commit 54cae019 authored by Robert Knight's avatar Robert Knight

Fix error if `matchShortcut` is called with non-KeyboardEvent

Work around an exception when testing for shortcuts in `keydown` event handlers,
if the handler is called with an argument that is not a `KeyboardEvent`, and is
missing the expected `key` property. The workaround has been added to
`matchShortcut` because it is easier to do there than remember for each
`keydown` event handler, although there is a risk that other code in these
handlers could get caught out by the same issue.

I was able to reproduce this problem locally in Chrome 118 when using autofill
to insert text into the search input field (see comments), though there might be
other causes as well.

This should fix https://hypothesis.sentry.io/issues/3987386810/?project=69811
parent 42858d04
...@@ -24,6 +24,14 @@ const modifiers = { ...@@ -24,6 +24,14 @@ const modifiers = {
* would be "!" instead. See also https://github.com/w3c/uievents/issues/247. * would be "!" instead. See also https://github.com/w3c/uievents/issues/247.
*/ */
export function matchShortcut(event: KeyboardEvent, shortcut: string): boolean { export function matchShortcut(event: KeyboardEvent, shortcut: string): boolean {
// Work around an issue where Chrome autofill can dispatch "keydown" events
// with an argument that is not a `KeyboardEvent`.
//
// See https://bugs.chromium.org/p/chromium/issues/detail?id=739792.
if (!(event instanceof KeyboardEvent)) {
return false;
}
const parts = shortcut.split('+').map(p => p.toLowerCase()); const parts = shortcut.split('+').map(p => p.toLowerCase());
let requiredModifiers = 0; let requiredModifiers = 0;
......
...@@ -4,13 +4,13 @@ import { act } from 'preact/test-utils'; ...@@ -4,13 +4,13 @@ import { act } from 'preact/test-utils';
import { matchShortcut, installShortcut, useShortcut } from '../shortcut'; import { matchShortcut, installShortcut, useShortcut } from '../shortcut';
function createEvent(key, { ctrl, alt, shift, meta } = {}) { function createEvent(key, { ctrl, alt, shift, meta } = {}) {
return { return new KeyboardEvent('keydown', {
key, key,
ctrlKey: !!ctrl, ctrlKey: !!ctrl,
altKey: !!alt, altKey: !!alt,
shiftKey: !!shift, shiftKey: !!shift,
metaKey: !!meta, metaKey: !!meta,
}; });
} }
describe('shared/shortcut', () => { describe('shared/shortcut', () => {
...@@ -82,6 +82,10 @@ describe('shared/shortcut', () => { ...@@ -82,6 +82,10 @@ describe('shared/shortcut', () => {
matchShortcut(createEvent('a'), 'a+b'); matchShortcut(createEvent('a'), 'a+b');
}, 'Multiple non-modifier keys specified'); }, 'Multiple non-modifier keys specified');
}); });
it('should return false if event is not a `KeyboardEvent`', () => {
assert.isFalse(matchShortcut(new Event('keydown'), 'a'));
});
}); });
/** /**
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment