pontoon icon indicating copy to clipboard operation
pontoon copied to clipboard

Comment editor crashes when writing diacritics

Open bugzilla-to-github opened this issue 4 years ago • 12 comments

This issue was created automatically by a script.

Bug 1713496

Bug Reporter: @MikkCZ CC: @Pike

STR:

  1. Open any string, e.g. https://pontoon.mozilla.org/cs/firefox/all-resources/?string=74127.
  2. Open the comment textarea either on a suggestion or the string in general.
  3. On Czech keyboard do two times: -- press acute/caron key (with or without Shift), followed by e.g. E, to write either "é" or "ě" -- press space bar

What should happen:

  • The area should contain "é é " or "ě ě " depending on what you pressed.

What happens:

  • The first time space is pressed, the letter disappears. The second time the whole frontend crashes and two errors appear in the console.
Error: Cannot resolve a DOM point from Slate point: {"path":[0,0],"offset":3}
    toDOMPoint react-editor.ts:262
    toDOMRange react-editor.ts:277
    wt editable.tsx:184
    React 3
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    onChange with-react.ts:177
    apply create-editor.ts:86
    promise callback*apply create-editor.ts:84
    apply with-react.ts:52
    insertText text.ts:448
    withoutNormalizing editor.ts:1587
    insertText text.ts:414
    insertText create-editor.ts:188
    insertText editor.ts:319
    onBeforeInput editable.tsx:498
    React 12
    unstable_runWithPriority scheduler.production.min.js:19
    React 13
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    Redux 5
    a middleware.js:22
    dispatch Redux
    r actions.js:67
    h runtime.js:63
    _invoke runtime.js:293
    S runtime.js:118
    Babel 2
react-dom.production.min.js:209:194
Uncaught (in promise) Error: Cannot resolve a DOM point from Slate point: {"path":[0,0],"offset":3}
    toDOMPoint react-editor.ts:262
    toDOMRange react-editor.ts:277
    wt editable.tsx:184
    React 3
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    onChange with-react.ts:177
    apply create-editor.ts:86
    promise callback*apply create-editor.ts:84
    apply with-react.ts:52
    insertText text.ts:448
    withoutNormalizing editor.ts:1587
    insertText text.ts:414
    insertText create-editor.ts:188
    insertText editor.ts:319
    onBeforeInput editable.tsx:498
    React 12
    unstable_runWithPriority scheduler.production.min.js:19
    React 13
    unstable_runWithPriority scheduler.production.min.js:19
    React 4
    Redux 5
    a middleware.js:22
    dispatch Redux
    r actions.js:67
    h runtime.js:63
    _invoke runtime.js:293
    S runtime.js:118
    Babel 2
react-editor.ts:262:12

bugzilla-to-github avatar May 29 '21 19:05 bugzilla-to-github

Comment Author: @MikkCZ

Created attachment 9224166 screen recording.webm

Attached file: Nahrávka obrazovky z 30.5.2021 11:50:27.webm (video/webm, 1206739 bytes) Description: screen recording.webm

bugzilla-to-github avatar May 29 '21 19:05 bugzilla-to-github

Comment Author: @MikkCZ

Observation, the "offset" in the error message differs based on what you write. This happens in the middle of the text as well.

Also when you use the keys in the row below F-keys, which by default in Czech layout write "ěščřžýáíé", directly, everything works. But note, this is only a partial workaround, because not all letters with diacritics can be written like that.

bugzilla-to-github avatar May 29 '21 19:05 bugzilla-to-github

Comment Author: @Pike

Can you test this on https://www.slatejs.org/examples/mentions? Maybe this is an upstream bug.

bugzilla-to-github avatar May 29 '21 20:05 bugzilla-to-github

Comment Author: @MikkCZ

I wasn't able to reproduce it there, however I see that https://github.com/mozilla/pontoon/pull/1955 updated slate, and in the issue tracker of slate there is a ton of bugs (some specific to Firefox) that it does not handle the cursor position properly under all conditions.

In this specific case and after looking at the error message in more detail, I guess something looses the information about number of characters being actually typed. When you start by pressing the acute/caron key (with or without Shift), ´ or ˇ appears, and is then merged with what's written next. That's normal behaviour. But if the library or frontend code does not account for that, it will definitely cause an index to go out of bounds of the text actually written in the area.

bugzilla-to-github avatar May 29 '21 20:05 bugzilla-to-github

@mathjazz can I be assigned to this one

Mitch4sho avatar Feb 03 '22 05:02 Mitch4sho

I was able to reproduce this upstream, and https://github.com/ianstormtaylor/slate/issues/4136 is exactly this, AFAICT.

I don't know that we have a way to fix this, short of wrapping slate in an error boundary, and reloading it with an empty comment on crash?

Pike avatar Feb 03 '22 09:02 Pike

I tried to reproduce (using the Apple Wireless Keyboard with Slovenian layout) and the sequence of characters in the original comment didn't trigger the error. However, I could reproduce the issue using the following sequence:

  1. Alt + E
  2. Alt + Space
  3. Alt + Space

However, I was unable to reproduce on https://www.slatejs.org/examples/plaintext (which is what https://github.com/ianstormtaylor/slate/issues/4136 references).

mathjazz avatar Feb 03 '22 21:02 mathjazz

The kicker is that you need to start with an empty inbox.

Then add a diacritic, add a char, move, delete, crash.

Pike avatar Feb 03 '22 21:02 Pike

Ah, thanks. Let's continue as agreed on the meeting (i.e. as you suggested), sorry for the false alarm.

mathjazz avatar Feb 03 '22 21:02 mathjazz

The same error appears when you type @ and paste some text.

mathjazz avatar Jun 14 '22 21:06 mathjazz

I am no longer able to reproduce this upstream, so it seems like updating Slate could be the way forward here?

At least as a short-term fix. A long-term fix is #3010.

mathjazz avatar Dec 18 '23 17:12 mathjazz

@Mitch4sho Welcome back. :)

Please start here: https://mozilla-pontoon.readthedocs.io/en/latest/dev/setup.html

mathjazz avatar Dec 18 '23 18:12 mathjazz