Draft-js: Focussing a field with tab then typing Japanese causes the text to appear twice using Firefox

[birtles]

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

What is the current behavior?
With Firefox on Windows 10 and Microsoft Japanese IME, if you focus a draft-js field using Tab and then input Japanese text, it will appear twice in the box (and twice in the DOM).

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/stopachka/m6z0xn4r/.

1. Open https://draftjs.org/
2. Once the page has finished loading, click on the last instance of "Draft.js" in the last paragraph.
3. Press Tab
4. If the IME is not activated (i.e. not yet in hiragana mode), activate it (e.g. press 半角/全角/漢字)
5. Type "tesuto"
6. Press space (to select the first candidate)
7. Press Enter to confirm the candidate.

What is the expected behavior?
テスト appears once.

Which versions of Draft.js, and which browser / OS are affected by this issue? Did this work in previous versions of Draft.js?
Affects at least draft.js v0.10.5, Firefox Nightly, Windows 10, Microsoft Japanese IME

[birtles]

Screencast:

[birtles]

If I inspect the DOM at this point the テスト string appears twice.

[birtles]

After this all hell breaks loose. For example, if I delete the typed text, one copy remains, now overlapping the placeholder.

[flarnie]

Thanks for reporting! I would be happy to review a PR with a fix, not sure when I'll have time to look into this. I would try doing a git bisect to see when this was introduced. If it's present no matter how far back we go, then it's likely due to a change in the Firefox browser itself and I would look at what happens inside the DraftEditorCompositionHandler - https://github.com/facebook/draft-js/blob/master/src/component/handlers/composition/DraftEditorCompositionHandler.js.

[birtles]

I started digging into this and comparing what happens in Firefox and Chrome I see that after we focus on the field and begin typing in Firefox I get the following sequence:

1. onBeforeInput in DraftEditorCompositionHandler
2. resolveComposition
3. restoreEditorDOM (Here contentsKey is 0)

So far Firefox and Chrome behave, as far as I have observed, identically.

However, at this point the call to setState which, in my local copy is something like:

_this2.restoreEditorDOM = function (scrollPosition) {
  _this2.setState({ contentsKey: _this2.state.contentsKey + 1 }, function () {
    _this2.focus(scrollPosition);
  });
}

Triggers a synchronous re-render.

That is we will end up re-rendering DraftEditorContents and children, including running setDraftEditorSelection and setting hasFocus to true, and then run the call to _this2.focus(scrollPosition); all before returning from restoreEditorDOM and running the rest of resolveComposition.

When I run in Chrome, the call to setState does not run synchronously (as observed by the callback function that calls focus() not running until after resolveComposition is complete).

That said, I don't think the synchronous render is actually the problem per se. If I wrap it in a setTimeout, for example, or even remove it altogether the problem still reproduces. I suspect the actual problem lies with pre-existing DraftEditorLeaf elements being re-rendered but I've yet to work out exactly why.

[birtles]

Digging in a bit deeper I see that when we call into setState from Chrome isBatching is true but not in Firefox. Comparing the stacks of both I see that we are in onKeyDown in Chrome but onCompositionEnd in Firefox.

I suspect the reason for the discrepancy is that Firefox doesn't dispatch keyup/keydown events during composition (bug 354358). We're hoping to fix that in the coming months and while I'm not entirely sure that's the cause of this bug, it's probably worth waiting for that to land before digging into this any further.

/cc @masayuki-nakano

[masayuki-nakano]

@birtles You can use these tryserver builds to check if bug 354358 fixes this bug (kick opt build before download):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c91b13eb96af5892b5a35fd03cc7ca186a3d9a8

Although, Linux still has a serious bug.

[birtles]

@birtles You can use these tryserver builds to check if bug 354358 fixes this bug (kick opt build before download):
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c91b13eb96af5892b5a35fd03cc7ca186a3d9a8

Thanks! I tried the Windows debug build from there but unfortunately it doesn't seem to fix this bug 😓

I'm not sure where to look next. The fact that there is a difference between clicking the field and tabbing to it is certainly odd so perhaps I need to compare those states more closely.

[masayuki-nakano]

In Draft.js, we reach resolveComposition() when we commit composition:

>     var mustReset = !composedChars || isSelectionAtLeafStart(editorState) || currentStyle.size > 0 || entityKey !== null;

mustReset becomes true.

> if (composedChars) {
>       if (gkx('draft_handlebeforeinput_composed_text') && editor.props.handleBeforeInput && isEventHandled(editor.props.handleBeforeInput(composedChars, editorState))) {
>         return;
>       }
>       // If characters have been composed, re-rendering with the update
>       // is sufficient to reset the editor.
>       var contentState = DraftModifier.replaceText(editorState.getCurrentContent(), editorState.getSelection(), composedChars, currentStyle, entityKey);
>       editor.update(EditorState.push(editorState, contentState, 'insert-characters'));
>       return;
>     }

And composedChars has commit string. So, we reach this block. Then, the commit string becomes duplicated.

[masayuki-nakano]

Gecko initializes selection from "focus" event listener added to the document node at capturing phase. So, it must be fine web apps to initialize selection from "focus" event listener of the editing host.

[birtles]

This is fixed by Firefox bug 662591 which is in the latest Firefox Nightly and, provided there are no Web compatibility problems, will be part of Firefox 60 when it ships in early-mid May.

Thank you @masayuki-nakano!