diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index e377d43e44e8..b31c8eb0bef6 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -59,6 +59,70 @@ describe("isPreviewRefreshShortcut", () => { }); }); +describe("isPreviewEditingShortcut", () => { + const input = (platform: NodeJS.Platform, key: string, overrides: Partial = {}) => + ({ + type: "keyDown", + key, + meta: platform === "darwin", + control: platform !== "darwin", + shift: false, + alt: false, + ...overrides, + }) as Electron.Input; + + it.each(["darwin", "linux", "win32"] as const)( + "allows native editing chords on %s without allowing host shortcuts", + (platform) => { + for (const key of ["a", "c", "v", "x", "z", "V"]) { + expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(true); + } + const redo = + platform === "win32" ? input(platform, "y") : input(platform, "z", { shift: true }); + expect(PreviewManager.isPreviewEditingShortcut(redo, platform)).toBe(true); + expect( + PreviewManager.isPreviewEditingShortcut( + input(platform, "v", { shift: true, alt: platform === "darwin" }), + platform, + ), + ).toBe(true); + + for (const key of ["k", ",", "w", "j", "q", "+", "=", "-", "0", "r", "F12"]) { + expect(PreviewManager.isPreviewEditingShortcut(input(platform, key), platform)).toBe(false); + } + for (const modifiers of [ + { meta: false, control: false }, + { meta: true, control: true }, + { meta: platform !== "darwin", control: platform === "darwin" }, + { alt: true }, + { shift: true, alt: platform !== "darwin" }, + ]) { + expect( + PreviewManager.isPreviewEditingShortcut(input(platform, "v", modifiers), platform), + ).toBe(false); + } + expect( + PreviewManager.isPreviewEditingShortcut(input(platform, "a", { shift: true }), platform), + ).toBe(false); + }, + ); + + it("recognizes macOS Paste and Match Style when Option changes the key to a symbol", () => { + const pasteAndMatchStyle = input("darwin", "◊", { code: "KeyV", alt: true, shift: true }); + expect(PreviewManager.isPreviewEditingShortcut(pasteAndMatchStyle, "darwin")).toBe(true); + for (const modifiers of [ + { code: "KeyC" }, + { alt: false }, + { shift: false }, + { control: true }, + ]) { + expect( + PreviewManager.isPreviewEditingShortcut({ ...pasteAndMatchStyle, ...modifiers }, "darwin"), + ).toBe(false); + } + }); +}); + const { browserWindowConstructor, createFromBuffer, @@ -208,6 +272,7 @@ const makeTestPreviewWebContents = ( ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -284,6 +349,7 @@ const makeFaviconWebContents = (options?: { send: webviewSend, session: { fetch }, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), executeJavaScriptInIsolatedWorld, debugger: { @@ -377,6 +443,114 @@ describe("PreviewManager", () => { webviewSend.mockClear(); }); + effectIt.effect("keeps preview shortcuts out of the host window", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + const sendInputEvent = vi.fn(); + const hostWebContents = { sendInputEvent }; + Object.assign(preview.webContents, { hostWebContents }); + fromId.mockReturnValue(preview.webContents); + getFocusedWebContents.mockReturnValue(preview.webContents as never); + yield* manager.setMainWindow({ + isDestroyed: () => false, + once: vi.fn(), + webContents: hostWebContents, + } as never); + yield* manager.createTab("tab_keys"); + yield* manager.registerWebview("tab_keys", 42); + + expect( + (preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts, + ).toHaveBeenCalledWith(true); + const beforeInput = preview.listeners.get("before-input-event")!; + for (const control of [false, true]) { + for (const key of ["k", ",", "w", "j", "q", "+"]) { + for (const type of ["keyDown", "keyUp"]) { + const preventDefault = vi.fn(); + beforeInput( + { preventDefault } as never, + { type, key, meta: !control, control, shift: key === "j", alt: false } as never, + ); + yield* Effect.yieldNow; + expect(preventDefault).not.toHaveBeenCalled(); + expect( + (preview.webContents as Electron.WebContents).setIgnoreMenuShortcuts, + ).toHaveBeenLastCalledWith(true); + } + } + } + expect(sendInputEvent).not.toHaveBeenCalled(); + + const preventDefault = vi.fn(); + beforeInput( + { preventDefault } as never, + { + type: "keyDown", + key: "r", + meta: true, + control: false, + shift: false, + alt: false, + } as never, + ); + yield* Effect.yieldNow; + expect(preventDefault).toHaveBeenCalledOnce(); + expect(preview.reload).toHaveBeenCalledOnce(); + expect(sendInputEvent).not.toHaveBeenCalled(); + }), + ), + ); + + effectIt.effect("preserves focused browser editing in tabs and sign-in popups", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + fromId.mockReturnValue(preview.webContents); + yield* manager.createTab("tab_editing"); + yield* manager.registerWebview("tab_editing", 42); + + const popup = makeFaviconWebContents({ id: 43 }); + preview.listeners.get("did-create-window")!({ webContents: popup.webContents } as never); + expect( + (popup.webContents as Electron.WebContents).setIgnoreMenuShortcuts, + ).toHaveBeenCalledWith(true); + + for (const browser of [preview, popup]) { + const contents = browser.webContents as Electron.WebContents; + getFocusedWebContents.mockReturnValue(browser.webContents as never); + const beforeInput = browser.listeners.get("before-input-event")!; + const preventDefault = vi.fn(); + const input = { + type: "keyDown", + key: "v", + meta: true, + control: false, + shift: false, + alt: false, + }; + beforeInput({ preventDefault } as never, input as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false); + // Releasing Command must not disable native fallback for the pending paste. + beforeInput( + { preventDefault } as never, + { ...input, type: "keyUp", key: "Meta", meta: false } as never, + ); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(false); + + beforeInput({ preventDefault } as never, { ...input, key: "w" } as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true); + + // An injected paste in an unfocused guest cannot edit the active renderer. + getFocusedWebContents.mockReturnValue(null); + beforeInput({ preventDefault } as never, input as never); + expect(contents.setIgnoreMenuShortcuts).toHaveBeenLastCalledWith(true); + expect(preventDefault).not.toHaveBeenCalled(); + } + }), + ), + ); + effectIt.effect("reports an unregistered webview as temporarily unavailable", () => withManager((manager) => Effect.gen(function* () { @@ -520,6 +694,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -621,6 +796,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), get debugger() { if (destroyed) throw new Error("Object has been destroyed"); @@ -1125,6 +1301,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1189,6 +1366,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1229,6 +1407,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1275,6 +1454,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1330,6 +1510,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1427,6 +1608,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1760,6 +1942,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -1852,6 +2035,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -2151,6 +2335,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -2363,6 +2548,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -2448,6 +2634,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3065,6 +3252,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn(), removeListener: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3123,6 +3311,7 @@ describe("PreviewManager", () => { goBack, goForward, }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3252,6 +3441,7 @@ describe("PreviewManager", () => { }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3562,6 +3752,7 @@ describe("PreviewManager", () => { }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3717,6 +3908,7 @@ describe("PreviewManager", () => { }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, @@ -3781,6 +3973,7 @@ describe("PreviewManager", () => { ipc: { on: vi.fn(), off: vi.fn() }, send: webviewSend, navigationHistory: { canGoBack: () => false, canGoForward: () => false }, + setIgnoreMenuShortcuts: vi.fn(), setWindowOpenHandler: vi.fn(), debugger: { isAttached: () => false, diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index f08400cbc6d1..d8209c680eda 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -470,22 +470,6 @@ interface ExpectedAgentInput { readonly expiresAt: number; } -const APP_FORWARDED_SHORTCUTS: ReadonlyArray<{ - key: string; - meta: boolean; - shift: boolean; - control: boolean; -}> = Object.freeze([ - // mod+shift+J → preview.toggle - { key: "j", meta: true, shift: true, control: false }, - // mod+K → command palette - { key: "k", meta: true, shift: false, control: false }, - // mod+, → settings (macOS convention) - { key: ",", meta: true, shift: false, control: false }, - // mod+W → close tab/panel - { key: "w", meta: true, shift: false, control: false }, -]); - export const isPreviewRefreshShortcut = (input: Electron.Input): boolean => input.type === "keyDown" && input.key.toLowerCase() === "r" && @@ -493,6 +477,29 @@ export const isPreviewRefreshShortcut = (input: Electron.Input): boolean => !input.shift && !input.alt; +export const isPreviewEditingShortcut = ( + input: Electron.Input, + platform: NodeJS.Platform, +): boolean => { + const isMac = platform === "darwin"; + if (isMac ? !input.meta || input.control : !input.control || input.meta) return false; + + const key = input.key.toLowerCase(); + // Option changes the DOM key for macOS Paste and Match Style (for example, to ◊). + if (isMac && input.alt && input.shift && input.code === "KeyV") return true; + if (key === "v" && input.shift) return input.alt === isMac; + if (input.alt) return false; + if (key === "z") return !input.shift || platform !== "win32"; + if (input.shift) return false; + return ( + key === "a" || + key === "c" || + key === "v" || + key === "x" || + (key === "y" && platform === "win32") + ); +}; + const isPreviewInputSignal = (value: unknown): value is PreviewInputSignal => { if (typeof value !== "object" || value === null || !("kind" in value)) return false; if (value.kind === "pointer") { @@ -1416,16 +1423,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function } }); - const isAppShortcut = (input: Electron.Input): boolean => - input.type === "keyDown" && - APP_FORWARDED_SHORTCUTS.some( - (shortcut) => - shortcut.key.toLowerCase() === input.key.toLowerCase() && - shortcut.meta === input.meta && - shortcut.shift === input.shift && - shortcut.control === input.control, - ); - const computeNavStatus = (wc: Electron.WebContents): PreviewNavStatus => { const url = wc.getURL(); const title = wc.getTitle(); @@ -1700,27 +1697,26 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }).pipe(Effect.ignore), ); }; - const forwardShortcut = Effect.fn("PreviewManager.forwardShortcut")(function* ( - event: Electron.Event, - input: Electron.Input, - ) { - const mainWindow = yield* Ref.get(mainWindowRef); - if (!isAppShortcut(input) || Option.isNone(mainWindow) || mainWindow.value.isDestroyed()) { - return; - } - event.preventDefault(); - mainWindow.value.webContents.sendInputEvent({ - type: "keyDown", - keyCode: input.key, - modifiers: [ - ...(input.meta ? (["meta"] as const) : []), - ...(input.shift ? (["shift"] as const) : []), - ...(input.control ? (["control"] as const) : []), - ...(input.alt ? (["alt"] as const) : []), - ], + const syncMenuShortcuts = (contents: Electron.WebContents, input: Electron.Input): void => { + if (input.type !== "keyDown") return; + // Native editing roles must remain available after the page handles the key. + // Background automation must not edit whichever other renderer has focus. + contents.setIgnoreMenuShortcuts( + !isPreviewEditingShortcut(input, hostPlatform) || + webContents.getFocusedWebContents() !== contents, + ); + }; + // Akeru currently denies window.open and loads the URL in the same guest. + // If a popup is created anyway, keep its shortcuts isolated from the host. + const windowCreated = (window: Electron.BrowserWindow): void => { + window.webContents.setIgnoreMenuShortcuts(true); + window.webContents.setWindowOpenHandler(() => ({ action: "deny" })); + window.webContents.on("before-input-event", (_event, input) => { + syncMenuShortcuts(window.webContents, input); }); - }); + }; const beforeInput = (event: Electron.Event, input: Electron.Input): void => { + syncMenuShortcuts(wc, input); if (isPreviewRefreshShortcut(input)) { event.preventDefault(); runFork( @@ -1728,9 +1724,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.reload(), ).pipe(Effect.ignore), ); - return; } - runFork(forwardShortcut(event, input)); }; yield* Scope.addFinalizer( scope, @@ -1745,6 +1739,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.off("did-stop-loading", sync); wc.off("did-fail-load", failed as never); wc.off("audio-state-changed", audioStateChanged); + wc.off("did-create-window", windowCreated); wc.off("before-input-event", beforeInput); wc.ipc.off(HUMAN_INPUT_CHANNEL, humanInput); wc.ipc.off(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); @@ -1752,6 +1747,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ); const install = Effect.fn("PreviewManager.installWebContentsListeners")(function* () { yield* attempt({ operation: "attachListeners", tabId, webContentsId: wc.id }, () => { + // Only focused native editing shortcuts may reach the application menu. + // Other preview input, including CDP keys, belongs to the page. + wc.setIgnoreMenuShortcuts(true); wc.on("did-start-navigation", navigationStarted); wc.on("did-navigate", syncNavigation); wc.on("did-navigate-in-page", syncInPageNavigation); @@ -1771,6 +1769,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ); return { action: "deny" }; }); + wc.on("did-create-window", windowCreated); wc.on("before-input-event", beforeInput); }); yield* Ref.update(attachedRef, (attached) =>