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

[bug] Delete character when using IME if there is bold/italic etc. throw error and lose cursor. #1698

Closed
stephenLYZ opened this issue Sep 15, 2017 · 2 comments

Comments

@stephenLYZ
Copy link

Hi, there is a bug that when using IME, delete character if there is bold/italic etc. would throw error and lose cursor. The console log is 'The given range isn't in document.' in 'selection.js' file.
demo

Steps for Reproduction

  1. Visit [quilljs.com]
  2. Type something, then click Bold button.
  3. Type using IME, and delete character

Expected behavior:
No error and not lose cursor
Actual behavior:
Throw error and lose cursor

Platforms:
macOS Sierra 10.21
Chrome Canary 61.0.3162.0

Version:
Version 1.3.0

@smura
Copy link

smura commented Aug 9, 2018

This issue occurs here.

https://github.com/quilljs/quill/blob/1.3.6/core/selection.js#L67-L73

Detail of this issue

  1. The compositionend handler (L65) is started.
  2. if (this.cursor.parent) (L67) is true, so setTimeout() (L70) is called.
  3. The compositionend handler is exited.
    • And another thread starts before setNativeRange() (L71) will be called.
  4. span.ql-cursor element is removed from document. (I don't know why)
    • span.ql-cursor is same as this.cursor (L67) and range.startNode (L71).
    • Now this.cursor has no parent. (see L67)
  5. setNativeRange() (L71) is called.
  6. range.setStart(startNode, startOffset); (L291) is called.
  7. But startNode is already removed from document. So the following error is occurred : The given range isn't in document.

Solution

In setTimeout() thread, check again whether this.cursor has a parent or not.

      if (this.cursor.parent) {
        const range = this.cursor.restore();
        if (!range) return;
        setTimeout(() => {
          if (this.cursor.parent) {  // Check again, because it's new thread.
            this.setNativeRange(range.startNode, range.startOffset, range.endNode, range.endOffset);
          }
        }, 1);
      }

@quill-bot
Copy link

Quill 2.0 has been released (announcement post) with many changes and fixes. If this is still an issue please create a new issue after reviewing our updated Contributing guide 🙏

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

Successfully merging a pull request may close this issue.

4 participants