From b36f2f4d4ec255f368984e3e65733237935338d8 Mon Sep 17 00:00:00 2001 From: Lambda Date: Mon, 13 Jul 2026 15:56:18 +0800 Subject: [PATCH] fix(shortcuts): limit desktop unlock to bare primary browser accelerators Review follow-up (MUL-4457): the desktop skip matched primary+key with any extra modifiers, which would also unreserve OS-owned combos such as Option+Cmd+D (macOS Dock toggle) and Ctrl+Alt+T (Linux terminal). Only the bare primary chord is now recordable on desktop; every extra-modifier variant keeps the historical reservation on both runtimes. Adds regression tests for the OS combos. Co-authored-by: multica-agent --- packages/core/shortcuts/definitions.test.ts | 32 ++++++++++++++++++--- packages/core/shortcuts/definitions.ts | 20 +++++++------ 2 files changed, 39 insertions(+), 13 deletions(-) diff --git a/packages/core/shortcuts/definitions.test.ts b/packages/core/shortcuts/definitions.test.ts index c743444308..7fb7e5d9d2 100644 --- a/packages/core/shortcuts/definitions.test.ts +++ b/packages/core/shortcuts/definitions.test.ts @@ -135,14 +135,38 @@ describe("keyboard shortcut definitions", () => { ).toBe(false); }); - it("reserves browser-owned accelerators on web but frees them on desktop", () => { + it("reserves browser-owned accelerators on web but frees the bare chords on desktop", () => { for (const key of ["P", "L", "T", "N", "D", "U"]) { const chord = createShortcutChord(key, { primary: true }); expect(isReservedShortcut(chord, "macos", "web")).toBe(true); expect(isReservedShortcut(chord, "windows", "web")).toBe(true); expect(isReservedShortcut(chord, "macos", "desktop")).toBe(false); expect(isReservedShortcut(chord, "windows", "desktop")).toBe(false); + // Only the bare primary chord opens up — extra modifiers keep the + // historical reservation on every runtime. + for (const extra of [{ shift: true }, { alt: true }, { control: true }]) { + const variant = createShortcutChord(key, { primary: true, ...extra }); + expect(isReservedShortcut(variant, "macos", "desktop")).toBe(true); + expect(isReservedShortcut(variant, "macos", "web")).toBe(true); + } } + // OS-owned combos that motivated the narrowing must stay reserved on + // desktop: Option+Cmd+D toggles the macOS Dock, Ctrl+Alt+T opens a + // terminal on common Linux desktops. + expect( + isReservedShortcut( + createShortcutChord("D", { primary: true, alt: true }), + "macos", + "desktop", + ), + ).toBe(true); + expect( + isReservedShortcut( + createShortcutChord("T", { primary: true, alt: true }), + "linux", + "desktop", + ), + ).toBe(true); }); it("keeps app-owned and editing accelerators reserved on desktop", () => { @@ -157,12 +181,12 @@ describe("keyboard shortcut definitions", () => { ).toBe(true); }); - it("allows recording Cmd/Ctrl+P for an action on desktop only", () => { + it("allows recording bare Cmd/Ctrl+P for an action on desktop only", () => { const cmdP = createShortcutChord("P", { primary: true }); expect(isShortcutAllowedForAction("openSearch", cmdP, "macos", "desktop")).toBe(true); expect(isShortcutAllowedForAction("openSearch", cmdP, "macos", "web")).toBe(false); expect(isShortcutAllowedForAction("goIssues", cmdP, "windows", "desktop")).toBe(true); - // The reservation keys on primary+key, so shifted variants open up too. + // Modifier variants keep the historical reservation even on desktop. expect( isShortcutAllowedForAction( "openSearch", @@ -170,7 +194,7 @@ describe("keyboard shortcut definitions", () => { "macos", "desktop", ), - ).toBe(true); + ).toBe(false); // Send stays locked to Enter / Mod+Enter regardless of runtime. expect(isShortcutAllowedForAction("send", cmdP, "macos", "desktop")).toBe(false); }); diff --git a/packages/core/shortcuts/definitions.ts b/packages/core/shortcuts/definitions.ts index 1ab96753ec..10bdb69edf 100644 --- a/packages/core/shortcuts/definitions.ts +++ b/packages/core/shortcuts/definitions.ts @@ -249,9 +249,13 @@ const PRIMARY_RESERVED_KEYS = new Set([ // Accelerators owned by the browser UI around a tab: print, address bar, // new tab/window, bookmark, view source. A web page cannot reliably own -// them, but the Electron renderer receives them as plain keydowns — neither -// Electron's default menu nor the desktop shell binds any of them — so they -// are recordable on desktop (MUL-4457). +// them, but the Electron renderer receives the bare primary chords as plain +// keydowns — neither Electron's default menu nor the desktop shell binds +// any of them — so exactly those are recordable on desktop (MUL-4457). +// Variants with extra modifiers stay reserved on both runtimes: several +// belong to the OS or window manager (Option+Cmd+D toggles the macOS Dock, +// Ctrl+Alt+T opens a terminal on common Linux desktops), which even the +// desktop app cannot own. const BROWSER_ONLY_PRIMARY_RESERVED_KEYS = new Set([ "P", "L", "T", "N", "D", "U", ]); @@ -265,12 +269,10 @@ export function isReservedShortcut( const { modifiers, key } = shortcut; if (key === "F5") return true; if (modifiers.primary && PRIMARY_RESERVED_KEYS.has(key)) return true; - if ( - runtime === "web" && - modifiers.primary && - BROWSER_ONLY_PRIMARY_RESERVED_KEYS.has(key) - ) { - return true; + if (modifiers.primary && BROWSER_ONLY_PRIMARY_RESERVED_KEYS.has(key)) { + const barePrimary = + !modifiers.control && !modifiers.meta && !modifiers.alt && !modifiers.shift; + if (runtime !== "desktop" || !barePrimary) return true; } if (platform === "macos") {