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

The editor can't be focused or the selection can't be correctly updated after input with IME #2149

Closed
freeplant opened this issue Sep 4, 2018 · 9 comments

Comments

@freeplant
Copy link

freeplant commented Sep 4, 2018

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

report a bug

What's the current behavior?

During our daily use of Slate, we occasionally encounter a problem that the editor can't be focus or the selection can't be updated after inputting characters with IME. The problem can be easily reproduced with the Input Tester on the demo site (maybe it is due to the "Input Tester" update the rendered react components for every event to show the event list). Here is the screenshot:

problem-with-selections

Whenever I want to put the focus after the last character "f", it changes back to before "f".

Currently I think the problem is caused by the following code:

content.js:

onEvent() {
     ...
    if (handler == 'onSelect') {
      const { editor } = this.props
      const { value } = editor
      const { selection } = value
      const window = getWindow(event.target)
      const native = window.getSelection()
      const range = findRange(native, value)

      if (range && range.equals(selection.toRange())) {
        this.updateSelection()
        return
      }
    }
  ...
}

Currently I'm trying to locate and resolve the problem. Any hints are welcome.

@freeplant
Copy link
Author

freeplant commented Sep 4, 2018

Here is my analysis of the problem (Chrome 68.0.3440.106 under Mac):

During IME composition, after I input three characters "fff":

image

the onSelection event is fired to the editor. In onEvent(), the native selection will take the character "fff" into consideration. So the offset is not 0 (should be a value after the characters "fff"). But the characters are not saved to slate document yet, so the range = findRange(native, value) will return a range with with offset 0. range.equals(selection.toRange() will be true, and this.updateSelection() is called.

image

In the screenshot, the selection means "native selection", the offset is 5.
The Range is of offset 0.

After changing the code to as following, IME can work in the "Input Tester" example:

    if (handler == 'onSelect') {
      const { editor } = this.props
      if (!editor.state.isComposing) {
        const { value } = editor
        const { selection } = value
        const window = getWindow(event.target)
        const native = window.getSelection()
        const range = findRange(native, value)

        if (range && range.equals(selection.toRange())) {
          this.updateSelection()
          return
        }
      }
    }

In before.js, during IME composing, DOM selection event will not be handled:

  function onSelect(event, change, editor) {
    if (isCopying) return true
    if (isComposing) return true

So I think during IME composing, the selection event should not be handled in content.js too.

@isubasti
Copy link
Contributor

isubasti commented Sep 4, 2018

@freeplant some ongoing discussion and works regarding IME and mobile on #2062

@freeplant
Copy link
Author

@isubasti Thanks for the notice.

@thesunny
Copy link
Collaborator

thesunny commented Sep 4, 2018

Feels like a similar problem to Android. This issue is a good candidate for being solved by #2062 by waiting for the DOM to be in a stable state before a re-render is done. I solved a similar issue by finding the right event (not always compositionEnd) or using a setTimeout to wait for the DOM to become stable.

@freeplant
Copy link
Author

I find the cause of the last character "f" can't be focused.

During IME composing, if you left the editor (focus on other elements out side of the editor). The inputed characters will be left in the DOM, but not in the slate document. So the characters are not visible to Slate.

@thesunny #2062 should solve the problems. But I wonder if there is a short-term solution. There are two issues I see so far with IME on desktop:

  1. During IME composing, if the user left the editor, the characters will be in the DOM but not in the slate document.
  2. During IME composing, the updateSelection() should not be called.

The second issue can be easily fixed.

@freeplant
Copy link
Author

Here is the events sequence during IME when user input a letter then input a number to select the candidate:

Native ContentEditable React ContentEditable Slate
keyDown keyDown keyDown
compositionStart compositionStart compositionStart
beforeInput
compositionUpdate compositionUpdate
input input input
keyUp keyUp keyUp
keyDown keyDown keyDown
beforeInput
compositionUpdate compositionUpdate
input beforeInput, input beforeInput (Slate actually insert the text)
compositionEnd compositionEnd compositionEnd
keyUp keyUp keyUp

Here is the events sequence during IME when user input a letter then focus out:

Native ContentEditable React ContentEditable Slate
keyDown keyDown keyDown
compositionStart compositionStart compositionStart
beforeInput
compositionUpdate compositionUpdate
input input input
keyUp keyUp keyUp
compositionEnd compositionEnd compositionEnd

The text is ignored by Slate but left in the DOM.

Waiting for the DOM to be in a stable state than using Diff to get the actual text can solve the problem.

Another solution is to remember the temporary text during input event. If the text is inserted in Slate in beforeInput, clear the temporary text. Otherwise insert the temporary text in compositionEnd.

@thesunny
Copy link
Collaborator

thesunny commented Sep 5, 2018

@freeplant

Can't remember what Slate was doing in the current version but there is a general issue with browsers being that the state during a composition is not consistent so even if the event fires, the DOM might not be what we expect at that moment.

Try this modified input tester https://thesunny.github.io/input-methods/index.html which outputs the DOM and cursor position during the event cycle. May help you figure out what's happening. Without it, I couldn't figure out the Android issues.

@ianstormtaylor
Copy link
Owner

Closing in favor of #2368, if you think this is unrelated let me know and we can reconsider.

@ArnaudRinquin
Copy link

@ianstormtaylor I'm having very similar issues with a plugin I'm writing, but it doesn't involve IME.

no-select

The text I'm trying to select is in div with contentEditable={false}. I tried catching events and preventing their propagation but nothing seems to work. I've tried hijacking onClick, onMouseUp, onSelect, everything I could think of without success.

Do you have any idea? Do you think it's related?

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

No branches or pull requests

5 participants