From c8ddadfb0c3924a00393c950fbcd8ed49e37f1a1 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Mon, 19 Oct 2026 22:12:18 -0700 Subject: [PATCH] fix(app): keep app shortcuts working inside the browser pane (#56697) --- .../browser-pane-restore.spec.ts | 37 ++++++- .../gui-extensions/src/browser/chromium.ts | 13 ++- .../src/browser/connection.test.ts | 1 + .../gui-extensions/src/browser/connection.ts | 6 + packages/gui-extensions/src/browser/ipc.ts | 10 ++ .../gui-extensions/src/browser/keys.test.ts | 103 +++++++++++++++++- packages/gui-extensions/src/browser/keys.ts | 51 +++++++++ packages/gui-extensions/src/browser/model.ts | 7 ++ packages/gui-extensions/src/browser/pane.ts | 7 ++ .../src/browser/panel.fixture.tsx | 51 ++++++++- packages/gui-extensions/src/browser/panel.tsx | 24 +++- 11 files changed, 304 insertions(+), 6 deletions(-) diff --git a/packages/app/component-tests/browser-pane-restore.spec.ts b/packages/app/component-tests/browser-pane-restore.spec.ts index 34d8ed60bb9..c7673f204fb 100644 --- a/packages/app/component-tests/browser-pane-restore.spec.ts +++ b/packages/app/component-tests/browser-pane-restore.spec.ts @@ -81,7 +81,7 @@ story.beforeEach(async ({ mount, page }) => { }) }, }), - Commands: (props: { close: () => void }) => { + Commands: (props: { close: () => void; review: () => void }) => { const command = commands.useCommand() const extensions = host.useExtensionHost() command.register("fixture-session", () => [ @@ -91,6 +91,12 @@ story.beforeEach(async ({ mount, page }) => { keybind: "mod+w", onSelect: props.close, }, + { + id: "review.toggle", + title: "Toggle review", + keybind: "mod+shift+r", + onSelect: props.review, + }, ]) // Storybook aliases the app's command hook to a no-op. Bridge the real extension contributions // to the real dispatcher imported by source URL, so the shortcut runs rather than its alias. @@ -185,6 +191,35 @@ story("toggles the element picker from the shortcut only inside the browser pane ) }) +story("keeps in-page shortcuts like ⌘K in the page while forwarding ⌘⇧R and ⌘W to the pane", async ({ page }) => { + const root = page.getByTestId("browser-region-fixture") + const mac = await page.evaluate(() => /Mac|iPhone|iPad|iPod/.test(navigator.platform)) + const mod = mac ? "Meta" : "Control" + + const editor = page + .frameLocator('iframe[title="Browser page"]') + .getByRole("textbox", { name: "Editor page input", exact: true }) + + await expect(root.getByText("Registrations: 1", { exact: true })).toBeVisible() + await root.getByRole("button", { name: "Beta", exact: true }).click() + await expect(root.getByText("Registrations: 2", { exact: true })).toBeVisible() + await root.getByRole("button", { name: "First inventory", exact: true }).click() + await expect(root.getByRole("tab", { name: "Preview", exact: true })).toHaveAttribute("aria-selected", "true") + + // In-page shortcuts like ⌘K stay with the focused page. + await editor.focus() + await editor.press(`${mod}+k`) + await expect(editor).toHaveValue("handled:k") + + // Shell shortcuts like ⌘⇧R and pane-scoped ⌘W are claimed before the page sees them. + await editor.press(`${mod}+Shift+r`) + await expect(root.getByTestId("review-toggles")).toHaveText("1") + await editor.press(`${mod}+w`) + await expect(editor).toHaveValue("handled:k") + await expect(root.getByTestId("browser-closes")).toHaveText('["tab_33333333-3333-3333-3333-333333333333"]') + await expect(root.getByTestId("session-state")).toHaveText("open") +}) + story("keeps a restored browser tab selected and undrawn until the desktop's first inventory", async ({ page }) => { const root = page.getByTestId("browser-region-fixture") const tabs = root.getByRole("tab") diff --git a/packages/gui-extensions/src/browser/chromium.ts b/packages/gui-extensions/src/browser/chromium.ts index e4d272cd574..bec25087997 100644 --- a/packages/gui-extensions/src/browser/chromium.ts +++ b/packages/gui-extensions/src/browser/chromium.ts @@ -7,7 +7,7 @@ import { createCdp, abortError, waitFor } from "./cdp" import { createBrowserFiles } from "./files" import { createDiagnostics } from "./diagnostics" import { loadIcon } from "./icon" -import { pageLetter } from "./keys" +import { pageLetter, pageShortcut, type PageShortcut } from "./keys" import { createProfiling, type Recording } from "./profiling" import type { BrowserNetwork } from "./network" import { @@ -137,6 +137,8 @@ export function createBrowserPage( zoomed?: () => void /** The user pressed the address shortcut while the page had focus. */ address?: () => void + /** Hands a browser or shell shortcut to the app window. */ + shortcut?: (shortcut: PageShortcut) => void fail: () => void /** Adopts a page the document opened; a background one leaves the current tab selected. */ popup: (options: Electron.BrowserWindowConstructorOptions, background: boolean) => WebContents @@ -190,6 +192,15 @@ export function createBrowserPage( return } + const shortcut = pageShortcut(input, process.platform === "darwin") + + if (shortcut) { + event.preventDefault() + options.shortcut?.(shortcut) + + return + } + if (input.alt || !(process.platform === "darwin" ? input.meta : input.control)) return // The same chord as Chromium DevTools' element picker. diff --git a/packages/gui-extensions/src/browser/connection.test.ts b/packages/gui-extensions/src/browser/connection.test.ts index 5e4899b2e91..58ee7218bac 100644 --- a/packages/gui-extensions/src/browser/connection.test.ts +++ b/packages/gui-extensions/src/browser/connection.test.ts @@ -111,6 +111,7 @@ function fixture(strip: string[] = []) { inspect: (event) => routed.inspect.push(event), page: () => undefined, address: () => undefined, + shortcut: () => undefined, }) const emit = (index: number, event: PaneEvent) => listeners.get(calls[index].input.binding)?.(event) diff --git a/packages/gui-extensions/src/browser/connection.ts b/packages/gui-extensions/src/browser/connection.ts index 2118537483f..b71804c1090 100644 --- a/packages/gui-extensions/src/browser/connection.ts +++ b/packages/gui-extensions/src/browser/connection.ts @@ -8,6 +8,8 @@ export type InspectEvent = Extract export type PageEvent = Extract +export type ShortcutEvent = Extract + export type Zoom = "in" | "out" | "reset" export type Connection = ReturnType @@ -58,6 +60,8 @@ export function createConnection(input: { page: (event: PageEvent) => void /** The user pressed the address shortcut while the page had focus. */ address: (tabID: Browser.TabID) => void + /** A focused page handed a browser or shell shortcut to the window. */ + shortcut: (event: ShortcutEvent) => void }) { const state: ConnectionState = { browser: null, embeds: {}, suspended: false } let disposed = false @@ -158,6 +162,8 @@ export function createConnection(input: { if (event.type === "address") return input.address(event.tabID) + if (event.type === "shortcut") return input.shortcut(event) + if (event.type === "embed") { state.embeds = { ...state.embeds, [event.tabID]: event.embed } diff --git a/packages/gui-extensions/src/browser/ipc.ts b/packages/gui-extensions/src/browser/ipc.ts index 00e74407cc3..c6d385bc895 100644 --- a/packages/gui-extensions/src/browser/ipc.ts +++ b/packages/gui-extensions/src/browser/ipc.ts @@ -52,6 +52,16 @@ export const PaneEvent = Schema.Union([ }), // The page asked for the address field, with the platform's address shortcut. Schema.Struct({ type: Schema.Literal("address"), tabID: Browser.TabID }), + // The page handed a browser or shell shortcut to the window, which runs it as its own key press. + Schema.Struct({ + type: Schema.Literal("shortcut"), + key: text(64), + code: detail(64), + ctrl: Schema.Boolean, + meta: Schema.Boolean, + shift: Schema.Boolean, + alt: Schema.Boolean, + }), ]) export type PaneEvent = typeof PaneEvent.Type diff --git a/packages/gui-extensions/src/browser/keys.test.ts b/packages/gui-extensions/src/browser/keys.test.ts index 94285bf52a7..fd1427c8807 100644 --- a/packages/gui-extensions/src/browser/keys.test.ts +++ b/packages/gui-extensions/src/browser/keys.test.ts @@ -1,5 +1,20 @@ import { expect, test } from "bun:test" -import { pageLetter } from "./keys" +import { pageLetter, pageShortcut, type PageKey } from "./keys" + +/** A key press with `mod` as the platform's modifier: Command on macOS, Control elsewhere. */ +const press = (modifiers: string, key: string, mac: boolean, code = ""): PageKey => { + const held = modifiers.split("+") + const mod = held.includes("mod") + + return { + key, + code, + control: held.includes("ctrl") || (mod && !mac), + meta: mod && mac, + shift: held.includes("shift"), + alt: held.includes("alt"), + } +} test.each([ { layout: "QWERTY", input: { key: "C", code: "KeyC" }, letter: "c" }, @@ -13,3 +28,89 @@ test.each([ ])("$layout $input.key on $input.code is $letter", (row) => { expect(pageLetter(row.input)).toBe(row.letter) }) + +test("forwards browser and shell shortcuts while leaving page editing, find, reload and in-page chords to the page", () => { + const cases: [modifiers: string, key: string, mac: boolean, forwarded: string | undefined, code?: string][] = [ + // Browser tab, window and shell chords forwarded from the page. + ["mod", "t", true, "t"], + ["mod", "t", false, "t"], + // Dvorak: T is on the physical K key and Y on the physical T key; the character decides. + ["mod", "t", true, "t", "KeyK"], + ["mod", "y", true, undefined, "KeyT"], + ["mod", "w", true, "w"], + ["mod", "w", false, "w"], + ["mod", "\u0432", true, "w", "KeyW"], + ["mod", "n", true, "n"], + ["mod", "n", false, "n"], + ["mod", "1", true, "1"], + ["mod", "9", false, "9"], + ["mod", ",", true, ","], + ["mod", "\\", false, "\\"], + ["mod+shift", "T", true, "t"], + ["mod+shift", "R", true, "r"], + ["mod+shift", "R", false, "r"], + ["mod+shift", "P", true, "p"], + ["mod+shift", "L", true, "l"], + ["mod+shift", "E", true, "e"], + ["mod+alt", "ArrowLeft", true, "ArrowLeft"], + ["mod+alt", "ArrowRight", false, "ArrowRight"], + ["ctrl", "Tab", true, "Tab"], + ["ctrl+shift", "Tab", false, "Tab"], + ["ctrl", "`", true, "`"], + ["ctrl", "`", false, "`"], + // New Window and Close Window belong to native menu accelerators, which a claimed chord would block. + ["mod+shift", "W", true, undefined], + ["mod+shift", "W", false, undefined], + ["mod+shift", "N", true, undefined], + ["mod+shift", "N", false, undefined], + // In-page editor chords (link/command menu, bold, italic, save) stay with the page. + ["mod", "k", true, undefined], + ["mod", "k", false, undefined], + ["mod", "b", true, undefined], + ["mod", "i", true, undefined], + ["mod", "s", true, undefined], + // Clipboard, select all, undo and redo stay with the page. + ["mod", "c", true, undefined], + ["mod", "x", true, undefined], + ["mod", "v", true, undefined], + ["mod+shift", "V", true, undefined], + ["mod", "a", true, undefined], + ["mod", "z", true, undefined], + ["mod+shift", "Z", true, undefined], + ["mod", "c", false, undefined], + ["mod", "v", false, undefined], + ["mod", "a", false, undefined], + ["mod", "z", false, undefined], + ["mod", "y", false, undefined], + // Find, reload, history, and the pane's own address, picker and zoom chords. + ["mod", "f", true, undefined], + ["mod", "g", true, undefined], + ["mod+shift", "G", true, undefined], + ["mod", "r", true, undefined], + ["mod", "r", false, undefined], + ["mod", "[", true, undefined], + ["mod", "]", true, undefined], + ["mod", "l", true, undefined], + ["mod+shift", "C", true, undefined], + ["mod", "=", true, undefined], + ["mod", "-", true, undefined], + ["mod", "0", false, undefined], + // Caret movement, macOS Control keys, and plain typing. + ["mod", "ArrowLeft", true, undefined], + ["mod+shift", "ArrowRight", false, undefined], + ["mod", "Backspace", false, undefined], + ["ctrl", "b", true, undefined], + ["ctrl+shift", "E", true, undefined], + ["", "k", true, undefined], + ["shift", "Tab", true, undefined], + ] + + const results = cases.map(([modifiers, key, mac, , code]) => [ + modifiers, + key, + mac, + pageShortcut(press(modifiers, key, mac, code), mac)?.key, + ]) + + expect(results).toEqual(cases.map(([modifiers, key, mac, forwarded]) => [modifiers, key, mac, forwarded])) +}) diff --git a/packages/gui-extensions/src/browser/keys.ts b/packages/gui-extensions/src/browser/keys.ts index 7aaecf9cc8a..555b839690a 100644 --- a/packages/gui-extensions/src/browser/keys.ts +++ b/packages/gui-extensions/src/browser/keys.ts @@ -1,3 +1,24 @@ +/** A key press in the page, with Electron's `Input` field names. */ +export type PageKey = Pick + +export type PageShortcut = { + readonly key: string + readonly code: string + readonly ctrl: boolean + readonly meta: boolean + readonly shift: boolean + readonly alt: boolean +} + +// Unshifted chords with the platform's modifier that belong to the app shell or browser pane rather than the page: +// opening, closing or switching tabs, opening settings, and toggling the file tree. +const shellKeys = ["t", "w", "n", "1", "2", "3", "4", "5", "6", "7", "8", "9", ",", "\\"] + +// Shifted chords with the platform's modifier that belong to the app shell rather than the page: reopening a tab, +// toggling the side panel, opening the command palette, or running session commands. New Window and Close Window +// (⇧⌘N, ⇧⌘W) stay unclaimed: only their menu accelerators handle them, and claiming a chord also blocks the menu. +const shellShiftKeys = ["t", "r", "p", "l", "e", "o", "s"] + /** * The letter a page key press names, as the app's keybinds read it: the layout's character, so Dvorak's C is "c" * although it sits on the physical I key. A layout that types no Latin letter falls back to the physical key. @@ -9,3 +30,33 @@ export function pageLetter(input: Pick) { return input.code.slice(3).toLowerCase() } + +/** + * Returns the shortcut to forward to the app window when a focused page receives a key press that belongs to the + * browser pane or app shell, or `undefined` when the key belongs to the page (typing, editing, clipboard, find, + * reload, history, or in-page shortcuts like ⌘K and ⌘B). + */ +export function pageShortcut(input: PageKey, mac: boolean): PageShortcut | undefined { + const key = pageLetter(input) + const mod = mac ? input.meta && !input.control : input.control && !input.meta + + const matched = + (input.control && !input.meta && !input.alt && (key === "tab" || (key === "`" && !input.shift))) || + (mod && + (input.alt + ? !input.shift && (key === "arrowleft" || key === "arrowright") + : input.shift + ? shellShiftKeys.includes(key) + : shellKeys.includes(key))) + + if (!matched) return undefined + + return { + key: /^[a-z]$/.test(key) ? key : input.key, + code: input.code, + ctrl: input.control, + meta: input.meta, + shift: input.shift, + alt: input.alt, + } +} diff --git a/packages/gui-extensions/src/browser/model.ts b/packages/gui-extensions/src/browser/model.ts index 4c024c22371..20440903093 100644 --- a/packages/gui-extensions/src/browser/model.ts +++ b/packages/gui-extensions/src/browser/model.ts @@ -10,6 +10,7 @@ import { type Connection, type InspectEvent, type Registration, + type ShortcutEvent, type Zoom, } from "./connection" import { recordable, remember, withIcon } from "./history" @@ -64,6 +65,8 @@ type PaneHandle = { inspect: () => void /** Focuses the address field and selects its text. */ focusAddress: () => void + /** Runs a key press the page handed to the app, as if it was pressed in this pane. */ + press: (event: ShortcutEvent) => void } export type Model = ReturnType @@ -214,6 +217,10 @@ export function createModel(ctx: SetupContext) { if (visits) history.set({ visits }) }, address: (tabID) => addressed.get(id)?.forEach((listener) => listener(tabID)), + // Only the pane on screen holds the page that had focus. + shortcut: (event) => { + if (sessions.current()?.key === id) panes()[0]?.press(event) + }, change: (next, mirror, native) => { if (next.error === "browser.pane.unsupported") { setState("unsupported", ref.server.id, true) diff --git a/packages/gui-extensions/src/browser/pane.ts b/packages/gui-extensions/src/browser/pane.ts index 91de480703e..078f78e5643 100644 --- a/packages/gui-extensions/src/browser/pane.ts +++ b/packages/gui-extensions/src/browser/pane.ts @@ -506,6 +506,13 @@ export function createBrowserPane(input: { entry.win.webContents.focus() publish(entry, { type: "address", tabID: id }) }, + shortcut: (shortcut) => { + if (!entry.pages.has(id) || entry.focusedTabID !== id || entry.win.isDestroyed()) return + + // As for the address shortcut: the command runs in the app, which may move focus there. + if (entry.win.isFocused()) entry.win.webContents.focus() + publish(entry, { type: "shortcut", ...shortcut }) + }, popup: (popupOptions, background) => { const popup = create(entry, false, popupOptions) diff --git a/packages/gui-extensions/src/browser/panel.fixture.tsx b/packages/gui-extensions/src/browser/panel.fixture.tsx index c395826c827..dc98fb5239c 100644 --- a/packages/gui-extensions/src/browser/panel.fixture.tsx +++ b/packages/gui-extensions/src/browser/panel.fixture.tsx @@ -49,6 +49,7 @@ import type definition from "./index" import { createModel } from "./model" import SessionBrowserPane from "./panel" import { BrowserPane, type PaneEvent } from "./ipc" +import { pageShortcut } from "./keys" import { refusal } from "./policy" /** The renderer host pieces the pane runs on, passed in by `packages/app/component-tests/browser-pane.spec.ts`. */ @@ -602,7 +603,7 @@ type RegionHost = { /** Real shell command providers for shortcut cases. */ Shell: ParentComponent /** Shell fallback and the real extension command adapter. */ - Commands: Component<{ close: () => void }> + Commands: Component<{ close: () => void; review: () => void }> /** The real host wrapper that renders an extension in its context. */ Contribution: Component<{ extension: string; children: () => JSX.Element }> } @@ -616,6 +617,8 @@ type RegionFixtureState = { away: boolean /** Whether the fixture's shell session was closed by its fallback shortcut. */ closed: boolean + /** How many times the review side panel toggled from a shortcut. */ + reviews: number /** The page main reports, blank or loaded. */ blank: boolean /** Native browser tab close requests, by tab id. */ @@ -647,6 +650,7 @@ export function mountBrowserRegion(input: RegionHost) { registrations: [], away: false, closed: false, + reviews: 0, blank: false, closes: [], inspects: [], @@ -880,6 +884,39 @@ export function mountBrowserRegion(input: RegionHost) { ) } + // Wires an embedded page with the same before-input-event shortcut interception as chromium.ts. + const wirePage = (frame: HTMLIFrameElement) => { + const win = frame.contentWindow + + if (!win) return + const mac = /(Mac|iPod|iPhone|iPad)/.test(navigator.platform) + + win.addEventListener( + "keydown", + (event) => { + const shortcut = pageShortcut( + { + key: event.key, + code: event.code, + control: event.ctrlKey, + meta: event.metaKey, + shift: event.shiftKey, + alt: event.altKey, + }, + mac, + ) + + const binding = latest()?.binding + + if (!shortcut || !binding) return + event.preventDefault() + event.stopImmediatePropagation() + listeners.forEach((listener) => listener({ binding, event: { type: "shortcut", ...shortcut } })) + }, + true, + ) + } + // Mounts once every extension is active, as the session screen does. function Strip() { const region = input.createRegion({ @@ -974,7 +1011,10 @@ export function mountBrowserRegion(input: RegionHost) { }} ipc={(token) => (token.id === BrowserPane.id && !store.away ? pane : undefined)} > - setStore("closed", true)} /> + setStore("closed", true)} + review={() => setStore("reviews", (count) => count + 1)} + />

Restored side strip

+