prosemirror-keymap uses deprecated keyCode property of KeyboardEvent

[Mr3zee]

Here keydownHandler uses keyCode which is deprecated and may lead to unexpected problems.

Steps to reproduce an issue (MacOs + Chrome):

• add bindings for Cmd+E (1) and Cmd+Shift+E (2) using keydownHandler
• execute code in browser console

  dom.dispatchEvent(new KeyboardEvent("keydown", {"metaKey": true, "shiftKey": true, "key": "e", "code": "KeyE"}))

Expected result: Handler for (2) was fired

Actual result: Handler for (1) was fired

Temporary solution: This code works ok (keyCode field was added)

dom.dispatchEvent(new KeyboardEvent("keydown", {"metaKey": true, "shiftKey": true, "key": "e", "code": "KeyE", "keyCode": 69}))

Note: It is an issue only for events fired in code, everything works fine with physical keyboard

[marijnh]

It is an issue only for events fired in code, everything works fine with physical keyboard

Right. And, if you look at the code properly, you'll see that it makes a lot of effort to use key, but has to fall back to keyCode because browsers do all kinds of broken things. So no, I don't consider the fact that we use keyCode a problem in itself.

Is the issue that the code should check if keyCode exists in case someone decides to fire synthetic events without it?

[Mr3zee]

@marijnh Yes, sorry, I fought that the reason the issue occurs is because of keyCode as if I add it everything works ok, but there is more complex logic behind it.

Is the issue that the code should check if keyCode exists in case someone decides to fire synthetic events without it?

No, I believe that the issue is something else, as if keyCode is not defined it is set to 0.

The behavior I observe is that if there are two shortcuts that differ in Shift key and keyCode is not set (synthetic event), it will match both of them in case of event with Shift key. The problem I experience is that Cmd+E and Cmd+Shift+E actions in my case are both valid (return true) and if I fire synthetic event Cmd+Shift+E - Cmd+E will trigger, return true and thus ignore Cmd+Shift+E action. I tried to dig into the logic of keydownHandler, but I am not sure why this happens (And I see that it will trigger Cmd+Shift+E event if Cmd+E returns false, so that why I say that it triggers both of them)

[marijnh]

Sooo, the part of the code that accessed keyCode never even runs in this situation, and thus is definitely not the problem. Sometimes, it's better to just report a problem as you observe it, rather than framing it using half-baked hypotheses, since those can really put people on the wrong foot.

The issue is that, unlike most platforms, and arguably going against the standard, macOS puts a lower-case 'e' in KeyboardEvent.key even though shift is held (as opposed to 'E'), when Cmd is being held. Your synthetic event follows this behavior. Attached patch should make the code more robust for this. You still can't bind "cmd-E" (since JavaScript has no access to keyboard layout and the behavior of shift in that), but "cmd-shift-e" should work now.

[Mr3zee]

Ok, thanks