fix(desktop): preserve Shift for Cmd/Ctrl+digit keybindings (#4863) by Jocs · Pull Request #4873 · marktext/marktext

Jocs · 2026-07-05T15:53:44Z

Summary

Fixes #4863. Two symptoms, one root cause:

Recording ⌘⇧7 in Preferences → Key Bindings showed and saved Cmd+7 — Shift silently dropped (the reported bug).
At runtime, even a correctly-spelled Cmd+Shift+7 binding never fired — the rebind was dead.

Root cause

Both the recorder (key-input-dialog.vue → getAcceleratorFromKeyboardEvent) and the shortcut matcher (electronLocalshortcut's before-input-event handler in the main process) run the same normalizeKeyboardEvent from @hfelix/electron-localshortcut's atom-keymap port. It keeps Shift as a modifier only for non-character keys and upper-case Latin letters:

if (key === 'shift' || (shiftKey && (isNonCharacterKey || (isLatinCharacter(key) && isUpperCaseCharacter(key))))) {
    keyInputEvent.shift = true
}

For shifted digits/punctuation the port records the shifted character instead (Shift+7 → &, Atom philosophy). But when a primary modifier is held, macOS reports the base character for KeyboardEvent.key (⌘⇧7 → 7), so neither the shifted character nor Shift survives:

Recording normalizes ⌘⇧7 → cmd+7.
Matching normalizes a physical ⌘⇧7 → cmd+7 too, which can never equal the stamped shift+cmd+7 → the command never fires.

Verified against the real library: equals(toKeyEvent('shift+cmd+7'), normalize(physical ⌘⇧7)) === false.

Fix

Since recorder and matcher share normalizeKeyboardEvent, fixing it there repairs both consistently. Patch the library (via patch-package, the mechanism already used for native-keymap) to also keep Shift whenever a primary modifier (Cmd/Ctrl) is held:

-  if (key === 'shift' || (shiftKey && (isNonCharacterKey || (isLatinCharacter(key) && isUpperCaseCharacter(key))))) {
+  if (key === 'shift' || (shiftKey && (isNonCharacterKey || ctrlKey || metaKey || (isLatinCharacter(key) && isUpperCaseCharacter(key))))) {

After the patch (verified against the real library, record + match):

Press	Recorded	Matches physical press
⌘⇧7	`shift+cmd+7`	✅
⌃⇧7	`ctrl+shift+7`	✅
⌘⇧/	`shift+cmd+/`	✅
⌘7 (no Shift)	`cmd+7`	✅ (now distinct from the shifted combo)
⇧7 (no primary modifier)	`&`	unchanged (Atom shifted-char behaviour)
⌘⇧B (letter)	`shift+cmd+B`	unchanged

Tests

test/unit/specs/keybinding-shift-modifier.spec.ts drives the patched library through its public recorder API on the platform-independent Ctrl path (Ctrl is honoured on every OS; the meta path is gated on macOS inside the library). Since recorder and matcher share the normalize function, guarding the recorded string guards the match.

pnpm run lint, pnpm run typecheck, and the keybinding unit specs pass. The record+match fix is verified end-to-end against the real library (patched normalize makes equals(toKeyEvent(accel), normalize(physical press)) true); in-app keystroke verification in a dev build is the final manual check.

Note: an upstream fix in @hfelix/electron-localshortcut would be preferable long-term; the patch-package patch keeps MarkText working today without forking.

🤖 Generated with Claude Code

github-actions · 2026-07-05T16:04:15Z

Build artifacts for PR #4873:

Run: https://github.com/marktext/marktext/actions/runs/28748398662

Artifact	Size	Link
marktext-windows-arm64	275.7 MB	Download
marktext-linux	627.7 MB	Download
marktext-windows-x64	283.9 MB	Download
marktext-macos-x64	293.1 MB	Download
marktext-macos-arm64	282.8 MB	Download

Recording ⌘⇧7 in Preferences → Key Bindings saved `Cmd+7`, and even the correctly-spelled `Cmd+Shift+7` never fired at runtime. Both symptoms share one root cause in `@hfelix/electron-localshortcut`'s atom-keymap port: `normalizeKeyboardEvent` keeps Shift as a modifier only for non-character keys and upper-case Latin letters. For digits/punctuation the recorder is meant to store the shifted character instead, but when a primary modifier is held macOS reports the base character (⌘⇧7 -> key "7"), so Shift is dropped from BOTH the recorded accelerator and the runtime `before-input` match — the binding becomes unreachable. Because the recorder and the shortcut matcher run the same normalize function, fixing it there repairs both at once. Patch the library (via patch-package, like the existing native-keymap patch) to also keep Shift whenever ctrl/meta is held. Without a primary modifier the shifted-character behaviour is unchanged (Shift+7 -> "&"). Guarded by test/unit/specs/keybinding-shift-modifier.spec.ts, which drives the patched library through its public recorder API on the platform-independent Ctrl path. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

The setup action installs with `--ignore-scripts`, so patch-package never runs in the unit-test job and specs execute against unpatched dependencies. Apply the patches first so the tests exercise the same patched libraries the app ships — notably the @hfelix/electron-localshortcut keybinding fix (#4863), whose regression test needs the patch to be present. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Upstream PRs applied (all diff-reviewed, tests included and passing): - marktext#4776: preserve ordered-list source markers — opening and saving no longer rewrites 1./1./1. to 1./2./3. (marktext#4772, silent doc mutation) - marktext#4788: UNC/WSL paths become valid file:// authority URLs (file://server/share/…, not file:////…) so network images load (marktext#4577, marktext#4563) - marktext#4952: table columns size to content (min-width 10em → 2em, marktext#4894) - marktext#4773: malformed percent-escape in a clicked link no longer throws an unhandled URIError (marktext#4749) - marktext#4873: Shift+digit/punctuation keybindings recordable and matchable again (patch-package fix for @hfelix/electron-localshortcut, marktext#4863) - marktext#4910: sidebar icons match by extension first, so Dockerfile-Notes.md gets the markdown icon (marktext#4890) - marktext#4317: restored windows are clamped to the target display's work area (multi-monitor / DPI, marktext#2928, marktext#1947) Found while integrating marktext#4776: cloneStateTree shallow-copied meta, so array-valued fields (order-list sourceMarkers, table aligns) stayed SHARED between a getState() clone and the live document — mutating a returned tree corrupted the document. Meta arrays are now copied, and the clone walks an explicit work list instead of recursing, so pathological nesting (600-level lists, marktext#4747) can no longer overflow the call stack (10k-depth regression test). Also: folder search debounces its per-keystroke ripgrep run (300ms, Enter searches immediately, IME-composing keys ignored) (marktext#3556), and a new undoFloor spec pins that undoing past the opened baseline never empties the document (marktext#5028 — legacy-engine bug, does not reproduce on @muyajs/core). muya 1536/1536, desktop 783/783, lint/tsc/madge clean. PLANS.md gains a prioritized backlog from the full 563-item issue/PR survey.

Jocs · 2026-09-22T02:54:07Z

Closing this for now — deprioritizing #4863 rather than landing another dependency patch.

Alternatives to patch-package were evaluated; none is both clean and small:

Upstream fix. @hfelix/electron-localshortcut is a fork whose last commit is 2022-02-20 and whose sole npm maintainer is the fork owner, so a 4.0.2 is not reachable from here.
Runtime monkey-patch. src/electron-localshortcut.js destructures normalizeKeyboardEvent at module load, so overriding it would depend on require order and would not survive bundling.
Fix outside the library. Not possible: the library normalizes both ⌘⇧7 and ⌘7 to cmd+7, so the two presses are indistinguishable to its matcher. A recorder-only fix makes the saved binding unreachable, and a second before-input-event matcher would duplicate the whole normalization semantics while competing for the same key.
Vendor the library (1423 LOC, MIT, debug as its only dependency, no electron import). This is the only genuinely clean route: the Shift fix becomes ordinary source code with ordinary tests, the patch disappears, and the declare module '@hfelix/electron-localshortcut' any-shim in src/types/shims.d.ts can go away too. It is a standalone refactor, though, not part of a bug fix.

The branch fix/keybinding-recorder-shift-4863 is kept, so the analysis and the one-line fix are available if the vendoring route is taken later.

Jocs force-pushed the fix/keybinding-recorder-shift-4863 branch from 4759eb0 to fd53796 Compare July 5, 2026 16:39

Jocs changed the title ~~fix(desktop): preserve Shift when recording Cmd/Ctrl+digit keybindings (#4863)~~ fix(desktop): preserve Shift for Cmd/Ctrl+digit keybindings (#4863) Jul 5, 2026

Jocs closed this Sep 22, 2026

Jocs mentioned this pull request Sep 22, 2026

[Bug] Keybindings recorder silently drops Shift for digit/punctuation combos (⌘⇧7 records as Cmd+7) #4863

Open

1 task done

Navigation Menu

Repository navigation

fix(desktop): preserve Shift for Cmd/Ctrl+digit keybindings (#4863) - #4873

fix(desktop): preserve Shift for Cmd/Ctrl+digit keybindings (#4863)#4873
Jocs wants to merge 2 commits into
developfrom
fix/keybinding-recorder-shift-4863

Jocs commented Jul 5, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Jul 5, 2026 •

edited

Loading

Uh oh!

Jocs commented Sep 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Jocs commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix

Tests

Uh oh!

github-actions Bot commented Jul 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jocs commented Sep 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Jocs commented Jul 5, 2026 •

edited

Loading

github-actions Bot commented Jul 5, 2026 •

edited

Loading