Skip to content
This repository has been archived by the owner on Feb 6, 2023. It is now read-only.

Unlucky keypresses get ignored due to buggy bailout logic #1830

Closed
sophiebits opened this issue Aug 7, 2018 · 0 comments
Closed

Unlucky keypresses get ignored due to buggy bailout logic #1830

sophiebits opened this issue Aug 7, 2018 · 0 comments

Comments

@sophiebits
Copy link
Contributor

sophiebits commented Aug 7, 2018

If you have an editor containing

ABC
defghi

and replace "d" with "A", or "e" with "B", or "f" with "C" by selecting the character and typing the new one, nothing happens.

That's because this check mistakenly slices relative to the entire editor content but the indices it's using are relative to a particular block:

https://github.com/facebook/draft-js/blob/2d7ad181d60b97a87e2f6fff6557ae31c02a8749/src/component/handlers/edit/editOnBeforeInput.js#L120-L127

alicayan008 pushed a commit to alicayan008/draft-js that referenced this issue Jul 4, 2023
Summary:
Fixes facebookarchive/draft-js#1830 (bug introduced in facebookarchive/draft-js#719; this is a proper fix for facebookarchive/draft-js#398).

* When we're allowing native insertion, we don't want to modify the selection because doing so blocks browser features like spellcheck from working
* In all other cases when typing, we *do* want to force the selection – failing to do this is why the original issue occurred
* We use the `insert-characters` change type for both native and non-native insertions
* Instead of guessing in `EditorState.push` whether to force a selection, accept it as a parameter
* Gate all changes by `draft_non_native_insertion_forces_selection`

I verified with the feature flag disabled (same behavior as before):

* Typing over a character works, and the cursor moves as expected (because of the `chars === currentlySelectedChars` check)
* With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *fails* to replace the char (because that check is buggy).

And with the feature flag enabled (new behavior):

* Typing over a character still works (but does so without the buggy check)
* With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *succeeds* at replacing the char.

Reviewed By: flarnie

Differential Revision: D9209691

fbshipit-source-id: f63551dbad689391aa9c2a69f1d6ba395b8bf1ac
aforismesen added a commit to aforismesen/draft-js that referenced this issue Jul 12, 2024
Summary:
Fixes facebookarchive/draft-js#1830 (bug introduced in facebookarchive/draft-js#719; this is a proper fix for facebookarchive/draft-js#398).

* When we're allowing native insertion, we don't want to modify the selection because doing so blocks browser features like spellcheck from working
* In all other cases when typing, we *do* want to force the selection – failing to do this is why the original issue occurred
* We use the `insert-characters` change type for both native and non-native insertions
* Instead of guessing in `EditorState.push` whether to force a selection, accept it as a parameter
* Gate all changes by `draft_non_native_insertion_forces_selection`

I verified with the feature flag disabled (same behavior as before):

* Typing over a character works, and the cursor moves as expected (because of the `chars === currentlySelectedChars` check)
* With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *fails* to replace the char (because that check is buggy).

And with the feature flag enabled (new behavior):

* Typing over a character still works (but does so without the buggy check)
* With editor contents "ABC\ndef", replacing "d", "e", or "f" with the character above *succeeds* at replacing the char.

Reviewed By: flarnie

Differential Revision: D9209691

fbshipit-source-id: f63551dbad689391aa9c2a69f1d6ba395b8bf1ac
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant