RichText: remove selection change listener during composition #14449
Conversation
ellatrix
added
[Feature] Rich Text
ellatrix
force-pushed
the
fix/composing-input-selection
branch
from
c43764b
to
9c8585a
Compare
youknowriad
added
the
[Release] Do Not Punt
| @@ -373,6 +373,8 @@ export class RichText extends Component { | |||
| // Browsers setting `isComposing` to `true` will usually emit a final | |||
| // `input` event when the characters are composed. | |||
| if ( event && event.nativeEvent.isComposing ) { | |||
| // Also don't update any selection. | |||
| document.removeEventListener( 'selectionchange', this.onSelectionChange ); | |||
There was a problem hiding this comment.
Do you think there's a simpler way togo instead of this selection enabling/disabling? maybe a flag?
There was a problem hiding this comment.
I see this as the simplest way. There's no need to run a listener if it's not needed?
There was a problem hiding this comment.
I mean, I think it's "cleanliest" if you remove listeners if you don't need them, add them back if you do need them again. I made the mistake before not to do that, and it had a negative performance impact (#12480), so I make a habit of taking this "cleaner" approach.
|
@youknowriad Thanks for the review! To answer your question: the browser always triggers a |
Description
This should be included in WP 5.2. It worked fine in WP 5.1.
Fixes #14434.
This bug can also be reproduced with other keyboards, including an American keyboard.
option+`. You started composing a character.a.Expected:
à. Actual:`à.Cause: the selection change event listener is called after input, which overwrites the DOM if the selection state is different (and it is different, but shouldn't be recorded). The overwrite happens for the format boundaries.
Solution: remove the selection change listener during composition.
Includes a minor clean up, as I was experimenting with different events bound to
Editable.How has this been tested?
See above.
Screenshots
Types of changes
Checklist: