facebook / lexical

Lexical is an extensible text editor framework that provides excellent reliability, accessibility and performance.
https://lexical.dev
MIT License
19.55k stars 1.66k forks source link

Bug: Undo hotkey stops working in a specific case #6674

Open Ulop opened 4 weeks ago

Ulop commented 4 weeks ago

Lexical version: lexical: 0.17.1

Steps To Reproduce

  1. Switch to a non-Latin keyboard layout (e.g. Greek layout)
  2. Open Lexical editor with a non-empty content (playground is a good variant)
  3. Select all content
  4. Press the Backspace (or Delete) key to clear all content
  5. Press the Ctrl+Z(undo) hotkey

Link to code example: https://playground.lexical.dev/

The current behavior

Nothing happens

The expected behavior

The editor restores the last state from the undoStack field of the history when you press the Ctrl + Z hotkey.

By default, in the LexicalEvents.ts file there are two points that dispatch the UNDO_COMMAND command:

When the content has been cleared as described above (in the Steps To Reproduce section) only the first listener (onKeyDown) is called and never the second one.

For Latin layout, it works correctly because it passes the isUndo function test. But it doesn't work for other layouts, since the event.key value is not the 'z' character.

However, if we modify the content (e.g. type in any character) before step 3 and then execute steps 3-5 everything will work as expected. The onBeforeInput event is called and the previous state is restored.

fantactuka commented 3 weeks ago

Thanks for reporting @Ulop

I've tried to repro with Greek and Korean layouts and in both cases it worked well, is it the same for you in all browsers, or specific one?

Regarding event handlers: as long as isUndo check passes in onKeyDown it preventsDefault so onBeforeInput won't be called (so it'll avoid double triggering for undo command)

Ulop commented 3 weeks ago

@fantactuka, Thanks for answer.

Maybe it's a browser-dependent issue?

I asked a colleague to check this situation on his MacBook (MacOS 17.6) in Safari - everything worked as expected.

On MacBook in Chrome the editor state is not restored after pressing the undo hotkey.

On Win(10|11) in Chrome\FireFox the situation is similar.

Ulop commented 3 weeks ago

I can assume that I may have found the cause, but I can’t suggest a simple solution.

As I'm mentioned above, Lexical has two point that can dispatch UNDO_COMMAND when the user presses the Ctrl + Z command.

onKeyDown listener will dispatch the undo command only if the user's current keyboard layout contains the z key.

So in other cases we must rely on the onBeforeInput listener. But with Chrome(and apparentyl with FireFox too) there is some problem. Input\Content editable element will receive onBeforeInput event with inputType == 'historyUndo' only if that element was previously modified by user input, not by JS.

A simallar issue was closed a comment that is expected behaviour.

More simple example:

  1. Switch to a non-Latin keyboard layout
  2. Open Lexical editor with a non-empty content
  3. Insert Image
  4. Press the undo hotkey

onBeforeInput doesn't called, nothing happened.

Ulop commented 3 weeks ago

Having thought about this problem once again, I came to the conclusion that it is most likely worth solution is changing hotkeys handling in the onKeyDown listener.

I decided to see how VSCode solves the hotkey problem. And as far as I understand, they follow the points from these recomendations and rely on KeyboardEvent::keyCode field. See extractKeyCode function from keyboardEvent.ts and how EVENT_KEY_CODE_MAP filled.

Similar issues with hotkeys handling: https://github.com/w3c/uievents/issues/377 https://github.com/w3c/uievents/issues/267