Bug: Undo hotkey stops working in a specific case

[Ulop]

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:

• onKeyDown listener (by checking whether event.ctrlKey is true and event.key equals to z character)
• onBeforeInput listener (by checking whether event.inputType equals to historyUndo)

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]

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]

@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]

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]

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

[EugeneVorobyev]

Hi,

We've got this problem reported by many users. Is there any known workaround or fix?

Anothe repro example:

• Switch to non-latin keyboard layout
• Load editor with content
• Change any paragraph to list
• Press the undo hot key

Repro 100% in Chrome on Windows in the playground (v0.25.0)

It's really annoying bug, cause basically any command except editing text can't be reverted if the editor is loaded in non-latin keyboard layout.

[etrepum]

The fix would be to update isUndo in LexicalEvents.ts with whatever the correct code is to detect that specific shortcut with all supported keyboards and platforms.

For a workaround, you would add a KEY_DOWN_COMMAND listener that handles the event how you want to, something like:

const unreg = editor.registerCommand(KEY_DOWN_COMMAND, (event) => {
  if (isUndo(event)) {
    event.preventDefault();
    dispatchCommand(editor, UNDO_COMMAND, undefined);
    return true;
  }
  return false;
});

[EugeneVorobyev]

Yeah, the issue is that isUndo never really pass the check, cause it uses key property of event, which is local dependend. Instead the function should use the code: "KeyZ" - which is the same for any keyboard layout.

Thanks for the workaround example, i will try!

[etrepum]

If you do have a systematic way of solving this, a PR would be appreciated. Similar issues may affect other built-in keyboard shortcuts.

[Ulop]

Instead the function should use the code: "KeyZ" - which is the same for any keyboard layout. Thanks for the workaround example, i will try!

Hi, @EugeneVorobyev ! Previously, isUndo had exactly the type of check you would want to introduce. But in this PR the condition was changed.

Also here is the following warning

For example, the code returned is "KeyQ" for the Q key on a QWERTY layout keyboard, but the same code value also represents the ' key on Dvorak keyboards and the A key on AZERTY keyboards. That makes it impossible to use the value of code to determine what the name of the key is to users if they're not using an anticipated keyboard layout.

[EugeneVorobyev]

It feels a bit rushed fix, here are few things:

• There are much more potential users with multiple non-latin keyboard layouts comparing to Drovak users
• Dvorak users can have also non-latin locales and the mentioned change doesn't take it into account
• Did a little research and it seems it's usual for Dvorak users to use some sort of remapping scripts to use the same physical shortcuts like they would you in Qwerty rather than use by symbols (for things like Ctrl + C, Crtl + V, Ctrl + Z)

So, looks like the previous change fixed a very specific case, when the major common use case was broken.

I would suggest to rollback that change or perhaps introduce some sort of an option flag to switch from code to key for handling. In that case developers could choose which mode to use for the editor.

@Ulop @etrepum what do you think?

[etrepum]

I don't personally have very strong opinions about this, but @Sahejkm might as the author of that PR. I agree that internationalization/localization is a lot more common than switching to an alternate keyboard layout. I would lean towards doing a survey of how other web based text editors handle key bindings (e.g. vscode, tiptap/prosemirror, slate, quill, google docs, etc.) to see what works well elsewhere and using that to guide the approach. This isn't exactly a "cutting edge" topic.

In an ideal world, the key bindings would not be at such a low-level in lexical and you would be able to specify how they work in the editor configuration (with some sensible default that matches typical expectations, you shouldn't have to explicitly choose).

[Ulop]

@Ulop @etrepum what do you think?

I don't have any specific suggestion for solving the problem. Maybe try to follow the example of vscode, which I mentioned to earlier.

Specifically, to solve my problems, a rollback of the mentioned PR would help. But this will solve the problem specifically for me, but not in general.

In my opinion, this is a complex problem with no simple solution.

[EugeneVorobyev]

Just FYI - workaround works fine, solving my major problem. Thanks.

Agree it's quite complex problem to solve for every case. But there is also a tradeoff between which group of users have it working out of the box: with non-latin locale or with Dvorak layout. I wear few hats at my position and as a Product Owner i would choose the original way of things, where it worked for localization as it's feels a more common case than exotic layouts. Just IMHO.

[etrepum]

It's probably also worth asking here, can you be precise about exactly which platforms, browsers and keyboard layouts you are experiencing this problem on? I primarily use macOS and can't reproduce an issue w/ Chrome, Safari or Firefox with undo or select all using the Greek keyboard (also tested Pinyin - Simplified and 2-Set Korean which both work).

[etrepum]

The contributors spent some time discussing this issue and we are open to accepting a PR that changes the hotkey logic to work more like it did before #6110 - even better if it lifts this logic out a bit so that it's easier to customize

[EugeneVorobyev]

@etrepum Sound good, I should have some time next week and can take this ticket. Please feel free to assign to me if it's ok.

[etrepum]

Sounds great, thanks!