RichText: Fix dead key input on Windows #37777
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
See #35086 In `useSpace`, don't intercept character composition that involves the Space key. For example, to input a standalone diacritic (like ´ or `) using a keyboard with dead keys, one must first press the dead key and then press the Space key. Many operating systems handle this in such a way that the second KeyboardEvent contains the property `keyCode: 229`. According to the spec, 229 allows the system to indicate that an Input Method Editor (IDE) is processing some key input. However, Windows doesn't use `keyCode: 229` for dead key composition, instead emitting an event with values `keyCode: SPACE` and `key: '´'`. That is why checking the `key` property for values other than `SPACE` is important. This should serve as a reminder that the `KeyboardEvent.keyCode` attribute is officially deprecated and that we should consider more consistent interfaces.
mcsf
added
[Type] Bug
|
Size Change: +7 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
lukegb
approved these changes
Jan 7, 2022
ellatrix
reviewed
Jan 10, 2022
| @@ -21,13 +21,38 @@ export function useSpace() { | |||
| return; | |||
| } | |||
|
|
|||
| const { keyCode, altKey, metaKey, ctrlKey } = event; | |||
| const { keyCode, altKey, metaKey, ctrlKey, key } = event; | |||
|
|
|||
| // Only consider the space key without modifiers pressed. | |||
| if ( keyCode !== SPACE || altKey || metaKey || ctrlKey ) { | |||
There was a problem hiding this comment.
Should we remove keyCode !== SPACE, or will that cause problems?
There was a problem hiding this comment.
Right now I just patched the existing approach, so I wouldn't touch that condition. But ideally we would ditch keyCode altogether and rely mostly on key, which is the main non-deprecated property for this stuff. Then we'll know what shortcomings key presents, and we could mitigate them with properties like code, location, etc.
There was a problem hiding this comment.
We should also mind the composition events.
There was a problem hiding this comment.
I was just reading this interesting thread on keydown and event.key - https://twitter.com/jaffathecake/status/1480493537112248325.
ellatrix
approved these changes
Jan 10, 2022
|
@noisysocks should we pick this up for RC 2? |
jorgefilipecosta
pushed a commit
that referenced
this pull request
Jan 10, 2022
youknowriad
added
the
Backport to WP 6.7 Beta/RC
|
I also support backporting this one as it's a very bad bug. |
noisysocks
removed
the
Backport to WP 6.7 Beta/RC
noisysocks
pushed a commit
that referenced
this pull request
Jan 10, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #35086
Description
In
useSpace, don't intercept character composition that involves the Space key. For example, to input a standalone diacritic (like ´ or `) using a keyboard with dead keys, one must first press the dead key and then press the Space key.Many operating systems handle this in such a way that the second KeyboardEvent contains the property
keyCode: 229. According to the spec, 229 allows the system to indicate that an Input Method Editor (IDE) is processing some key input.However, Windows doesn't use
keyCode: 229for dead key composition, instead emitting an event with valueskeyCode: SPACEandkey: '´'. That is why checking thekeyproperty for values other thanSPACEis important.This should serve as a reminder that the
KeyboardEvent.keyCodeattribute is officially deprecated and that we should consider more consistent interfaces.cc @ellatrix @Flameeyes
How has this been tested?
See parent issue (#35086).
Screenshots
Types of changes
Bug fix (regression) affecting a particular system.
Checklist:
*.native.jsfiles for terms that need renaming or removal).