🐛 Fix shortcuts with modifiers on letters and digits (#12292)

Mousetrap matched keydown shortcuts against event.key, the glyph the
key types, but bindings name the key. Modifiers change the glyph: on
any OS with a US layout Shift+0 types ")" and Shift+5 types "%", and
macOS Option changes it further (Option+Shift+5 types "fi", Option+A
types "å"). So shortcuts like Alt+Shift+5 for strikethrough or
Shift+0/1/2 for zoom never fired, on every platform. Read letters and
digits from keyCode on keydown, and record them the same way when a
modifier is held.

Closes #12217

AI-assisted-by: claude-opus-5-5
This commit is contained in:
Belén Albeza 2026-10-13 15:59:04 +02:00 committed by GitHub
parent eafa483a18
commit 21ecfb67b7
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 93 additions and 2 deletions

View File

@ -178,6 +178,16 @@ function _addEvent(object, type, callback) {
object.attachEvent("on" + type, callback);
}
/**
* true for the letter (A-Z) and main-row digit (0-9) keycodes
*
* @param {number} which
* @return {boolean}
*/
function _isLetterOrDigit(which) {
return (which >= 48 && which <= 57) || (which >= 65 && which <= 90);
}
/**
* takes the event and returns the key character
*
@ -222,6 +232,12 @@ function _characterFromEvent(e) {
return _KEYCODE_MAP[e.which];
}
// keydown bindings name the key, not the glyph it types under modifiers
// ("fi" for Option+Shift+5, ")" for Shift+0)
if (_isLetterOrDigit(e.which)) {
return String.fromCharCode(e.which).toLowerCase();
}
// if it is not in the special map
if (typeof e.key === "string") {
return e.key.toLowerCase();

View File

@ -53,8 +53,9 @@
;; aren't in that map. Recording punctuation from `code` therefore matches
;; what Mousetrap triggers regardless of layout (e.g. the key next to Right
;; Shift on a US layout is always "Period", whatever glyph it types on a
;; given ISO layout). Letters/digits must stay glyph-based to match
;; Mousetrap's own resolution for them.
;; given ISO layout). With modifiers, letters and digits come from keyCode
;; like Mousetrap's keydown match (see `letter-or-digit`); otherwise from the
;; glyph, like its keypress match.
(def ^:private code-name-map
{"Minus" "-"
"Equal" "="
@ -76,12 +77,24 @@
(or (.getBrowserEvent event) event)
event))
(defn- letter-or-digit
"Letter or digit named by keyCode when a modifier is held, ignoring the glyph
it makes the key type (Option+Shift+5 types \"fi\" on macOS)."
[^js event]
(let [key-code (.-keyCode event)]
(when (and (or (.-ctrlKey event) (.-metaKey event)
(.-altKey event) (.-shiftKey event))
(or (<= 48 key-code 57)
(<= 65 key-code 90)))
(.toLowerCase (js/String.fromCharCode key-code)))))
(defn- normalize-key
[^js event]
(let [key (.-key event)]
(when (and key (not (contains? modifier-keys key)))
(or (get key-name-map key)
(get code-name-map (.-code (event->native event)))
(letter-or-digit event)
(some-> key .toLowerCase)))))
(defn- event->modifier-parts

View File

@ -121,6 +121,7 @@
[frontend-tests.ui.settings-shortcuts-test]
[frontend-tests.ui.shape-filters-test]
[frontend-tests.ui.shortcuts-labels-test]
[frontend-tests.ui.shortcuts-recording-test]
[frontend-tests.ui.sidebar-scroll-test]
[frontend-tests.ui.stroke-menu-test]
[frontend-tests.ui.workspace-libraries-test]
@ -265,6 +266,7 @@
'frontend-tests.ui.settings-shortcuts-test
'frontend-tests.ui.shape-filters-test
'frontend-tests.ui.shortcuts-labels-test
'frontend-tests.ui.shortcuts-recording-test
'frontend-tests.ui.sidebar-scroll-test
'frontend-tests.ui.stroke-menu-test
'frontend-tests.ui.workspace-libraries-test

View File

@ -0,0 +1,60 @@
;; This Source Code Form is subject to the terms of the Mozilla Public
;; License, v. 2.0. If a copy of the MPL was not distributed with this
;; file, You can obtain one at http://mozilla.org/MPL/2.0/.
;;
;; Copyright (c) KALEIDOS SUBSIDIARY SL
(ns frontend-tests.ui.shortcuts-recording-test
(:require
[app.main.ui.shortcuts :as ss]
[cljs.test :as t :include-macros true]))
(def ^:private keyboard-event->mousetrap
@#'ss/keyboard-event->mousetrap)
(defn- key-event
[props]
(clj->js (merge {:altKey false :shiftKey false :ctrlKey false :metaKey false}
props)))
(t/deftest records-alt-digit-from-key-code
;; macOS Option+Shift+5 types "fi"; Mousetrap matches it as "5"
(t/is (= "alt+shift+5"
(keyboard-event->mousetrap
(key-event {:key "fi" :code "Digit5" :keyCode 53 :altKey true :shiftKey true})))))
(t/deftest records-alt-letter-from-key-code
;; macOS Option+A types "å"; Mousetrap matches it as "a"
(t/is (= "alt+a"
(keyboard-event->mousetrap
(key-event {:key "å" :code "KeyA" :keyCode 65 :altKey true})))))
(t/deftest records-shift-digit-from-key-code
;; US Shift+0 types ")"; Mousetrap matches it as "0"
(t/is (= "shift+0"
(keyboard-event->mousetrap
(key-event {:key ")" :code "Digit0" :keyCode 48 :shiftKey true})))))
(t/deftest records-modified-letter-from-key-code
;; Russian layout V types "м"; Mousetrap matches Alt+V as "v"
(t/is (= "alt+v"
(keyboard-event->mousetrap
(key-event {:key "м" :code "KeyV" :keyCode 86 :altKey true})))))
(t/deftest records-unmodified-key-from-glyph
;; without modifiers Mousetrap matches the typed glyph on keypress
(t/is (= "&"
(keyboard-event->mousetrap
(key-event {:key "&" :code "Digit1" :keyCode 49}))))
(t/is (= "ñ"
(keyboard-event->mousetrap
(key-event {:key "ñ" :code "" :keyCode 0})))))
(t/deftest records-shift-punctuation-from-code
;; Shift+. types ":" and Shift+= types "+" (US); the key is recorded
(t/is (= "shift+."
(keyboard-event->mousetrap
(key-event {:key ":" :code "Period" :keyCode 190 :shiftKey true}))))
(t/is (= "shift+="
(keyboard-event->mousetrap
(key-event {:key "+" :code "Equal" :keyCode 187 :shiftKey true})))))