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

Inserting decorated emojis while using an IME crashes editor #2187

Closed
jdecked opened this issue Sep 20, 2019 · 2 comments
Closed

Inserting decorated emojis while using an IME crashes editor #2187

jdecked opened this issue Sep 20, 2019 · 2 comments
Assignees
@claudiopro
Labels
bug IME

Comments

@jdecked
Copy link

jdecked commented Sep 20, 2019 •
edited
Loading

Do you want to request a feature or report a bug?
Bug.

What is the current behavior?
Typing in a language which uses an IME, inserting an emoji directly after it (i.e. no spaces), then typing in the IME-using language again gives the following error if you've applied a decorator to the emojis:
Screen Shot 2019-09-19 at 7 34 40 PM

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. You can use this jsfiddle to get started: https://jsfiddle.net/gmertk/e61z7nfa/.

Fiddle: https://jsfiddle.net/36tvhmce/

For example, in Chinese on Mac:
js-fiddle-draft-js-chinese-bug

  1. Switch your input method to "Pinyin - Traditional".
  2. Type in the Draft.js field "yldx" (Yale University) and hit spacebar to insert the Chinese characters.
  3. Type "hhh" (for laughing face emoji) and hit spacebar to select the laughing face emoji.
  4. Type "yldx" and hit spacebar again to insert more Chinese characters.
  5. Notice the above error in the console. Also notice that you can't backspace anything you've typed.

There's similar behavior inserting Japanese:
js-fiddle-draft-js-japanese-bug

  1. Switch your input method to Hiragana.
  2. Type in the Draft.js field "watashi" (I) then hit Enter to select 私.
  3. Type "emoji" and scroll down by hitting spacebar until you land on any emoji, then hit Enter to select it.
  4. Type "watashi" and select 私 with Enter again.
  5. Notice the above error in the console. Also notice that you can't backspace anything you've typed.

What is the expected behavior?
No error when inserting characters via an IME, an emoji, and then more characters, and you can backspace everything. Here is a fiddle where I use Draft.js v0.10.5 instead and this issue is no longer present. This is what it looks like:
js-fiddle-draft-js-chinese-v10-5

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js?
Draft.js version 0.11.0
Chrome v76.0.3809.132, Firefox v69.0.1, and Safari v12.1.2 on Mac OS 10.14.6
This previously worked in Draft.js v0.10.5. The PR that changed this is #2035. Reverting the changes made to resolveComposition in DraftEditorCompositionHandler.js in that patch fixes this issue.
@robbertbrak
Copy link
Contributor

Hi @jdecked, I tried your scenarios in my fork of Draft, and they both work correctly. I suspect that this commit is the one that fixes it. Would you have time to give it a try and see if it solves your issue? If so, feel free to "borrow" the commit to open a pull request.

@claudiopro
Copy link
Contributor

Thanks for reporting this issue @jdecked, the repro steps are very well written and helped me understand the problem!

Thanks for chiming in @robbertbrak, I was able to verify that the suggested check does indeed prevent the error reported by Justine. While I'm not fond of silently ignoring errors, recovering from the error returning the previous selection seems safe enough to unbreak the scenario of this issue.

I'll prepare a PR shortly

@claudiopro claudiopro mentioned this issue Sep 20, 2019
Closed
@claudiopro claudiopro self-assigned this Sep 20, 2019
jdecked pushed a commit to twitter-forks/draft-js that referenced this issue Oct 9, 2019
@jdecked
fix(selection): Returns previous selection if either leaf is null (fa…
948d9df 
…cebookarchive#2189)

Summary:
**Summary**

Bails out early from the `getUpdatedSelectionState` method with previous selection when `anchorLeaf` or `focusLeaf` are null to prevent an error surfaced when using a decorator for emoji inserted between two spans of text entered via IME.

Thanks to robbertbrak for suggesting and cherry-picking a fix from their fork 🎉

Fixes facebookarchive#2187

**Test Plan**
1. Build Draft.js
1. Inject into jsfiddle reported by jdecked https://jsfiddle.net/36tvhmce/
1. Try to repro issues with Pinyin and Hiragana IME
1. Verify there is no error in the console and text is entered normally
Pull Request resolved: facebookarchive#2189

Reviewed By: kedromelon

Differential Revision: D17504999

Pulled By: claudiopro

fbshipit-source-id: dbd2180cf5c1af5bbe1c2b94c50767c58f524dcf
mmissey pushed a commit to mmissey/draft-js that referenced this issue Mar 24, 2020
@mmissey
fix(selection): Returns previous selection if either leaf is null (fa…
6565109 
…cebookarchive#2189)

Summary:
**Summary**

Bails out early from the `getUpdatedSelectionState` method with previous selection when `anchorLeaf` or `focusLeaf` are null to prevent an error surfaced when using a decorator for emoji inserted between two spans of text entered via IME.

Thanks to robbertbrak for suggesting and cherry-picking a fix from their fork 🎉

Fixes facebookarchive#2187

**Test Plan**
1. Build Draft.js
1. Inject into jsfiddle reported by jdecked https://jsfiddle.net/36tvhmce/
1. Try to repro issues with Pinyin and Hiragana IME
1. Verify there is no error in the console and text is entered normally
Pull Request resolved: facebookarchive#2189

Reviewed By: kedromelon

Differential Revision: D17504999

Pulled By: claudiopro

fbshipit-source-id: dbd2180cf5c1af5bbe1c2b94c50767c58f524dcf
alicayan008 pushed a commit to alicayan008/draft-js that referenced this issue Jul 4, 2023
@facebook-github-bot
fix(selection): Returns previous selection if either leaf is null (#2…
e990333 
…189)

Summary:
**Summary**

Bails out early from the `getUpdatedSelectionState` method with previous selection when `anchorLeaf` or `focusLeaf` are null to prevent an error surfaced when using a decorator for emoji inserted between two spans of text entered via IME.

Thanks to robbertbrak for suggesting and cherry-picking a fix from their fork 🎉

Fixes facebookarchive/draft-js#2187

**Test Plan**
1. Build Draft.js
1. Inject into jsfiddle reported by jdecked https://jsfiddle.net/36tvhmce/
1. Try to repro issues with Pinyin and Hiragana IME
1. Verify there is no error in the console and text is entered normally
Pull Request resolved: facebookarchive/draft-js#2189

Reviewed By: kedromelon

Differential Revision: D17504999

Pulled By: claudiopro

fbshipit-source-id: dbd2180cf5c1af5bbe1c2b94c50767c58f524dcf
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@claudiopro claudiopro

Labels
bug IME
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(selection): Returns previous selection if either leaf is null claudiopro/draft-js
3 participants
@robbertbrak @claudiopro @jdecked