Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BeforeInput is fired with a wrong text at a wrong time on IE #7107

Merged
merged 1 commit into from
Sep 13, 2016
Merged

BeforeInput is fired with a wrong text at a wrong time on IE #7107

merged 1 commit into from
Sep 13, 2016

Conversation

msmania
Copy link
Contributor

@msmania msmania commented Jun 23, 2016

This patch addresses a typing issue with Japanese IME on Internet Explorer.
The issue is related to the known issue about Korean IME documented in draft.js,
but different. Korean issue is not addressed by this patch.
This patch also adds a new testcase to cover BeforeInputEventPlugin.

Repro steps:

  1. Open IE11 and navigate to https://facebook.github.io/draft-js/
  2. Turn on Japanese IME and type Japanese text on the 'Tell a story' box

Issue description:

The 1st candaidate is always committed even when we try to commit the 2nd
or later candidate. Since facebook.com is using draft.js, this issue can
be reproed on the NewsFeed Post box of facebook.com.

Root cause/Fix:

React creates a browser-independent beforeInput event to match W3C spec
from the combination of the existing events (e.g. compositionstart, keyup,
textnput, and etc.). The problem is that in IE, onBeforeInput is fired with
a wrong text at a wrong time.

Basically React uses textInput events to create beforeInput. However, IE
fires an event named textinput (all lowercases), that is not useful due to
a couple of IE's incorrect behaviors as below.

  1. With Japanese IME, after one or more implicit commits, when a user ends
    composition, a textinput is fired, but it contains only the last composition
    text.
  2. With Japanese IME, a textinput event does not contain an ideographic
    character (U+3000) when a user hits a space key not in composition state.

As a fallback, React stores a text of a target element at compositionend or
additional keyboard/mouse events, and fires beforeinput at compositionend
with the stored text. One of the additional events is keyUp with keyCode=32.

With Korean and Chinese IME, a space key is one of keys to end composition.
With Japanese IME, however, a space key does not mean the end of composition
but just switching candidates. As a result, the first candidate is stored as
a fallback text, and it is displayed even if a user selects the 2nd or
a subsequent candidate.

The fix is that we don't use additional keyboard/mouse events if composition
event is available. IE9-11 fires composition events even though its event data
is not correct. It's good enough to use as a time to store a fallback text.

@gaearon
Copy link
Collaborator

gaearon commented Jun 26, 2016

@hellendag Is this something you could look into?

@sophiebits
Copy link
Contributor

@msmania Do you mind rebasing this? Otherwise I think this is okay to go in.

@ghost ghost added the CLA Signed label Sep 12, 2016
@ghost ghost added the CLA Signed label Sep 13, 2016
@msmania
Copy link
Contributor Author

msmania commented Sep 13, 2016

Thank you for looking at this, @spicyj . Rebase is done. I also removed topLevelTypes from the test code.

@ghost ghost added the CLA Signed label Sep 13, 2016
@sophiebits sophiebits merged commit a64ca9b into facebook:master Sep 13, 2016
@sophiebits sophiebits added this to the 15-next milestone Sep 13, 2016
@sophiebits
Copy link
Contributor

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants