Skip to content

Fix Chrome autocomplete incorrectly triggerring actions#853

Open
MatheusRich wants to merge 1 commit intohotwired:mainfrom
MatheusRich:handle-chrome-autocomplete
Open

Fix Chrome autocomplete incorrectly triggerring actions#853
MatheusRich wants to merge 1 commit intohotwired:mainfrom
MatheusRich:handle-chrome-autocomplete

Conversation

@MatheusRich
Copy link

When using a Stimulus action like

<button data-action="keydown.meta+k@window->search#open"> 

in Google Chrome, clicking an input's autocomplete suggestion unexpectedly triggers the controller action even though no key is pressed.

It seems that Chrome dispatches an Event with type === "keydown", but it is not an instance of KeyboardEvent. This happens regardless of the key filter.

Closes #852

When using a Stimulus action like <button data-action="keydown.meta+k@window->search#open"> in Google Chrome, clicking an input's autocomplete suggestion unexpectedly triggers the controller action even though no key is pressed. It seems that Chrome dispatches an Event with type === "keydown", but it is not an instance of KeyboardEvent. This happens regardless of the key filter. Closes hotwired#852
@seanpdoyle
Copy link
Contributor

@jorgemanrubia are you able to enable this PR's CI workflow?

Comment on lines +89 to +93
if (event.type === "keydown") {
// when accepting an autocomplete suggestion, Chrome dispatches a keydown event,
// but it isn't a KeyboardEvent instance
const keyboardEvent: KeyboardEvent = event instanceof KeyboardEvent ? event : new KeyboardEvent("keydown")
if (this.action.shouldIgnoreKeyboardEvent(keyboardEvent)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The underlying Action.shouldIgnoreKeyboardEvent function ultimately accesses properties on the KeyboardEvent instance, so instantiating a new KeyboardEvent instance without those properties might lead to unexpected behavior.

As an alternative, what do you think about using typecasting instead?

Suggested change
if (event.type === "keydown") {
// when accepting an autocomplete suggestion, Chrome dispatches a keydown event,
// but it isn't a KeyboardEvent instance
const keyboardEvent: KeyboardEvent = event instanceof KeyboardEvent ? event : new KeyboardEvent("keydown")
if (this.action.shouldIgnoreKeyboardEvent(keyboardEvent)) {
// When accepting an autocomplete suggestion, Chrome dispatches a keydown event that is not a KeyboardEvent instance
if (event.type === "keydown" && this.action.shouldIgnoreKeyboardEvent(event as KeyboardEvent)) {

That assumes that Chrome's synthetic event does have the properties required to behave like a KeyboardEvent. That might not be the case, but my hunch is that it'd behave better as the syntetic event than as an event that Stimulus constructs itself.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to use a newly created keyboard event where all of those keys are blank (so nothing matches). Typecasting probably works, but that synthetic event doesn't have any of the KeyboardEvent properties (besides type). The code would be a lot cleaner, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants