New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress composition events default prevented on previous keydown #15266

Merged
merged 1 commit into from Aug 12, 2017

Conversation

Projects
None yet
1 participant
@nathansobo
Contributor

nathansobo commented Aug 11, 2017

Fixes #15189

It seems like Chrome 56 has a regression that causes compositionstart and compositionupdate events to still be fired even if preventDefault is called on the original keydown event. This PR introduces a workaround that suppresses handling of these events if defaultPrevented is true on the most recent keydown event.

/cc @Ben3eeE @ungb

@nathansobo nathansobo merged commit 592d174 into master Aug 12, 2017

3 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@nathansobo nathansobo deleted the ns-fix-alt-bindings-on-mac branch Aug 12, 2017

nathansobo added a commit that referenced this pull request Aug 12, 2017

Merge pull request #15266 from atom/ns-fix-alt-bindings-on-mac
Suppress composition events default prevented on previous keydown

nathansobo added a commit that referenced this pull request Aug 12, 2017

Merge pull request #15266 from atom/ns-fix-alt-bindings-on-mac
Suppress composition events default prevented on previous keydown

as-cii added a commit that referenced this pull request Aug 15, 2017

Suppress text input for default-prevented keydown events
This is a continuation of #15266. In that pull-request we managed to
prevent IME previews from being displayed in the editor when the
originating `keydown` event was default-prevented. However, it was still
possible for IME input to make it through the previous workarounds, thus
triggering the `textInput` event and showing unwanted text.

Pressing another key that would abort the in-progress IME input would,
in fact, first replace `this.lastKeydown` and then trigger the
`textInput` event. In the handling of that event we would detect
`this.lastKeydown` as "non-default-prevented" and therefore mistakenly
insert it.

With this commit we are adopting a different strategy to mitigate the
issue. When receiving the wrong `compositionupdate` event we will first
disable the hidden input and then re-enable it on the next tick.
Disabling the input causes the in-progress IME input to be aborted and
the browser to never fire `textInput` nor `compositionupdate` events
anymore after that.

The only downside of this approach is that the hidden input also loses
focus, but we transfer it back to it as soon as the next tick of the
event loop is served and the input has been re-enabled.

as-cii added a commit that referenced this pull request Aug 15, 2017

Suppress text input for default-prevented keydown events
This is a continuation of #15266. In that pull-request we managed to
prevent IME previews from being displayed in the editor when the
originating `keydown` event was default-prevented. However, it was still
possible for IME input to make it through the previous workarounds, thus
triggering the `textInput` event and showing unwanted text.

Pressing another key that would complete the in-progress IME input
would, in fact, first replace `this.lastKeydown` and then trigger the
`textInput` event. In the handling of that event we would detect
`this.lastKeydown` as "non-default-prevented" and therefore mistakenly
insert the IME text.

With this commit we are adopting a different strategy to mitigate the
issue. When receiving the wrong `compositionupdate` event we will first
disable the hidden input and then re-enable it on the next tick.
Disabling the input causes the in-progress IME input to be aborted and
the browser to never fire `textInput` nor `compositionupdate` events
anymore after that.

The only downside of this approach is that the hidden input also loses
focus, but we transfer it back to it as soon as the next tick of the
event loop is served and the input has been re-enabled.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment