diff --git a/packages/core/platform/core-provider.tsx b/packages/core/platform/core-provider.tsx index 21f07e518f..34bee14bf5 100644 --- a/packages/core/platform/core-provider.tsx +++ b/packages/core/platform/core-provider.tsx @@ -18,7 +18,10 @@ import { defaultStorage } from "./storage"; import { AuthInitializer } from "./auth-initializer"; import type { CoreProviderProps, ClientIdentity } from "./types"; import type { StorageAdapter } from "../types/storage"; -import { configureShortcutPlatform } from "../shortcuts/platform"; +import { + configureShortcutPlatform, + configureShortcutRuntime, +} from "../shortcuts/platform"; // Module-level singletons — created once at first render, never recreated. // Vite HMR preserves module-level state, so these survive hot reloads. @@ -43,6 +46,11 @@ function initCore( ? identity.os : null, ); + // Authoritative override; before this runs (module-eval store hydration) + // detectShortcutRuntime() reads the preload globals and already agrees. + configureShortcutRuntime( + identity?.platform === "desktop" ? "desktop" : null, + ); const api = new ApiClient(apiBaseUrl, { logger: createLogger("api"), diff --git a/packages/core/shortcuts/definitions.test.ts b/packages/core/shortcuts/definitions.test.ts index 2defda5bd0..7fb7e5d9d2 100644 --- a/packages/core/shortcuts/definitions.test.ts +++ b/packages/core/shortcuts/definitions.test.ts @@ -9,7 +9,13 @@ import { shortcutFromEvent, shortcutMatchesEvent, } from "./definitions"; -import { configureShortcutPlatform, detectShortcutPlatform } from "./platform"; +import { + configureShortcutPlatform, + configureShortcutRuntime, + detectShortcutPlatform, + detectShortcutRuntime, + getShortcutRuntime, +} from "./platform"; function keyEvent( key: string, @@ -27,6 +33,7 @@ function keyEvent( afterEach(() => { configureShortcutPlatform(null); + configureShortcutRuntime(null); vi.unstubAllGlobals(); }); @@ -128,6 +135,83 @@ describe("keyboard shortcut definitions", () => { ).toBe(false); }); + 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", () => { + for (const key of ["W", "R", "Q", "A", "C", "V", "X", "Y", "Z", "0", "Minus", "Plus"]) { + expect( + isReservedShortcut(createShortcutChord(key, { primary: true }), "macos", "desktop"), + ).toBe(true); + } + expect(isReservedShortcut(createShortcutChord("F5"), "windows", "desktop")).toBe(true); + expect( + isReservedShortcut(createShortcutChord("Space", { primary: true }), "macos", "desktop"), + ).toBe(true); + }); + + 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); + // Modifier variants keep the historical reservation even on desktop. + expect( + isShortcutAllowedForAction( + "openSearch", + createShortcutChord("P", { primary: true, shift: true }), + "macos", + "desktop", + ), + ).toBe(false); + // Send stays locked to Enter / Mod+Enter regardless of runtime. + expect(isShortcutAllowedForAction("send", cmdP, "macos", "desktop")).toBe(false); + }); + + it("detects the desktop runtime from preload globals with a configure override", () => { + expect(detectShortcutRuntime()).toBe("web"); + vi.stubGlobal("window", { desktopAPI: {} }); + expect(detectShortcutRuntime()).toBe("desktop"); + vi.stubGlobal("window", { electron: {} }); + expect(detectShortcutRuntime()).toBe("desktop"); + vi.stubGlobal("window", {}); + expect(detectShortcutRuntime()).toBe("web"); + expect(getShortcutRuntime()).toBe("web"); + configureShortcutRuntime("desktop"); + expect(getShortcutRuntime()).toBe("desktop"); + }); + it("rejects modifier-only, composition-only, and unidentified key events", () => { for (const key of ["Fn", "CapsLock", "Dead", "Process", "Unidentified"]) { expect(shortcutFromEvent(keyEvent(key), "macos")).toBeNull(); diff --git a/packages/core/shortcuts/definitions.ts b/packages/core/shortcuts/definitions.ts index 1282558a34..10bdb69edf 100644 --- a/packages/core/shortcuts/definitions.ts +++ b/packages/core/shortcuts/definitions.ts @@ -1,6 +1,8 @@ import { getShortcutPlatform, + getShortcutRuntime, type ShortcutPlatform, + type ShortcutRuntime, } from "./platform"; export type ShortcutActionId = @@ -236,21 +238,42 @@ export function isEditableShortcutTarget(target: EventTarget | null): boolean { } const PRIMARY_RESERVED_KEYS = new Set([ - // Window/browser operations. - "W", "R", "L", "T", "N", "P", "Q", "D", "U", + // Window operations the app itself owns on every runtime: W closes the + // tab, R/F5 is the reload guard, Q quits. + "W", "R", "Q", // Fundamental editing operations should never become product actions. "A", "C", "V", "X", "Y", "Z", + // Zoom accelerators: fixed app shortcuts on desktop, browser zoom on web. "Equals", "Plus", "Minus", "Underscore", "0", ]); -/** Browser/window/OS accelerators that cannot be reliably owned by the app. */ +// 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 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", +]); + +/** Browser/window/OS accelerators the app cannot reliably own in `runtime`. */ export function isReservedShortcut( shortcut: ShortcutChord, platform: ShortcutPlatform = getShortcutPlatform(), + runtime: ShortcutRuntime = getShortcutRuntime(), ): boolean { const { modifiers, key } = shortcut; if (key === "F5") return true; if (modifiers.primary && 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") { if (modifiers.primary && (key === "Space" || key === "Tab" || key === "M" || key === "H")) return true; @@ -292,9 +315,10 @@ export function isShortcutAllowedForAction( actionId: ShortcutActionId, shortcut: ShortcutChord, platform: ShortcutPlatform = getShortcutPlatform(), + runtime: ShortcutRuntime = getShortcutRuntime(), ): boolean { if (!isShortcutChordActionable(shortcut)) return false; - if (isReservedShortcut(shortcut, platform)) return false; + if (isReservedShortcut(shortcut, platform, runtime)) return false; // Tab is focus navigation (and Ctrl+Tab is browser tab navigation) on every // supported platform. It is never a dependable product-level binding. if (shortcut.key === "Tab") return false; diff --git a/packages/core/shortcuts/index.ts b/packages/core/shortcuts/index.ts index 800c119b42..883b2ebd74 100644 --- a/packages/core/shortcuts/index.ts +++ b/packages/core/shortcuts/index.ts @@ -18,9 +18,13 @@ export { } from "./definitions"; export { configureShortcutPlatform, + configureShortcutRuntime, detectShortcutPlatform, + detectShortcutRuntime, getShortcutPlatform, + getShortcutRuntime, type ShortcutPlatform, + type ShortcutRuntime, } from "./platform"; export { useShortcutStore, diff --git a/packages/core/shortcuts/platform.ts b/packages/core/shortcuts/platform.ts index 27fea6ecb6..ea428cf104 100644 --- a/packages/core/shortcuts/platform.ts +++ b/packages/core/shortcuts/platform.ts @@ -1,6 +1,10 @@ export type ShortcutPlatform = "macos" | "windows" | "linux" | "unknown"; +/** Where shortcut handling runs: a browser tab or the Electron renderer. */ +export type ShortcutRuntime = "web" | "desktop"; + let configuredPlatform: ShortcutPlatform | null = null; +let configuredRuntime: ShortcutRuntime | null = null; function normalizePlatform(value: string): ShortcutPlatform { const platform = value.toLowerCase(); @@ -37,3 +41,35 @@ export function configureShortcutPlatform( export function getShortcutPlatform(): ShortcutPlatform { return configuredPlatform ?? detectShortcutPlatform(); } + +/** + * Environment fallback, and unlike the OS it must be trustworthy at + * module-evaluation time: the shortcut store hydrates (and sanitizes + * persisted overrides) when its module first loads, before CoreProvider + * gets a chance to configure anything. That is safe because the desktop + * preload exposes its bridge globals before any renderer script runs. + * Same signals as `detectClientType` in ../analytics — not imported so + * this module stays dependency-free. + */ +export function detectShortcutRuntime(): ShortcutRuntime { + if (typeof window === "undefined") return "web"; + const w = window as unknown as { electron?: unknown; desktopAPI?: unknown }; + if (w.electron || w.desktopAPI) return "desktop"; + if ( + typeof navigator !== "undefined" && + /Electron/i.test(navigator.userAgent) + ) { + return "desktop"; + } + return "web"; +} + +export function configureShortcutRuntime( + runtime: ShortcutRuntime | null | undefined, +): void { + configuredRuntime = runtime ?? null; +} + +export function getShortcutRuntime(): ShortcutRuntime { + return configuredRuntime ?? detectShortcutRuntime(); +} diff --git a/packages/core/shortcuts/store.test.ts b/packages/core/shortcuts/store.test.ts index 5dbd0dfcc3..d6f21d6030 100644 --- a/packages/core/shortcuts/store.test.ts +++ b/packages/core/shortcuts/store.test.ts @@ -1,5 +1,6 @@ import { afterEach, describe, expect, it } from "vitest"; import { createShortcutChord } from "./definitions"; +import { configureShortcutRuntime } from "./platform"; import { findShortcutConflict, getShortcut, @@ -10,6 +11,7 @@ import { afterEach(() => { useShortcutStore.getState().resetAll(); + configureShortcutRuntime(null); }); describe("shortcut store", () => { @@ -68,6 +70,21 @@ describe("shortcut store", () => { ).toEqual({}); }); + it("keeps browser-only bindings when the runtime is desktop", () => { + const cmdP = createShortcutChord("P", { primary: true }); + + configureShortcutRuntime("desktop"); + expect(sanitizeShortcutOverrides({ openSearch: cmdP })).toEqual({ + openSearch: cmdP, + }); + useShortcutStore.getState().setShortcut("openSearch", cmdP); + expect(getShortcut("openSearch")).toEqual(cmdP); + + // The same persisted value hydrating in a browser falls back to default. + configureShortcutRuntime("web"); + expect(sanitizeShortcutOverrides({ openSearch: cmdP })).toEqual({}); + }); + it("finds conflicts against defaults and overrides", () => { expect( findShortcutConflict(