-
Notifications
You must be signed in to change notification settings - Fork 10.4k
Description
Attach (recommended) or Link to PDF file
N/A
Web browser and its version
Firefox 141.0
Operating system and its version
Windows 11
PDF.js version
v5.3.93
Is the bug present in the latest PDF.js version?
Yes
Is a browser extension
No
Steps to reproduce the problem
Scenario 1:
- Have an English keyboard layout
- Open any pdf file
- Use the Draw or Highlight tools to draw something
- Enable CapsLock on your keyboard
- Press Ctrl+z
Scenario 2:
- Have a Greek keyboard layout
- Open any pdf file
- Use the Draw or Highlight tools to draw something
- Press Ctrl+z
What is the expected behavior?
In both scenarios an undo should be performed on the drawing
What went wrong?
Nothing happens, the shortcut does not register
Link to a viewer
No response
Additional context
The problem in both cases lies in the use of KeyboardEvent.key
to decide which keys were pressed in the KeyboardManager
class. Since KeyboardEvent.key
returns an actual character taking modifiers and locale into account, in many cases the value that is returned is not what the KeyboardManager
expects.
For example in scenario 1, the fact CapsLock is enabled means that when Ctrl+z is pressed for an undo, the corresponding event.key
is an uppercase "Z" rather than "z". Since the KeyboardManager
is set up to only handle "ctrl+z" and there is no normalization on the input, this is interpreted as a different combination of keys, "ctrl+Z". The KeyboardManager
isn't configured to handle the combination, and it is thus ignored.
In scenario 2, something worse happens. Here event.key
is actually the Greek letter "ζ" which the KeyboardManager obviously has no hope of handling. Of course the same happens in other non-Latin based keyboards like Cyrillic, where pressing the key that is where Z would be gives "я".
One possible fix would be to use the more stable KeyboardEvent.code
property, which returns the actual physical key that was pressed, so it isn't affected by modifiers or the keyboard layout. This should handle both scenario 1 and scenario 2 with no issues since event.code === "KeyZ"
in all cases. However this could cause issues for users who have a non-QWERTY Latin-script layout, like QWERTZ or AZERTY. Pressing the key labelled Z on a QWERTZ keyboard gives {event.key: "z"}
but {event.code: "KeyY"}
. I'm not sure how people who use these layouts expect shortcuts to work (based on absolute key positions or key labels) but using KeyEvent.code
could potentially lead to confusion here. I guess another solution would be to keep using KeyboardEvent.key
but with a normalized letter case, and only fall back to using KeyboardEvent.code
when event.key
is a non-ASCII character