ianstormtaylor / slate

A completely customizable framework for building rich text editors. (Currently in beta.)
http://slatejs.org
MIT License
29.71k stars 3.24k forks source link

Press space in the wrapped second line eats the first line #938

Closed terryoy closed 7 years ago

terryoy commented 7 years ago

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

bug

What's the current behavior?

image

What's the expected behavior?

Just insert plain space...

hubgit commented 7 years ago

I'm able to reproduce this in Chrome 59 on http://slatejs.org/#/plain-text (and the rich text editor) by entering a long line of text and trying to type a space at the start of a wrapped line. The preceding text (up to the start of the paragraph) is replaced by the typed space.

J22Melody commented 7 years ago

Same here, on Chrome it will eat the first line while on Safari it won't get any response.

Looking for workaround ...

tommyokeefe commented 7 years ago

Any eta on a fix for this? We're using slate for the editor in our cms and I just ran into this bug when an editor lost a chunk of text in a story.

danburzo commented 7 years ago

I can confirm the simplified use-case: just causing the line to wrap and pressing Space at the beginning of the second line will eat the first line of text in Chrome 60/macOS. Safari/FF seem to be okay.

Later edit: I've traced it down to Content.onInput() — returning early in the function prevents the first line from being deleted.

danburzo commented 7 years ago

Okay, found it! The problem is generated by the CSS rule -webkit-user-modify: read-write-plaintext-only;. It causes Chrome to wrongly report window.getSelection().anchorNode as being only the second line of text, and the offset = 0

(Chrome splits the text in two adjacent TextNodes)

Edit: Logged a bug with Chromium to get their input on the problem.

danburzo commented 7 years ago

To mitigate the problem we can prevent the default behavior in Chrome and replace it with a synthetic insertion of the Space character:

  // this is in plugins/core.js
  function onKeyDownSpace(e, data, state) {
    if (IS_CHROME) {
      e.preventDefault();
      return state.transform().insertText(' ').apply();
    }
  }

Seems to get rid of the problem. You can put this into a plugin, or — if it's the kind of thing we want to have as part of the core — directly in the core plugin.

tommyokeefe commented 7 years ago

@danburzo I played with that approach to mitigating the problem locally but ended up deciding against it. The extra space characters all get pushed to the end of the previous line (before the break) so a user could theoretically add a bunch of spaces unintentionally and not see them until text is published. Just something to consider.

danburzo commented 7 years ago

@tommyokeefe you're right, but logically that's where it's supposed to be, and by patching Chrome all browsers behave in a fairly consistent manner. You'll see the same behavior in Google Docs and various desktop apps, as well.

tommyokeefe commented 7 years ago

Ahh. True. Good point.

danburzo commented 7 years ago

@ianstormtaylor Seems I have messed up the emoji input (and possibly IME) on Chrome 🥇 I've updated with a better PR.