diff --git a/apps/desktop/package.json b/apps/desktop/package.json index ddef3ca9eb147eb6aca405378cd26cd8b70e244e..15bbbda6c6dfd79f781a8ac4d9a3f65524081742 100644 --- a/apps/desktop/package.json +++ b/apps/desktop/package.json @@ -24,7 +24,7 @@ "@t3tools/tailscale": "workspace:*", "dbus-next": "0.10.2", "effect": "catalog:", - "electron": "44.4.2", + "electron": "44.4.5", "electron-store": "^8.2.0", "electron-updater": "^6.8.9", "ffi-rs": "1.3.2", diff --git a/apps/desktop/src/app/DesktopLifecycle.test.ts b/apps/desktop/src/app/DesktopLifecycle.test.ts index 98e9cf3646904e026a648f903d09b1c8653e738a..5caa18a53aabf9a19e778e50ff7254609312cee1 100644 --- a/apps/desktop/src/app/DesktopLifecycle.test.ts +++ b/apps/desktop/src/app/DesktopLifecycle.test.ts @@ -96,6 +96,7 @@ function layerDesktopWindow( dispatchMenuAction: () => Effect.void, dispatchSnapShotEvent: () => Effect.void, zoomMain: () => Effect.void, + runMainContentsCommand: () => Effect.void, syncAppearance: Effect.void, }); } diff --git a/apps/desktop/src/backend/DesktopBackendPool.test.ts b/apps/desktop/src/backend/DesktopBackendPool.test.ts index 44ede122474d99d4ca0cf26ef8168e18bdf519d6..d8d7813e79721d981ae119d5128f769a484c2232 100644 --- a/apps/desktop/src/backend/DesktopBackendPool.test.ts +++ b/apps/desktop/src/backend/DesktopBackendPool.test.ts @@ -100,6 +100,7 @@ function layerPool(labelRef: Ref.Ref): Layer.Layer Effect.die("unexpected menu action"), dispatchSnapShotEvent: () => Effect.void, zoomMain: () => Effect.die("unexpected zoom"), + runMainContentsCommand: () => Effect.die("unexpected main contents command"), syncAppearance: Effect.void, } satisfies DesktopWindow.DesktopWindow["Service"]), ), diff --git a/apps/desktop/src/electron/ElectronDialog.ts b/apps/desktop/src/electron/ElectronDialog.ts index 18280333754c40d839ddd6ae4408a9ff105824a1..b48953f117d38cc572130a034698dc6d7c48dd84 100644 --- a/apps/desktop/src/electron/ElectronDialog.ts +++ b/apps/desktop/src/electron/ElectronDialog.ts @@ -95,8 +95,10 @@ export class ElectronDialog extends Context.Service< readonly pickFiles: ( input: ElectronDialogPickFilesInput, ) => Effect.Effect; + /** Pass `owner` to attach the box to that window so it cannot fall behind it. */ readonly showMessageBox: ( options: Electron.MessageBoxOptions, + owner?: Electron.BrowserWindow, ) => Effect.Effect; readonly showErrorBox: (title: string, content: string) => Effect.Effect; } @@ -164,9 +166,12 @@ export const make = ElectronDialog.of({ }); return result.canceled ? [] : result.filePaths; }), - showMessageBox: (options) => + showMessageBox: (options, owner) => Effect.tryPromise({ - try: () => Electron.dialog.showMessageBox(options), + try: () => + owner === undefined + ? Electron.dialog.showMessageBox(options) + : Electron.dialog.showMessageBox(owner, options), catch: (cause) => new ElectronDialogShowMessageBoxError({ type: options.type ?? null, diff --git a/apps/desktop/src/ipc/channels.ts b/apps/desktop/src/ipc/channels.ts index 5b078314ac49a2545a4b466301e32fc4981e2f75..e6b936c100ba0dd19b0266475bb58bc86bce8a21 100644 --- a/apps/desktop/src/ipc/channels.ts +++ b/apps/desktop/src/ipc/channels.ts @@ -109,6 +109,7 @@ export const PREVIEW_RECORDING_SAVE_CHANNEL = "desktop:preview-recording-save"; export const PREVIEW_RECORDING_FRAME_CHANNEL = "desktop:preview-recording-frame"; export const PREVIEW_STATE_CHANGE_CHANNEL = "desktop:preview-state-change"; export const PREVIEW_POINTER_EVENT_CHANNEL = "desktop:preview-pointer-event"; +export const PREVIEW_OPEN_LINK_CHANNEL = "desktop:preview-open-link"; export const MAC_PERMISSION_HELPER_CHANNEL = "desktop:mac-permission-helper"; diff --git a/apps/desktop/src/ipc/methods/preview.ts b/apps/desktop/src/ipc/methods/preview.ts index 4afc61d6f76f5b611477871be233b7cb53f2166f..94c56464f7c476f9d640d9277dd862470848015c 100644 --- a/apps/desktop/src/ipc/methods/preview.ts +++ b/apps/desktop/src/ipc/methods/preview.ts @@ -54,6 +54,9 @@ export const installPreviewEventForwarding = Effect.fn( yield* manager.subscribePointerEvents((event) => electronWindow.sendAll(IpcChannels.PREVIEW_POINTER_EVENT_CHANNEL, event), ); + yield* manager.subscribeOpenLinks((event) => + electronWindow.sendAll(IpcChannels.PREVIEW_OPEN_LINK_CHANNEL, event), + ); }); export const setForwardedShortcuts = DesktopIpc.makeIpcMethod({ diff --git a/apps/desktop/src/preload.ts b/apps/desktop/src/preload.ts index 0dd9604f98651d5d978bc418292d9749bc0326ee..12b68f186089ccfbd2ee6956ec7c7dbe96c02459 100644 --- a/apps/desktop/src/preload.ts +++ b/apps/desktop/src/preload.ts @@ -1,5 +1,6 @@ import type { DesktopBridge, + DesktopPreviewOpenLinkEvent, DesktopPreviewPointerEvent, DesktopPreviewRecordingInputEvent, DesktopPreviewRecordingFrame, @@ -416,5 +417,14 @@ contextBridge.exposeInMainWorld("desktopBridge", { return () => ipcRenderer.removeListener(IpcChannels.PREVIEW_POINTER_EVENT_CHANNEL, wrappedListener); }, + onOpenLink: (listener) => { + const wrappedListener = (_event: Electron.IpcRendererEvent, linkEvent: unknown) => { + if (typeof linkEvent !== "object" || linkEvent === null) return; + listener(linkEvent as DesktopPreviewOpenLinkEvent); + }; + ipcRenderer.on(IpcChannels.PREVIEW_OPEN_LINK_CHANNEL, wrappedListener); + return () => + ipcRenderer.removeListener(IpcChannels.PREVIEW_OPEN_LINK_CHANNEL, wrappedListener); + }, }, } satisfies DesktopBridge); diff --git a/apps/desktop/src/preview/BrowserSession.test.ts b/apps/desktop/src/preview/BrowserSession.test.ts index 805c684dfbecd04003e53f6ba7ddd5d366caac29..9799d88fdc9cab22f0bfcd08f9fc32ad5c2edc60 100644 --- a/apps/desktop/src/preview/BrowserSession.test.ts +++ b/apps/desktop/src/preview/BrowserSession.test.ts @@ -21,15 +21,35 @@ const { fromPartition, sessions } = vi.hoisted(() => ({ >(), })); +const { fromWebContents } = vi.hoisted(() => ({ + fromWebContents: vi.fn<(webContents: unknown) => unknown>(() => null), +})); + vi.mock("electron", () => ({ + BrowserWindow: { fromWebContents }, session: { fromPartition, }, })); +import * as ElectronDialog from "../electron/ElectronDialog.ts"; import * as BrowserSession from "./BrowserSession.ts"; -const layer = BrowserSession.layer.pipe(Layer.provide(NodeServices.layer)); +const { showMessageBox } = vi.hoisted(() => ({ + showMessageBox: vi.fn< + (options: Electron.MessageBoxOptions, owner?: Electron.BrowserWindow) => number + >(() => 1), +})); +const layerDialog = Layer.succeed(ElectronDialog.ElectronDialog, { + pickFolder: () => Effect.die("unused"), + pickFiles: () => Effect.die("unused"), + showMessageBox: (options, owner) => + Effect.sync(() => ({ response: showMessageBox(options, owner), checkboxChecked: false })), + showErrorBox: () => Effect.void, +}); +const layer = BrowserSession.layer.pipe( + Layer.provide(Layer.merge(NodeServices.layer, layerDialog)), +); describe("BrowserSession", () => { beforeEach(() => { @@ -168,6 +188,7 @@ describe("BrowserSession", () => { "clipboard-sanitized-write", "notifications", "geolocation", + "fullscreen", ]) { assert.isTrue(requestAllows(permission), `request handler should allow ${permission}`); assert.isTrue( @@ -189,6 +210,73 @@ describe("BrowserSession", () => { }).pipe(Effect.provide(layer)), ); + it.effect("opens custom-scheme links externally only after the user confirms", () => + Effect.gen(function* () { + const browserSessions = yield* BrowserSession.BrowserSession; + const partition = yield* browserSessions.getPartition("scope-a"); + yield* browserSessions.getSession("scope-a"); + const requestHandler = + sessions.get(partition)?.setPermissionRequestHandler.mock.calls[0]?.[0]; + assert.isFunction(requestHandler); + + const request = (externalURL: string) => + Effect.callback((resume) => { + requestHandler( + null, + "openExternal", + (granted: boolean) => resume(Effect.succeed(granted)), + { externalURL }, + ); + }); + + showMessageBox.mockReset(); + showMessageBox.mockReturnValueOnce(0).mockReturnValueOnce(1); + assert.isTrue(yield* request("slack://open?team=T1")); + assert.isFalse(yield* request("zoommtg://zoom.us/join")); + assert.strictEqual(showMessageBox.mock.calls.length, 2); + + for (const url of ["file:///etc/passwd", "javascript:alert(1)", "data:text/html,x"]) { + assert.isFalse(yield* request(url), `${url} must never open externally`); + } + assert.strictEqual(showMessageBox.mock.calls.length, 2); + }).pipe(Effect.provide(layer)), + ); + + // A prompt without an owner can open behind the app window, leaving the one + // prompt slot taken and every later link silently denied. + it.effect("attaches the open-externally prompt to the window hosting the preview", () => + Effect.gen(function* () { + const browserSessions = yield* BrowserSession.BrowserSession; + const partition = yield* browserSessions.getPartition("scope-a"); + yield* browserSessions.getSession("scope-a"); + const requestHandler = + sessions.get(partition)?.setPermissionRequestHandler.mock.calls[0]?.[0]; + assert.isFunction(requestHandler); + + const hostWindow = { id: 7 } as unknown as Electron.BrowserWindow; + const host = { isDestroyed: () => false }; + const guest = { isDestroyed: () => false, hostWebContents: host }; + fromWebContents.mockImplementation((webContents) => + webContents === host ? hostWindow : null, + ); + showMessageBox.mockReset(); + showMessageBox.mockReturnValueOnce(1); + + yield* Effect.callback((resume) => { + requestHandler( + guest, + "openExternal", + (granted: boolean) => resume(Effect.succeed(granted)), + { + externalURL: "slack://open?team=T1", + }, + ); + }); + + assert.strictEqual(showMessageBox.mock.calls[0]?.[1], hostWindow); + }).pipe(Effect.provide(layer)), + ); + it.effect("preserves partition scope and the platform failure chain", () => { const nativeCause = new Error("native digest failed"); const platformCause = PlatformError.systemError({ @@ -218,7 +306,11 @@ describe("BrowserSession", () => { "Failed to derive a desktop preview browser partition for scope environment-a.", ); assert.notInclude(error.message, nativeCause.message); - }).pipe(Effect.provide(BrowserSession.layer.pipe(Layer.provide(layerFailingCrypto)))); + }).pipe( + Effect.provide( + BrowserSession.layer.pipe(Layer.provide(Layer.merge(layerFailingCrypto, layerDialog))), + ), + ); }); it.effect("preserves session scope, partition, and the Electron failure", () => diff --git a/apps/desktop/src/preview/BrowserSession.ts b/apps/desktop/src/preview/BrowserSession.ts index 9681be83fdbbc52cadba6979caebe60f62bae42c..10e2a2575f387ccdecf85d3eaee1b79967dce575 100644 --- a/apps/desktop/src/preview/BrowserSession.ts +++ b/apps/desktop/src/preview/BrowserSession.ts @@ -1,5 +1,5 @@ -import type { Session } from "electron"; -import { session } from "electron"; +import type { Session, WebContents } from "electron"; +import { BrowserWindow, session } from "electron"; import * as Context from "effect/Context"; import * as Crypto from "effect/Crypto"; import * as Effect from "effect/Effect"; @@ -9,6 +9,8 @@ import * as PlatformError from "effect/PlatformError"; import * as Schema from "effect/Schema"; import * as SynchronizedRef from "effect/SynchronizedRef"; +import * as ElectronDialog from "../electron/ElectronDialog.ts"; + const PREVIEW_PARTITION_PREFIX = "persist:t3code-preview-"; /** * Incognito partitions deliberately omit the `persist:` prefix, which is what @@ -33,12 +35,51 @@ const ALLOWED_PREVIEW_PERMISSIONS: ReadonlySet = new Set([ "clipboard-sanitized-write", "notifications", "geolocation", + // The Fullscreen API. `DesktopWindow` disables HTML fullscreen window resizing + // for the app window and every preview guest, so a page that goes fullscreen + // fills its own webview instead of taking over the whole T3 window. + "fullscreen", // Deliberately NOT local-fonts: preview sessions run untrusted web content, // and silently granting it would hand every page the user's installed-font // fingerprint (and font file bytes via FontData.blob()). The app's own font // picker runs in the main window session, which is unaffected by this list. ]); +/** + * Schemes a preview page may never hand to the OS, even with the user's + * consent: they read local files, run script, or are web pages that belong in + * the preview itself. + */ +const NEVER_EXTERNAL_PROTOCOLS: ReadonlySet = new Set([ + "about:", + "blob:", + "chrome:", + "data:", + "devtools:", + "file:", + "filesystem:", + "http:", + "https:", + "javascript:", + "view-source:", +]); + +/** + * The URL to offer the user when a preview page navigates to a custom scheme + * such as `slack://` or `zoom://`, or `null` when it must stay denied. + */ +const externalProtocolPromptUrl = (rawUrl: string | undefined): string | null => { + if (!rawUrl) return null; + try { + const url = new URL(rawUrl); + return NEVER_EXTERNAL_PROTOCOLS.has(url.protocol) ? null : url.href; + } catch { + return null; + } +}; + +const MAX_PROMPT_URL_LENGTH = 300; + export class BrowserSessionPartitionDerivationError extends Schema.TaggedError()( "BrowserSessionPartitionDerivationError", { @@ -159,9 +200,68 @@ const encodeScopeForDigest = (scope: string): Uint8Array => ), ); +/** + * The window showing a preview guest. A `` guest has no window of its + * own, so look it up through the page that embeds it. + */ +const previewHostWindow = (guest: WebContents | null): BrowserWindow | undefined => { + if (guest === null || guest.isDestroyed()) return undefined; + const host = guest.hostWebContents ?? guest; + if (host.isDestroyed()) return undefined; + return BrowserWindow.fromWebContents(host) ?? undefined; +}; + /** @public Service construction is part of the canonical Effect module API. */ export const make = Effect.gen(function* BrowserSessionMake() { const crypto = yield* Crypto.Crypto; + const electronDialog = yield* ElectronDialog.ElectronDialog; + const runFork = Effect.runForkWith(yield* Effect.context()); + // One prompt at a time, so a page cannot stack dialogs by looping a deep link. + let externalPromptOpen = false; + /** + * Electron grants an external-protocol navigation through the `openExternal` + * permission and then launches the OS handler itself. Ask first: previews run + * untrusted pages, and a silent grant would let any of them launch apps. + */ + const confirmOpenExternal = ( + requester: WebContents | null, + rawUrl: string | undefined, + callback: (granted: boolean) => void, + ): void => { + const url = externalProtocolPromptUrl(rawUrl); + if (url === null || externalPromptOpen) { + callback(false); + return; + } + externalPromptOpen = true; + const settle = (granted: boolean) => + Effect.sync(() => { + externalPromptOpen = false; + callback(granted); + }); + const shownUrl = + url.length > MAX_PROMPT_URL_LENGTH ? `${url.slice(0, MAX_PROMPT_URL_LENGTH)}…` : url; + runFork( + electronDialog + .showMessageBox( + { + type: "question", + buttons: ["Open", "Cancel"], + defaultId: 0, + cancelId: 1, + message: `Open this ${new URL(url).protocol.slice(0, -1)} link?`, + detail: `A page in the browser wants to open another application:\n\n${shownUrl}`, + }, + previewHostWindow(requester), + ) + .pipe( + Effect.map(({ response }) => response === 0), + Effect.orElseSucceed(() => false), + Effect.flatMap(settle), + Effect.onInterrupt(() => settle(false)), + ), + ); + }; const sessionsRef = yield* SynchronizedRef.make>(new Map()); const getPartition = Effect.fn("BrowserSession.getPartition")(function* ( @@ -203,9 +303,19 @@ export const make = Effect.gen(function* BrowserSessionMake() { // the challenge every few seconds, so logins behind it never complete // (#5002). Re-setting the unchanged native string is harmless, so it // is the rewritten string itself that trips the check. - browserSession.setPermissionRequestHandler((_webContents, permission, callback) => { - callback(ALLOWED_PREVIEW_PERMISSIONS.has(permission)); - }); + browserSession.setPermissionRequestHandler( + (webContents, permission, callback, details) => { + if (permission === "openExternal") { + confirmOpenExternal( + webContents, + "externalURL" in details ? details.externalURL : undefined, + callback, + ); + return; + } + callback(ALLOWED_PREVIEW_PERMISSIONS.has(permission)); + }, + ); browserSession.setPermissionCheckHandler((_webContents, permission) => ALLOWED_PREVIEW_PERMISSIONS.has(permission), ); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.test.ts b/apps/desktop/src/preview/DesktopBrowserHost.test.ts index eff9b98b105e3413c087135ca51d8f405c031fe1..81ea040701f0ed160803a318a6f31d128c5fa938 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.test.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.test.ts @@ -119,4 +119,33 @@ describe("DesktopBrowserHost", () => { expect(paths).toEqual(["/srv/downloads/guid-1"]); }), ); + + it.effect("tells a download the person clicked from one the agent's input started", () => + Effect.gen(function* () { + const host = yield* DesktopBrowserHost.make; + const debuggee = makeDebuggee(); + host.attach(key, debuggee.tab); + // Reading the page is not acting on it. + yield* host.handleCommandLine( + encodeJson({ + type: "cdp", + ...key, + message: encodeJson({ id: 1, method: "DOM.getDocument", params: {} }), + }), + ); + expect(host.humanStartedDownload(debuggee.tab.webContents)).toBe(true); + yield* host.handleCommandLine( + encodeJson({ + type: "cdp", + ...key, + message: encodeJson({ + id: 2, + method: "Input.dispatchMouseEvent", + params: { type: "mousePressed", x: 1, y: 1 }, + }), + }), + ); + expect(host.humanStartedDownload(debuggee.tab.webContents)).toBe(false); + }), + ); }); diff --git a/apps/desktop/src/preview/DesktopBrowserHost.ts b/apps/desktop/src/preview/DesktopBrowserHost.ts index e6349d06b916a5fccf339c59d8f10a7f3a0831e2..cfb1ff753c47033e0f2ce74a11e3afe2e3023e46 100644 --- a/apps/desktop/src/preview/DesktopBrowserHost.ts +++ b/apps/desktop/src/preview/DesktopBrowserHost.ts @@ -28,6 +28,11 @@ import { createCdpRelayConnection, type CdpRelayConnection } from "./CdpRelay.ts const encodeEvent = Schema.encodeSync(Schema.fromJsonString(DesktopBrowserEvent)); const decodeCommand = Schema.decodeUnknownOption(Schema.fromJsonString(DesktopBrowserCommand)); const lineEncoder = new TextEncoder(); +/** CDP commands through which an agent clicks, types, or navigates the page. */ +const AGENT_INPUT_COMMAND = + /"method":"(?:Input\.|Page\.navigate"|Runtime\.(?:evaluate|callFunctionOn)")/; +/** How long after an agent's input a download still counts as the agent's. */ +const AGENT_DOWNLOAD_WINDOW_MS = 5_000; export interface DesktopBrowserTabKey { readonly threadId: string; @@ -50,6 +55,8 @@ interface AttachedTab { downloadDirectory: string | null; /** The guid CDP gave the download that is about to start. */ pendingDownloadGuid: string | null; + /** When the server last sent this page input, which only an agent does. */ + agentInputAt: number; readonly onMessage: ( event: Electron.Event, method: string, @@ -72,6 +79,11 @@ export class DesktopBrowserHost extends Context.Service< readonly attach: (key: DesktopBrowserTabKey, debuggee: DesktopBrowserTabDebugger) => void; /** Withdraws it: closed, swapped, crashed, or devtools needs the debugger. */ readonly detach: (key: DesktopBrowserTabKey) => void; + /** + * Whether a server tab's download came from the person at this desktop: + * the server sent the page no input just before it started. + */ + readonly humanStartedDownload: (source: Electron.WebContents) => boolean; /** Points a server tab's download at the server; false for any other download. */ readonly placeDownload: (source: Electron.WebContents, item: Electron.DownloadItem) => boolean; /** The agent's cursor positions for attached tabs, keyed by their server tab. */ @@ -161,6 +173,7 @@ export const make = Effect.gen(function* () { relay: null, downloadDirectory: null, pendingDownloadGuid: null, + agentInputAt: Number.NEGATIVE_INFINITY, onMessage: (_event, method, params, sessionId) => { if (method === "Browser.downloadWillBegin") { const guid = (params as { guid?: unknown } | undefined)?.guid; @@ -190,9 +203,15 @@ export const make = Effect.gen(function* () { tab.relay = null; return; } + if (AGENT_INPUT_COMMAND.test(command.value.message)) tab.agentInputAt = performance.now(); relayFor(tab).receive(command.value.message); }); + const humanStartedDownload = (source: Electron.WebContents) => { + const tab = [...tabs.values()].find((candidate) => candidate.debuggee.webContents === source); + return tab !== undefined && performance.now() - tab.agentInputAt > AGENT_DOWNLOAD_WINDOW_MS; + }; + // Read when a backend starts, not when the host is built. const announceAll = Effect.suspend(() => Effect.forEach( @@ -219,6 +238,7 @@ export const make = Effect.gen(function* () { attach, detach, placeDownload, + humanStartedDownload, }); }); diff --git a/apps/desktop/src/preview/Manager.test.ts b/apps/desktop/src/preview/Manager.test.ts index 11ee2d83e481c31859389b8ca82abf412151f597..84c6273aad6230bae2b2db7425dc3ab1f6d14945 100644 --- a/apps/desktop/src/preview/Manager.test.ts +++ b/apps/desktop/src/preview/Manager.test.ts @@ -158,21 +158,31 @@ describe("previewWindowOpenAction", () => { ).toBe("popup"); }); - it("keeps target=_blank links in the preview tab", () => { + it("opens target=_blank links as a new tab", () => { expect(PreviewManager.previewWindowOpenAction(details({ disposition: "foreground-tab" }))).toBe( - "navigate", + "new-tab", ); expect(PreviewManager.previewWindowOpenAction(details({ disposition: "background-tab" }))).toBe( - "navigate", + "new-tab", ); }); + it("keeps a form POST with a body on the in-place path", () => { + // A new tab can only reopen the URL as a GET, which would drop the body. + const postBody = { data: [], contentType: "application/x-www-form-urlencoded" }; + expect( + PreviewManager.previewWindowOpenAction({ + ...details({ disposition: "foreground-tab" }), + postBody, + }), + ).toBe("navigate"); + }); + it("does not hand a window to schemes that cannot be hardened", () => { // A popup skips the `will-attach-webview` hardening, so it only gets a window // when its preferences can be overridden. Chromium copies the guest's // preferences for `about:blank` and forbids overriding them. for (const url of [ - "about:blank", "javascript:alert(1)", "file:///etc/passwd", "vscode://vscode-remote/ssh-remote+box/tmp", @@ -181,6 +191,16 @@ describe("previewWindowOpenAction", () => { expect(PreviewManager.previewWindowOpenAction(details({ url }))).toBe("navigate"); } }); + + it("denies a blank window instead of blanking the opener", () => { + // MSAL opens `about:blank` first and falls back to a redirect on `null`; + // loading the blank page into the tab would destroy that fallback. + for (const url of ["about:blank", ""]) { + for (const disposition of ["new-window", "foreground-tab"] as const) { + expect(PreviewManager.previewWindowOpenAction(details({ url, disposition }))).toBe("deny"); + } + } + }); }); const { @@ -355,7 +375,7 @@ const makeTestPreviewWebContents = ( getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -460,7 +480,7 @@ const makeFaviconWebContents = (options?: { getType: () => "webview", getURL: () => currentUrl, getTitle: () => "Preview", - isLoading: () => loading, + isLoadingMainFrame: () => loading, isDevToolsOpened: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), @@ -572,6 +592,40 @@ describe("PreviewManager", () => { webviewSend.mockClear(); }); + effectIt.effect("opens a target=_blank link as a new tab without navigating the opener", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + fromId.mockReturnValue(preview.webContents); + const opened: Array<{ tabId: string; url: string; background: boolean }> = []; + yield* manager.subscribeOpenLinks((event) => Effect.sync(() => void opened.push(event))); + yield* manager.createTab("tab_links"); + yield* manager.registerWebview("tab_links", 42); + const openHandler = (preview.webContents as Electron.WebContents).setWindowOpenHandler as + | ReturnType + | undefined; + const handler = openHandler?.mock.lastCall?.[0] as ( + details: Partial, + ) => { action: string }; + + expect( + handler({ url: "https://github.com/issues/1", disposition: "foreground-tab" }), + ).toEqual({ action: "deny" }); + expect(handler({ url: "about:blank", disposition: "new-window" })).toEqual({ + action: "deny", + }); + handler({ url: "https://github.com/issues/2", disposition: "background-tab" }); + yield* settle(() => opened.length === 2); + + expect(preview.loadURL).not.toHaveBeenCalled(); + expect(opened).toEqual([ + { tabId: "tab_links", url: "https://github.com/issues/1", background: false }, + { tabId: "tab_links", url: "https://github.com/issues/2", background: true }, + ]); + }), + ), + ); + effectIt.effect("keeps preview shortcuts out of the host window", () => withManager((manager) => Effect.gen(function* () { @@ -885,7 +939,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "about:blank", getTitle: () => "", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -978,7 +1032,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "http://localhost:3200/", getTitle: () => "Preview", - isLoading: () => false, + isLoadingMainFrame: () => false, isDevToolsOpened: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), @@ -1481,7 +1535,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => url, getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => { if (!zoomReadable) throw new Error("zoom unavailable"); return effectiveZoom; @@ -1551,7 +1605,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => url, getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: replacementSetZoomFactor, setAudioMuted: vi.fn(), @@ -1592,7 +1646,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor, setAudioMuted: vi.fn(), @@ -1639,7 +1693,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor, setAudioMuted: vi.fn(), @@ -1695,7 +1749,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -1790,7 +1844,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "http://localhost:5173/README.md", getTitle: () => "README.md", - isLoading: () => true, + isLoadingMainFrame: () => true, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -1915,7 +1969,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted, @@ -2258,7 +2312,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => url, getTitle: () => "localhost:5733", - isLoading: () => loading, + isLoadingMainFrame: () => loading, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -2339,6 +2393,28 @@ describe("PreviewManager", () => { ), ); + effectIt.effect("ignores a late cross-origin iframe load once the page has loaded", () => + withManager((manager) => + Effect.gen(function* () { + const preview = makeFaviconWebContents(); + // The frame tree is still loading the iframe; the main frame is done. + Object.assign(preview.webContents, { isLoading: () => true }); + fromId.mockReturnValue(preview.webContents); + const states: PreviewManager.PreviewTabState[] = []; + yield* manager.subscribeStateChanges((_tabId, state) => + Effect.sync(() => void states.push(state)), + ); + yield* manager.createTab("tab_iframe"); + yield* manager.registerWebview("tab_iframe", 42); + + preview.listeners.get("did-start-loading")?.(); + preview.listeners.get("did-stop-loading")?.(); + yield* settle(() => states.length > 2); + expect(states.at(-1)?.navStatus.kind).toBe("Success"); + }), + ), + ); + effectIt.effect("captures a PNG screenshot into browser artifacts", () => withManager((manager) => Effect.gen(function* () { @@ -2351,7 +2427,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com:8443/path?query=value", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -2919,7 +2995,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => `https://example.com/${id}`, getTitle: () => `Example ${id}`, - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -3381,7 +3457,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), @@ -3880,7 +3956,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, isFocused: () => true, getZoomFactor: () => 1, setZoomFactor: vi.fn(), @@ -4026,7 +4102,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, isFocused: () => true, getZoomFactor: () => 1, setZoomFactor: vi.fn(), @@ -4095,6 +4171,69 @@ describe("PreviewManager", () => { ), ); + effectIt.effect("crops the picked area out of a full-page capture at device scale", () => + withManager((manager) => + Effect.gen(function* () { + let onPicked: ((event: unknown, ...args: unknown[]) => void) | undefined; + const crop = vi.fn((rect: Electron.Rectangle) => ({ + getSize: () => ({ width: rect.width, height: rect.height }), + toDataURL: () => "data:image/png;base64,cropped", + })); + const capturePage = vi.fn(async () => ({ + getSize: () => ({ width: 2000, height: 1200 }), + crop, + })); + fromId.mockReturnValue( + Object.assign(makeTestPreviewWebContents(capturePage as never), { + isFocused: () => true, + once: vi.fn(), + ipc: { + on: vi.fn((channel: string, listener: typeof onPicked) => { + if (channel === "preview:element-picked") onPicked = listener; + }), + off: vi.fn(), + removeListener: vi.fn(), + }, + }), + ); + + yield* manager.createTab("tab_1"); + yield* manager.registerWebview("tab_1", 42); + const pick = yield* manager.pickElement("tab_1").pipe(Effect.forkChild); + yield* Effect.yieldNow; + onPicked?.( + {}, + { + id: "annotation_1", + pageUrl: "https://example.com", + pageTitle: "Example", + comment: "Tighten this spacing", + elements: [], + regions: [{ id: "region_1", rect: { x: 5, y: 6, width: 20, height: 30 } }], + strokes: [], + styleChanges: [], + screenshot: null, + createdAt: "2026-06-11T00:00:00.000Z", + }, + { x: 5, y: 6, width: 20, height: 30 }, + "attach", + 2, + ); + + const result = yield* Fiber.join(pick); + expect(capturePage).toHaveBeenCalledWith(); + expect(crop).toHaveBeenCalledWith({ x: 10, y: 12, width: 40, height: 60 }); + expect(result?.screenshotFailed).toBeUndefined(); + expect(result?.annotation.screenshot).toEqual({ + dataUrl: "data:image/png;base64,cropped", + width: 40, + height: 60, + cropRect: { x: 5, y: 6, width: 20, height: 30 }, + }); + }), + ), + ); + effectIt.effect("a stale capture from a replaced pick never touches the next pick", () => withManager((manager) => Effect.gen(function* () { @@ -4105,7 +4244,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, isFocused: () => true, getZoomFactor: () => 1, setZoomFactor: vi.fn(), @@ -4190,7 +4329,7 @@ describe("PreviewManager", () => { getType: () => "webview", getURL: () => "https://example.com", getTitle: () => "Example", - isLoading: () => false, + isLoadingMainFrame: () => false, getZoomFactor: () => 1, setZoomFactor: vi.fn(), setAudioMuted: vi.fn(), diff --git a/apps/desktop/src/preview/Manager.ts b/apps/desktop/src/preview/Manager.ts index f98694145b67b762be4eda7687af3f32f01563c3..123943f70f9a3df5ce3f3ead74033b85abb1713f 100644 --- a/apps/desktop/src/preview/Manager.ts +++ b/apps/desktop/src/preview/Manager.ts @@ -13,6 +13,7 @@ import type { DesktopPreviewAnnotationTheme, DesktopPreviewColorScheme, DesktopPreviewFavicon, + DesktopPreviewOpenLinkEvent, DesktopPreviewPointerEvent, PreviewAnnotationPayload, PreviewAnnotationRect, @@ -30,6 +31,7 @@ import { normalizePreviewUrl } from "@t3tools/shared/preview"; import { BrowserWindow, ClipboardItem, + app, type Session, clipboard, nativeImage, @@ -284,61 +286,36 @@ const normalizeCaptureRect = (value: unknown): PreviewAnnotationRect | null => { }; }; -/** `capturePage` never settles when the guest's compositor is wedged. */ -const ANNOTATION_SCREENSHOT_TIMEOUT = "5 seconds"; - /** - * Crops the guest for a picked annotation. A stalled `capturePage` resolves to - * `null` after the timeout: the annotation is still sendable without its - * screenshot, and the pick session must settle either way. + * Crops a full-page capture to a picked annotation. `cropRect` is in CSS px; + * the capture's pixels are CSS px × the guest's `devicePixelRatio`. */ -const captureAnnotationScreenshot = ( - tabId: string, - wc: Electron.WebContents, +const cropAnnotationScreenshot = ( + image: Electron.NativeImage, cropRect: PreviewAnnotationRect | null, -): Effect.Effect => - Effect.tryPromise({ - // The unused abort signal is what makes this interruptible, and therefore - // what lets the timeout below fire. Drop the parameter and a stalled - // capture strands the pick session again. - try: (_signal) => - wc.capturePage( - cropRect - ? { - x: cropRect.x, - y: cropRect.y, - width: cropRect.width, - height: cropRect.height, - } - : undefined, - ), - catch: (cause) => - new PreviewOperationError({ - operation: "captureAnnotationScreenshot", - tabId, - webContentsId: wc.id, - cause, - }), - }).pipe( - Effect.map((image): PreviewAnnotationPayload["screenshot"] => { - const size = image.getSize(); - return { - dataUrl: image.toDataURL(), - width: size.width, - height: size.height, - cropRect: cropRect ?? { x: 0, y: 0, width: size.width, height: size.height }, - }; - }), - Effect.timeoutOption(ANNOTATION_SCREENSHOT_TIMEOUT), - Effect.flatMap((screenshot) => - Option.isSome(screenshot) - ? Effect.succeed(screenshot.value) - : Effect.logWarning("preview annotation screenshot timed out").pipe( - Effect.annotateLogs({ tabId, webContentsId: wc.id }), - Effect.as(null), - ), - ), - ); + devicePixelRatio: number, +): PreviewAnnotationPayload["screenshot"] => { + const full = image.getSize(); + if (!cropRect) { + return { + dataUrl: image.toDataURL(), + width: full.width, + height: full.height, + cropRect: { x: 0, y: 0, width: full.width, height: full.height }, + }; + } + const scale = Number.isFinite(devicePixelRatio) && devicePixelRatio > 0 ? devicePixelRatio : 1; + const x = Math.min(full.width - 1, Math.max(0, Math.floor(cropRect.x * scale))); + const y = Math.min(full.height - 1, Math.max(0, Math.floor(cropRect.y * scale))); + const cropped = image.crop({ + x, + y, + width: Math.max(1, Math.min(full.width - x, Math.ceil(cropRect.width * scale))), + height: Math.max(1, Math.min(full.height - y, Math.ceil(cropRect.height * scale))), + }); + const size = cropped.getSize(); + return { dataUrl: cropped.toDataURL(), width: size.width, height: size.height, cropRect }; +}; const findZoomStep = (current: number): number => { const index = ZOOM_LEVELS.findIndex( @@ -437,6 +414,8 @@ type RecordingInputListener = (event: DesktopPreviewRecordingInputEvent) => Effe type PointerEventListener = (event: DesktopPreviewPointerEvent) => Effect.Effect; +type OpenLinkListener = (event: DesktopPreviewOpenLinkEvent) => Effect.Effect; + /** * Protocols a preview page may open in a real popup window. * @@ -483,14 +462,29 @@ const POPUP_WINDOW_OPTIONS = { * navigating the preview tab instead destroys the opener the popup has to * `postMessage` its result back to. * - * `target="_blank"` links arrive as a tab disposition and keep loading in the - * preview tab, which is what people expect from a link inside a preview. + * `"new-tab"` hands a `target="_blank"` link (a tab disposition) to the web + * app, which opens it as another preview tab so the page that held the link + * stays put. Schemes a popup cannot be hardened for keep loading in place, and + * so does a form POST with a body, which a new tab could only reopen as a GET. + * + * `"deny"` is for a blank window (`""` or `about:blank`). It cannot get a real + * window (see `POPUP_PROTOCOLS`), and loading it into the preview tab would + * replace the opener with an empty page. Denying makes `window.open()` return + * `null`, which SDKs such as MSAL treat as a blocked popup and fall back from. */ export const previewWindowOpenAction = (details: { readonly url: string; readonly disposition: Electron.HandlerDetails["disposition"]; -}): "popup" | "navigate" => - details.disposition === "new-window" && isPopupUrl(details.url) ? "popup" : "navigate"; + readonly postBody?: Electron.PostBody; +}): "popup" | "new-tab" | "navigate" | "deny" => { + if (details.url === "" || details.url === "about:blank") return "deny"; + if (!isPopupUrl(details.url)) return "navigate"; + if (details.disposition === "new-window") return "popup"; + return !details.postBody && + (details.disposition === "foreground-tab" || details.disposition === "background-tab") + ? "new-tab" + : "navigate"; +}; export const isPreviewRefreshShortcut = (input: Electron.Input): boolean => input.type === "keyDown" && @@ -566,6 +560,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const attachedRef = yield* Ref.make>(new Map()); const listenersRef = yield* Ref.make>(new Set()); const pointerEventListenersRef = yield* Ref.make>(new Set()); + const openLinkListenersRef = yield* Ref.make>(new Set()); const recordingInputListenersRef = yield* Ref.make>( new Set(), ); @@ -836,7 +831,12 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function }); const deliverEvent = ( - eventKind: "state-change" | "recording-frame" | "recording-input" | "pointer-event", + eventKind: + | "state-change" + | "recording-frame" + | "recording-input" + | "pointer-event" + | "open-link", tabId: string, delivery: () => Effect.Effect, ) => @@ -1241,7 +1241,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const url = wc.getURL(); const title = wc.getTitle(); if (url === "" || url === "about:blank") return { kind: "Idle" }; - if (wc.isLoading()) return { kind: "Loading", url, title }; + // Main frame only. `isLoading()` covers the whole frame tree, so a + // cross-origin iframe that loads after the page can leave it true with no + // later event to clear it, and the tab's loading bar never finishes. + if (wc.isLoadingMainFrame()) return { kind: "Loading", url, title }; return { kind: "Success", url, title }; }; @@ -1607,9 +1610,21 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function wc.ipc.on(RECORDING_INPUT_CHANNEL, recordingInput); wc.ipc.on(MOUSE_NAVIGATE_CHANNEL, mouseNavigate); wc.setWindowOpenHandler((details) => { - if (previewWindowOpenAction(details) === "popup") { + const action = previewWindowOpenAction(details); + if (action === "popup") { return { action: "allow", overrideBrowserWindowOptions: POPUP_WINDOW_OPTIONS }; } + if (action === "deny") return { action: "deny" }; + if (action === "new-tab") { + runFork( + emitOpenLink({ + tabId, + url: details.url, + background: details.disposition === "background-tab", + }), + ); + return { action: "deny" }; + } runFork( attemptPromise({ operation: "openPreviewWindow", tabId, webContentsId: wc.id }, () => wc.loadURL(details.url), @@ -2234,8 +2249,23 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function const cropRect = normalizeCaptureRect(args[1]); const submission = args[2] === "send" && annotationSendEnabled.get(tabId) === true ? "send" : "attach"; + const devicePixelRatio = typeof args[3] === "number" ? args[3] : 1; runFork( - captureAnnotationScreenshot(tabId, wc, cropRect).pipe( + // The full-page capture with retries is what the screenshot button + // uses. A single cropped `capturePage(rect)` with a fixed timeout + // dropped the crop on pages where that capture was merely slow. + capturePageWithRetry( + { operation: "captureAnnotationScreenshot", tabId, webContentsId: wc.id }, + tabId, + wc, + ).pipe( + Effect.map((image) => cropAnnotationScreenshot(image, cropRect, devicePixelRatio)), + Effect.tapError((error) => + Effect.logWarning("preview annotation screenshot failed").pipe( + Effect.annotateLogs({ tabId, webContentsId: wc.id, error: error.message }), + ), + ), + Effect.withSpan("PreviewManager.captureAnnotationScreenshot"), // The renderer cannot tell a dropped crop from a comment-only // pick by the null alone, so a failed or timed-out capture is // flagged on the result. @@ -3320,6 +3350,16 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function ); }); + const emitOpenLink = Effect.fn("PreviewManager.emitOpenLink")(function* ( + event: DesktopPreviewOpenLinkEvent, + ) { + const listeners = yield* Ref.get(openLinkListenersRef); + yield* Effect.forEach( + listeners, + (listener) => deliverEvent("open-link", event.tabId, () => listener(event)), + { discard: true }, + ); + }); const subscribe = ( ref: Ref.Ref>, listener: A, @@ -3341,6 +3381,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function [ Ref.set(listenersRef, new Set()), Ref.set(pointerEventListenersRef, new Set()), + Ref.set(openLinkListenersRef, new Set()), Ref.set(recordingFrameListenersRef, new Set()), Ref.set(recordingInputListenersRef, new Set()), ], @@ -3386,6 +3427,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function stopRecording, subscribePointerEvents: (listener: PointerEventListener) => subscribe(pointerEventListenersRef, listener), + subscribeOpenLinks: (listener: OpenLinkListener) => subscribe(openLinkListenersRef, listener), subscribeRecordingInputs: (listener: RecordingInputListener) => subscribe(recordingInputListenersRef, listener), subscribeRecordingFrames: (listener: RecordingFrameListener) => @@ -3627,6 +3669,9 @@ export class PreviewManager extends Context.Service< readonly subscribePointerEvents: ( listener: PointerEventListener, ) => Effect.Effect; + readonly subscribeOpenLinks: ( + listener: OpenLinkListener, + ) => Effect.Effect; readonly subscribeRecordingInputs: ( listener: RecordingInputListener, ) => Effect.Effect; @@ -3642,13 +3687,48 @@ export const make = Effect.gen(function* PreviewManagerMake() { const browserSession = yield* BrowserSession.BrowserSession; const browserHost = yield* DesktopBrowserHost.DesktopBrowserHost; const downloadSessions = new WeakSet(); + const fileSystem = yield* FileSystem.FileSystem; + const path = yield* Path.Path; + const runForkDownload = Effect.runForkWith(yield* Effect.context()); + /** Puts a copy in Downloads under a name that never replaces a file already there. */ + const copyToDownloads = Effect.fn("PreviewManager.copyToDownloads")(function* ( + source: string, + fileName: string, + ) { + const directory = app.getPath("downloads"); + const extension = path.extname(fileName); + const stem = path.basename(fileName, extension) || "download"; + for (let attempt = 0; attempt < 100; attempt += 1) { + const target = path.join( + directory, + attempt === 0 ? `${stem}${extension}` : `${stem} (${attempt})${extension}`, + ); + if (yield* fileSystem.exists(target)) continue; + yield* fileSystem.copyFile(source, target); + shell.showItemInFolder(target); + return; + } + }); // Server tabs save downloads where the server's engine reads them. Downloads // the person starts in a tab the server is not driving keep Electron's dialog. const placeServerDownloads = (session: Electron.Session) => { if (downloadSessions.has(session)) return; downloadSessions.add(session); session.on("will-download", (_event, item, source) => { - browserHost.placeDownload(source, item); + // Decided before the server's path is set: placing it consumes the guid. + const human = browserHost.humanStartedDownload(source); + if (!browserHost.placeDownload(source, item) || !human) return; + // The server keeps its copy for agents; the person who clicked gets theirs. + item.once("done", (_doneEvent, state) => { + if (state !== "completed") return; + runForkDownload( + copyToDownloads(item.getSavePath(), item.getFilename()).pipe( + Effect.catch((error) => + Effect.logWarning("preview download copy failed", { cause: error }), + ), + ), + ); + }); }); }; @@ -3732,6 +3812,7 @@ export const make = Effect.gen(function* PreviewManagerMake() { saveRecording: operations.saveRecording, subscribeStateChanges: operations.subscribeStateChanges, subscribePointerEvents: operations.subscribePointerEvents, + subscribeOpenLinks: operations.subscribeOpenLinks, subscribeRecordingFrames: operations.subscribeRecordingFrames, subscribeRecordingInputs: operations.subscribeRecordingInputs, }); diff --git a/apps/desktop/src/preview/PickPreload.ts b/apps/desktop/src/preview/PickPreload.ts index a9155677ad381b27051ba57b7371c319c094dbbe..a656070f87b4ab74677f6808ad146080987e2ed5 100644 --- a/apps/desktop/src/preview/PickPreload.ts +++ b/apps/desktop/src/preview/PickPreload.ts @@ -1410,6 +1410,8 @@ function startAnnotation(sendEnabled: boolean): void { annotation, screenshotRect, submission === "send" && !sendEnabled ? "attach" : submission, + // Main crops a full-page capture, whose pixels are CSS px × this. + window.devicePixelRatio, ); }) .catch(() => { diff --git a/apps/desktop/src/window/DesktopApplicationMenu.test.ts b/apps/desktop/src/window/DesktopApplicationMenu.test.ts index 8de28324a58dc06e8ab397cffba47fd5e385ea9a..f111dac25db8df757b337dd6367e5083442843d6 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.test.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.test.ts @@ -87,6 +87,8 @@ const layerDesktopWindow = (selectedAction: Deferred.Deferred) => dispatchSnapShotEvent: () => Effect.void, zoomMain: (direction) => Deferred.succeed(selectedAction, `zoom-${direction}`).pipe(Effect.asVoid), + runMainContentsCommand: (command) => + Deferred.succeed(selectedAction, `main-${command}`).pipe(Effect.asVoid), syncAppearance: Effect.void, } satisfies DesktopWindow.DesktopWindow["Service"]); @@ -265,7 +267,9 @@ describe("DesktopApplicationMenu", () => { } assert.isUndefined( - viewMenu.submenu.find((item) => item.role?.toLowerCase().includes("zoom")), + viewMenu.submenu.find((item) => + ["zoom", "reload", "devtools"].some((role) => item.role?.toLowerCase().includes(role)), + ), ); const zoomIn = viewMenu.submenu.find((item) => item.label === "Zoom In"); @@ -279,4 +283,28 @@ describe("DesktopApplicationMenu", () => { assert.equal(yield* Deferred.await(selectedAction), "zoom-in"); }), ); + + it.effect("reloads the main window even while a browser page has focus", () => + Effect.gen(function* () { + const selectedAction = yield* Deferred.make(); + const applicationMenuTemplate = + yield* Deferred.make(); + + yield* configureMenu(selectedAction, applicationMenuTemplate); + + const template = yield* Deferred.await(applicationMenuTemplate); + const viewMenu = template.find((item) => item.label === "View"); + if (!Array.isArray(viewMenu?.submenu)) { + throw new Error("Expected View menu submenu to be an array."); + } + const reload = viewMenu.submenu.find((item) => item.label === "Reload"); + assert.equal(reload?.accelerator, "CmdOrCtrl+R"); + if (typeof reload?.click !== "function") { + throw new Error("Expected Reload menu item to have a click handler."); + } + + reload.click({} as Electron.MenuItem, {} as Electron.BrowserWindow, {} as KeyboardEvent); + assert.equal(yield* Deferred.await(selectedAction), "main-reload"); + }), + ); }); diff --git a/apps/desktop/src/window/DesktopApplicationMenu.ts b/apps/desktop/src/window/DesktopApplicationMenu.ts index 5aa59a2ae1b295399d3e1ceb3db3232f2d6bdec1..d0363fb01a272a99a7ef543c47be3ab11d2cae13 100644 --- a/apps/desktop/src/window/DesktopApplicationMenu.ts +++ b/apps/desktop/src/window/DesktopApplicationMenu.ts @@ -57,6 +57,13 @@ const zoomMainWindow = Effect.fn("desktop.menu.zoomMainWindow")(function* ( yield* desktopWindow.zoomMain(direction); }); +const runMainContentsCommand = Effect.fn("desktop.menu.runMainContentsCommand")(function* ( + command: DesktopWindow.MainWindowContentsCommand, +): Effect.fn.Return { + const desktopWindow = yield* DesktopWindow.DesktopWindow; + yield* desktopWindow.runMainContentsCommand(command); +}); + const checkForUpdatesFromMenu = Effect.gen(function* () { const updates = yield* DesktopUpdates.DesktopUpdates; const electronDialog = yield* ElectronDialog.ElectronDialog; @@ -150,6 +157,9 @@ export const make = Effect.gen(function* () { const zoomClick = (direction: DesktopWindow.MainWindowZoomDirection) => () => { runMenuEffect(`zoom-${direction}`, zoomMainWindow(direction)); }; + const mainContentsClick = (command: DesktopWindow.MainWindowContentsCommand) => () => { + runMenuEffect(command, runMainContentsCommand(command)); + }; const template: Electron.MenuItemConstructorOptions[] = []; if (environment.platform === "darwin") { @@ -227,16 +237,24 @@ export const make = Effect.gen(function* () { { label: "View", submenu: [ - { role: "reload" }, - { role: "forceReload" }, - { role: "toggleDevTools" }, - { type: "separator" }, /* - Not the zoom roles: those act on the focused webContents, so with - an embedded preview WebContentsView focused they zoom the guest - page and the app UI appears stuck. These always zoom the main - window (see DesktopWindow.zoomMain). + Not the reload, DevTools or zoom roles: those act on the focused + webContents, so with a browser page focused they reload or zoom + the guest page and the app UI appears stuck. These always target + the main window (see DesktopWindow.zoomMain). */ + { label: "Reload", accelerator: "CmdOrCtrl+R", click: mainContentsClick("reload") }, + { + label: "Force Reload", + accelerator: "Shift+CmdOrCtrl+R", + click: mainContentsClick("forceReload"), + }, + { + label: "Toggle Developer Tools", + accelerator: environment.platform === "darwin" ? "Alt+Command+I" : "Ctrl+Shift+I", + click: mainContentsClick("toggleDevTools"), + }, + { type: "separator" }, { label: "Actual Size", accelerator: "CmdOrCtrl+0", click: zoomClick("reset") }, { label: "Zoom In", accelerator: "CmdOrCtrl+=", click: zoomClick("in") }, { diff --git a/apps/desktop/src/window/DesktopWindow.test.ts b/apps/desktop/src/window/DesktopWindow.test.ts index cb374b8103deca6e183a4524b2066b50d9348b3f..10c3cb8adee92444785e600e5f105ab710fc0132 100644 --- a/apps/desktop/src/window/DesktopWindow.test.ts +++ b/apps/desktop/src/window/DesktopWindow.test.ts @@ -90,10 +90,12 @@ function makeFakeBrowserWindow() { once: vi.fn<(eventName: string, listener: (...args: readonly unknown[]) => void) => void>(), openDevTools: vi.fn(), reload: vi.fn(), + reloadIgnoringCache: vi.fn(), replaceMisspelling: vi.fn(), send: vi.fn(), setBackgroundThrottling: vi.fn(), setWindowOpenHandler: vi.fn(), + toggleDevTools: vi.fn(), }; const window = { @@ -138,6 +140,8 @@ function makeFakeBrowserWindow() { maximize: window.maximize, openDevTools: webContents.openDevTools, reload: webContents.reload, + reloadIgnoringCache: webContents.reloadIgnoringCache, + toggleDevTools: webContents.toggleDevTools, send: webContents.send, setZoomLevel: webContents.setZoomLevel, setWindowButtonPosition: window.setWindowButtonPosition, @@ -227,6 +231,7 @@ function layerTest(input: { readonly onPopupTemplate?: (input: ElectronMenu.ElectronMenuTemplateInput) => Effect.Effect; readonly previewZoomReapplies?: number[]; readonly onReveal?: (window: Electron.BrowserWindow) => void; + readonly focusedWindow?: Electron.BrowserWindow; }) { let desktopSettings = input.desktopSettings ?? DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS; const layerDesktopAppSettings = Layer.succeed(DesktopAppSettings.DesktopAppSettings, { @@ -273,7 +278,10 @@ function layerTest(input: { ), main: Ref.get(input.mainWindow), currentMainOrFirst: Ref.get(input.mainWindow), - focusedMainOrFirst: Ref.get(input.mainWindow), + focusedMainOrFirst: + input.focusedWindow === undefined + ? Ref.get(input.mainWindow) + : Effect.succeedSome(input.focusedWindow), setMain: (window) => Ref.set(input.mainWindow, Option.some(window)), clearMain: () => Ref.set(input.mainWindow, Option.none()), prepareReveal: () => Effect.succeed(false), @@ -782,6 +790,37 @@ describe("DesktopWindow", () => { }), ); + it.effect("reloads and inspects the main window even when a popup has focus", () => + Effect.gen(function* () { + const fakeWindow = makeFakeBrowserWindow(); + const popup = makeFakeBrowserWindow(); + const createCount = yield* Ref.make(0); + const mainWindow = yield* Ref.make>(Option.none()); + const layer = layerTest({ + window: fakeWindow.window, + createCount, + mainWindow, + focusedWindow: popup.window, + }); + + yield* Effect.gen(function* () { + const desktopWindow = yield* DesktopWindow.DesktopWindow; + yield* desktopWindow.handleBackendReady(new URL("http://127.0.0.1:3773")); + + yield* desktopWindow.runMainContentsCommand("reload"); + yield* desktopWindow.runMainContentsCommand("forceReload"); + yield* desktopWindow.runMainContentsCommand("toggleDevTools"); + + assert.equal(fakeWindow.reload.mock.calls.length, 1); + assert.equal(fakeWindow.reloadIgnoringCache.mock.calls.length, 1); + assert.equal(fakeWindow.toggleDevTools.mock.calls.length, 1); + assert.equal(popup.reload.mock.calls.length, 0); + assert.equal(popup.reloadIgnoringCache.mock.calls.length, 0); + assert.equal(popup.toggleDevTools.mock.calls.length, 0); + }).pipe(Effect.provide(layer)); + }), + ); + it.effect("keeps macOS window buttons centered when zooming and leaving fullscreen", () => Effect.gen(function* () { const fakeWindow = makeFakeBrowserWindow(); diff --git a/apps/desktop/src/window/DesktopWindow.ts b/apps/desktop/src/window/DesktopWindow.ts index 4e8fb0969ad3f6ef120744d429a06799916989c4..6dede819356a5b00df886b41db8519afef7e098e 100644 --- a/apps/desktop/src/window/DesktopWindow.ts +++ b/apps/desktop/src/window/DesktopWindow.ts @@ -93,6 +93,7 @@ export type DesktopWindowError = | PreviewManager.PreviewManagerError; export type MainWindowZoomDirection = "in" | "out" | "reset"; +export type MainWindowContentsCommand = "reload" | "forceReload" | "toggleDevTools"; export class DesktopWindow extends Context.Service< DesktopWindow, @@ -137,6 +138,10 @@ export class DesktopWindow extends Context.Service< // guest page instead of the app UI. The menu routes here to always target // the main window. readonly zoomMain: (direction: MainWindowZoomDirection) => Effect.Effect; + // Reload and DevTools for the main window's own webContents, for the same + // reason as zoomMain: the Electron roles act on the focused webContents, + // which is a preview guest whenever a browser page has focus. + readonly runMainContentsCommand: (command: MainWindowContentsCommand) => Effect.Effect; readonly syncAppearance: Effect.Effect; } >()("@t3tools/desktop/window/DesktopWindow") {} @@ -420,6 +425,10 @@ export const make = Effect.gen(function* () { nodeIntegration: false, sandbox: true, webviewTag: true, + // A preview guest's fullscreen request is mirrored onto this embedder, + // which would otherwise put the whole window into OS fullscreen. With + // both sides opted out the page fills its webview and the window stays. + disableHtmlFullscreenWindowResize: true, }, }); @@ -527,6 +536,7 @@ export const make = Effect.gen(function* () { webPreferences.nodeIntegration = false; webPreferences.nodeIntegrationInSubFrames = false; webPreferences.contextIsolation = false; + webPreferences.disableHtmlFullscreenWindowResize = true; }); const contextMenuContents = new WeakSet(); @@ -1027,6 +1037,17 @@ export const make = Effect.gen(function* () { // own zoom, so put each guest back where the preview left it. yield* previewManager.reapplyZoom(); }), + runMainContentsCommand: Effect.fn("desktop.window.runMainContentsCommand")(function* (command) { + yield* Effect.annotateCurrentSpan({ command }); + // The registered main window, never the focused one: with an OAuth popup + // focused, Reload would otherwise reload the popup mid sign-in. + const window = yield* electronWindow.main; + if (Option.isNone(window) || window.value.isDestroyed()) return; + const webContents = window.value.webContents; + if (command === "reload") webContents.reload(); + else if (command === "forceReload") webContents.reloadIgnoringCache(); + else webContents.toggleDevTools(); + }), syncAppearance: Effect.gen(function* () { const shouldUseDarkColors = yield* electronTheme.shouldUseDarkColors; yield* electronWindow.syncAllAppearance((window) => diff --git a/apps/web/src/browser/BrowserSurfaceSlot.tsx b/apps/web/src/browser/BrowserSurfaceSlot.tsx index b875e4c221efc2bc4391ae212a412da95e317649..c30a622746142c2fcc6f6209695730184bd2cd43 100644 --- a/apps/web/src/browser/BrowserSurfaceSlot.tsx +++ b/apps/web/src/browser/BrowserSurfaceSlot.tsx @@ -68,6 +68,12 @@ export function BrowserSurfaceSlot(props: { // full width. The slot moves without resizing, so measure on shell resizes too. const panel = element.closest('[data-preview-panel-mode="inline"]'); if (panel) observer.observe(panel); + // A sidebar opening or closing shifts the whole column, so a slot that keeps + // its size and its offset inside the column (the mini player on the left) + // still moves on screen. The gap element carries the sidebar's width. + for (const gap of document.querySelectorAll('[data-slot="sidebar-gap"]')) { + observer.observe(gap); + } window.addEventListener("resize", update); window.addEventListener("scroll", update, true); return () => { diff --git a/apps/web/src/browser/ElectronBrowserHost.tsx b/apps/web/src/browser/ElectronBrowserHost.tsx index 5cbefd9dded661ae57a4a575db392ff33fe3bd83..e32dcb9ee4df03e22d3388b49791708c10db5c46 100644 --- a/apps/web/src/browser/ElectronBrowserHost.tsx +++ b/apps/web/src/browser/ElectronBrowserHost.tsx @@ -3,18 +3,21 @@ import { parseScopedThreadKey } from "@t3tools/client-runtime/environment"; import { AuthPreviewOperateScope, FILL_PREVIEW_VIEWPORT } from "@t3tools/contracts"; import { useAtomValue } from "@effect/atom-react"; -import { type ComponentProps, useEffect, useMemo } from "react"; +import { type ComponentProps, useEffect, useMemo, useRef } from "react"; import { primaryEnvironmentIdAtom } from "~/state/primaryEnvironment"; import { isElectron } from "~/env"; import { useTheme } from "~/hooks/useTheme"; import { useActivePreviewSessions } from "~/previewStateStore"; +import { previewEnvironment } from "~/state/preview"; import { useEnvironmentScope } from "~/state/session"; +import { useAtomCommand } from "~/state/use-atom-command"; import { readPreviewAnnotationTheme } from "./annotationTheme"; import { useBrowserPointerStore } from "./browserPointerStore"; import { HostedBrowserWebview } from "./HostedBrowserWebview"; +import { openUrlInPreview } from "./openFileInPreview"; import { rendersServerTabNatively } from "./previewRuntime"; import { previewRuntimeTabId } from "./previewRuntimeTabId"; @@ -92,6 +95,32 @@ export function ElectronBrowserHost() { }); }, []); + // A `target="_blank"` link inside a hosted page opens as another tab of the + // same thread, so the page that held the link stays where it is. + const openPreview = useAtomCommand(previewEnvironment.open, { reportFailure: true }); + const sessionByRuntimeTabId = useRef(new Map()); + useEffect(() => { + sessionByRuntimeTabId.current = new Map( + sessions.map((session) => [session.runtimeTabId, session]), + ); + }, [sessions]); + useEffect(() => { + const preview = window.desktopBridge?.preview; + if (!preview) return; + return preview.onOpenLink(({ tabId, url, background }) => { + const source = sessionByRuntimeTabId.current.get(tabId); + if (!source) return; + // The new tab keeps the source tab's profile so its cookies carry over. + void openUrlInPreview({ + threadRef: source.threadRef, + url, + openPreview, + profileId: source.snapshot.profileId, + background, + }); + }); + }, [openPreview]); + if (!isElectron) return null; return (
diff --git a/apps/web/src/browser/browserTargetResolver.test.ts b/apps/web/src/browser/browserTargetResolver.test.ts index c2b3432402ed07e6f073df1efcd6045b3453c901..c2b6c9ea64b476a1041df3de7a7082bbb0568dc4 100644 --- a/apps/web/src/browser/browserTargetResolver.test.ts +++ b/apps/web/src/browser/browserTargetResolver.test.ts @@ -2,8 +2,10 @@ import { EnvironmentId } from "@t3tools/contracts"; import { beforeEach, describe, expect, it, vi } from "vite-plus/test"; const readPreparedConnection = vi.fn(); +const env = vi.hoisted(() => ({ isElectron: false })); vi.mock("~/state/session", () => ({ readPreparedConnection })); +vi.mock("~/env", () => env); describe("browser target resolver", () => { beforeEach(() => readPreparedConnection.mockReset()); @@ -160,6 +162,33 @@ describe("browser target resolver", () => { ).toBe("http://192.168.1.25:3000/app"); }); + it("keeps localhost for servers on the desktop's own WSL or primary backend", async () => { + // WSL NAT mode serves the backend on the distro IP, but a loopback-only dev + // server there answers only on localhost from the Windows desktop. + const { resolveDiscoveredServerUrl } = await import("./browserTargetResolver"); + env.isElectron = true; + try { + for (const target of [ + { _tag: "BearerConnectionTarget", connectionId: "local:wsl:Ubuntu" }, + { _tag: "PrimaryConnectionTarget" }, + ]) { + readPreparedConnection.mockReturnValue({ httpBaseUrl: "http://172.24.66.27:3773", target }); + expect( + resolveDiscoveredServerUrl(EnvironmentId.make("environment-1"), "localhost:3001/"), + ).toBe("http://localhost:3001/"); + } + readPreparedConnection.mockReturnValue({ + httpBaseUrl: "http://192.168.1.25:3773", + target: { _tag: "BearerConnectionTarget", connectionId: "saved:remote" }, + }); + expect( + resolveDiscoveredServerUrl(EnvironmentId.make("environment-1"), "localhost:3001/"), + ).toBe("http://192.168.1.25:3001/"); + } finally { + env.isElectron = false; + } + }); + it("preserves localhost server-picker values when the prepared base is 127.0.0.1", async () => { readPreparedConnection.mockReturnValue({ httpBaseUrl: "http://127.0.0.1:3773" }); const { resolveDiscoveredServerUrl } = await import("./browserTargetResolver"); diff --git a/apps/web/src/browser/browserTargetResolver.ts b/apps/web/src/browser/browserTargetResolver.ts index 4acb1e2b487f6caf6a32253072cd0df21732fa4f..ec1b150819a0369bef0f7ee7cfa1316617b71caf 100644 --- a/apps/web/src/browser/browserTargetResolver.ts +++ b/apps/web/src/browser/browserTargetResolver.ts @@ -6,6 +6,10 @@ import type { import { isLoopbackHost, normalizePreviewUrl } from "@t3tools/shared/preview"; import { isLocalLoopbackHost, isPrivateNetworkHost } from "@t3tools/shared/hostClassification"; +import type { ConnectionTarget } from "@t3tools/client-runtime/connection"; + +import { isDesktopLocalConnectionTarget } from "~/connection/desktopLocal"; +import { isElectron } from "~/env"; import { readPreparedConnection } from "~/state/session"; export { @@ -75,11 +79,24 @@ export function resolveBrowserNavigationTarget( return resolveEnvironmentPortTarget(environmentId, target, readEnvironmentUrl(environmentId)); } +/** + * Whether a discovered loopback server is reachable as-is from this client's + * own browser tabs. The desktop app draws tabs on the machine that runs its + * primary backend and any desktop-local backend (WSL). Its connection URL can + * still be a LAN address (WSL NAT mode binds the distro IP), but a dev server + * bound to loopback only answers on `localhost` there. + */ +const reachesEnvironmentLoopback = (target: ConnectionTarget): boolean => + isElectron && + (target._tag === "PrimaryConnectionTarget" || isDesktopLocalConnectionTarget(target)); + export function resolveDiscoveredServerUrl(environmentId: EnvironmentId, rawUrl: string): string { try { const normalizedUrl = normalizePreviewUrl(rawUrl); const parsed = new URL(normalizedUrl); if (!isLoopbackHost(parsed.hostname)) return normalizedUrl; + const target = readPreparedConnection(environmentId)?.target; + if (target && reachesEnvironmentLoopback(target)) return normalizedUrl; return resolveEnvironmentPortTarget( environmentId, { diff --git a/apps/web/src/browser/browserViewportLayout.test.ts b/apps/web/src/browser/browserViewportLayout.test.ts index 23c8a2996cc050e6ff85c5cf1fc7c8e92cf85125..ca2832bea6ce175495c51677fb4c1d90d88a7bf0 100644 --- a/apps/web/src/browser/browserViewportLayout.test.ts +++ b/apps/web/src/browser/browserViewportLayout.test.ts @@ -123,15 +123,15 @@ describe("resolveBrowserViewportLayout", () => { expect( resolveBrowserDeviceViewportLayout( { width: 1200, height: 900 }, - { _tag: "freeform", width: 1180, height: 858 }, + { _tag: "freeform", width: 1180, height: 852 }, ), ).toEqual({ canvasWidth: 1200, canvasHeight: 900, viewportX: 10, - viewportY: 32, + viewportY: 38, viewportWidth: 1180, - viewportHeight: 858, + viewportHeight: 852, viewportScale: 1, fillsPanel: false, }); @@ -140,11 +140,11 @@ describe("resolveBrowserViewportLayout", () => { it("captures the available framed area when responsive mode is enabled", () => { expect(resolveResponsiveBrowserViewportSize({ width: 1200, height: 900 })).toEqual({ width: 1180, - height: 858, + height: 852, }); expect(resolveResponsiveBrowserViewportSize({ width: 1200, height: 900 }, 2)).toEqual({ width: 590, - height: 429, + height: 426, }); }); diff --git a/apps/web/src/browser/browserViewportLayout.ts b/apps/web/src/browser/browserViewportLayout.ts index 414701a816c552955abf749e2d1606d9713cdb89..489a8f16f3913064103b51659706340770582f29 100644 --- a/apps/web/src/browser/browserViewportLayout.ts +++ b/apps/web/src/browser/browserViewportLayout.ts @@ -19,7 +19,10 @@ export interface BrowserViewportLayout { readonly fillsPanel: boolean; } -export const BROWSER_DEVICE_TOOLBAR_HEIGHT = 32; +// Room for a compact size input (30px with its border) plus its 3px focus ring +// on both sides, inside the toolbar's 1px bottom border. The bar scrolls +// horizontally, which also clips anything that overflows vertically. +export const BROWSER_DEVICE_TOOLBAR_HEIGHT = 38; export const BROWSER_VIEWPORT_RESIZE_RAIL_SIZE = 10; export type BrowserViewportResizeDirection = diff --git a/apps/web/src/browser/openFileInPreview.ts b/apps/web/src/browser/openFileInPreview.ts index 615b8545490573e00b566ab6181d0c00090816f0..c582e5a5b6c8b4457f2d0e0ed84b604a5696f66a 100644 --- a/apps/web/src/browser/openFileInPreview.ts +++ b/apps/web/src/browser/openFileInPreview.ts @@ -17,8 +17,14 @@ import { AsyncResult } from "effect/reactivity"; import { resolveAssetUrl } from "~/assets/assetUrls"; import { isPreviewAvailableFor, previewRuntimeFor } from "~/browser/previewRuntime"; -import { applyPreviewServerSnapshot, rememberPreviewUrl } from "~/previewStateStore"; -import { useRightPanelStore } from "~/rightPanelStore"; +import { + applyPreviewServerSnapshot, + readThreadPreviewState, + rememberPreviewUrl, + setActivePreviewTab, + updatePreviewServerSnapshot, +} from "~/previewStateStore"; +import { selectSelectedRightPanelSurface, useRightPanelStore } from "~/rightPanelStore"; import { browserDefaultOpenProfileId, @@ -52,6 +58,10 @@ export async function openUrlInPreview(input: { readonly threadRef: ScopedThreadRef; readonly url: string; readonly openPreview: OpenPreviewMutation; + /** Profile to open under; omit for the configured default. */ + readonly profileId?: PreviewOpenInput["profileId"]; + /** Open the tab without switching the thread to it. */ + readonly background?: boolean; }): Promise> { const defaults = await resolveBrowserDefaults().catch( (cause: unknown) => new BrowserSettingsReadError({ cause }), @@ -60,6 +70,13 @@ export async function openUrlInPreview(input: { return AsyncResult.failure(Cause.fail(defaults)); } const runtime = previewRuntimeFor(input.threadRef.environmentId); + const previousActiveTabId = readThreadPreviewState(input.threadRef).activeTabId; + // The server's "opened" event switches the preview tab but not the panel's + // selection, so a changed selection means the user picked a tab themselves. + const selectedSurface = () => + selectSelectedRightPanelSurface(useRightPanelStore.getState().byThreadKey, input.threadRef) + ?.id ?? null; + const surfaceBeforeOpen = selectedSurface(); const result = await input.openPreview({ environmentId: input.threadRef.environmentId, input: { @@ -69,13 +86,26 @@ export async function openUrlInPreview(input: { // maps the result differently, so the configured defaults have to be // applied explicitly or file/link opens would ignore them. viewport: browserDefaultOpenViewport(defaults), - profileId: browserDefaultOpenProfileId(defaults), + profileId: input.profileId ?? browserDefaultOpenProfileId(defaults), ...(runtime === undefined ? {} : { runtime }), }, }); return mapAtomCommandResult(result, (snapshot) => { - applyPreviewServerSnapshot(input.threadRef, snapshot); rememberPreviewUrl(input.threadRef, input.url); + if (input.background) { + updatePreviewServerSnapshot(input.threadRef, snapshot); + // The server's "opened" event activates the new tab; hand focus back, + // unless the user picked a tab, this one included, while the open was in flight. + if ( + previousActiveTabId && + readThreadPreviewState(input.threadRef).activeTabId === snapshot.tabId && + selectedSurface() === surfaceBeforeOpen + ) { + setActivePreviewTab(input.threadRef, previousActiveTabId); + } + return; + } + applyPreviewServerSnapshot(input.threadRef, snapshot); useRightPanelStore.getState().openBrowser(input.threadRef, snapshot.tabId); }); } diff --git a/apps/web/src/components/AppSidebarLayout.tsx b/apps/web/src/components/AppSidebarLayout.tsx index 918d28845068a4daab7310d261deac80f39502cd..00d5c05ef089029f04a18ed6fbb64b5d3a89b1ca 100644 --- a/apps/web/src/components/AppSidebarLayout.tsx +++ b/apps/web/src/components/AppSidebarLayout.tsx @@ -124,7 +124,14 @@ function SidebarControl() { // Capture before focused editors consume commands such as Mod+B for rich-text formatting. window.addEventListener("keydown", onKeyDown, true); - return () => window.removeEventListener("keydown", onKeyDown, true); + // A focused desktop browser page forwards the chord as a menu action. + const unsubscribe = window.desktopBridge?.onMenuAction((action) => { + if (action === "sidebar.toggle") toggleSidebar(); + }); + return () => { + window.removeEventListener("keydown", onKeyDown, true); + unsubscribe?.(); + }; }, [keybindings, toggleSidebar, usagePageOpen]); return ( diff --git a/apps/web/src/components/ChatView.tsx b/apps/web/src/components/ChatView.tsx index d6baf94b5ff4f284dafc898349846739746a27a5..69a204ccd8a35a9d0ddef972765dd86c52fa4680 100644 --- a/apps/web/src/components/ChatView.tsx +++ b/apps/web/src/components/ChatView.tsx @@ -8076,6 +8076,13 @@ export default function ChatView(props: ChatViewProps) { return; } + if (command === "rightPanel.toggleMaximized") { + event.preventDefault(); + event.stopPropagation(); + if (!event.repeat) toggleRightPanelMaximized(); + return; + } + if (command === "threadPanel.toggle") { event.preventDefault(); event.stopPropagation(); @@ -8313,6 +8320,7 @@ export default function ChatView(props: ChatViewProps) { closeThreadFind, isThreadFindActive, toggleRightPanel, + toggleRightPanelMaximized, toggleThreadPanel, toggleTerminalVisibility, composerRef, @@ -8324,6 +8332,14 @@ export default function ChatView(props: ChatViewProps) { onEnvironmentChange, ]); + // A focused desktop browser page forwards these chords as menu actions. + useEffect(() => { + return window.desktopBridge?.onMenuAction((action) => { + if (action === "rightPanel.toggle") toggleRightPanel(); + else if (action === "rightPanel.toggleMaximized") toggleRightPanelMaximized(); + }); + }, [toggleRightPanel, toggleRightPanelMaximized]); + // Paste-to-focus: the resting composer blurs on a click into the timeline, // so a paste that follows has no editable target and would be dropped. // Route it to the composer like a typed key, which also expands it. diff --git a/apps/web/src/components/ReopenClosedViewShortcut.test.tsx b/apps/web/src/components/ReopenClosedViewShortcut.test.tsx index e43fd4069b691ba2056ff6cab3c53704fbd1fcd2..9add737a2944f6ab2a6317f5dd646e5c903a7c3c 100644 --- a/apps/web/src/components/ReopenClosedViewShortcut.test.tsx +++ b/apps/web/src/components/ReopenClosedViewShortcut.test.tsx @@ -309,17 +309,27 @@ describe("root reopen shortcut", () => { ); it("forwards the chord to a focused desktop browser only while history exists", async () => { + const forwardedCommands = () => + (state.setShortcuts.mock.lastCall as unknown as [Array<{ command: string }>])[0].map( + ({ command }) => command, + ); useClosedViewStore .getState() .remember({ kind: "panel-tab", threadRef: ref, surface: { kind: "diff", id: "diff" } }); await render(); + expect(forwardedCommands()).toContain("view.reopenClosed"); + await act(() => { + menuAction?.("view.reopenClosed"); + }); + expect(useClosedViewStore.getState().entries).toEqual([]); + expect(forwardedCommands()).not.toContain("view.reopenClosed"); + }); + + it("always forwards the layout toggles out of a focused desktop browser", async () => { + await render(); expect(state.setShortcuts).toHaveBeenLastCalledWith([ - expect.objectContaining({ command: "view.reopenClosed" }), + expect.objectContaining({ command: "sidebar.toggle" }), + expect.objectContaining({ command: "rightPanel.toggle" }), ]); - await act(() => { - menuAction?.("view.reopenClosed"); - }); - expect(useClosedViewStore.getState().entries).toEqual([]); - expect(state.setShortcuts).toHaveBeenLastCalledWith([]); }); }); diff --git a/apps/web/src/components/ReopenClosedViewShortcut.tsx b/apps/web/src/components/ReopenClosedViewShortcut.tsx index d8e3b65a5c9eb2a886ddfffb01191418c22015d6..41cc2144deec7e2e2927228f4527932e1921e686 100644 --- a/apps/web/src/components/ReopenClosedViewShortcut.tsx +++ b/apps/web/src/components/ReopenClosedViewShortcut.tsx @@ -43,6 +43,13 @@ import { } from "../threadRoutes"; import { toastManager } from "./ui/toast"; +/** Layout commands a focused desktop browser page hands back to the app. */ +const PREVIEW_FORWARDED_LAYOUT_COMMANDS = [ + "sidebar.toggle", + "rightPanel.toggle", + "rightPanel.toggleMaximized", +] as const; + const isGlobalPullRequests = (ref: ScopedThreadRef) => scopedThreadKey(ref) === scopedThreadKey(PULL_REQUESTS_PANEL_REF); @@ -175,22 +182,33 @@ export function ReopenClosedViewShortcut() { useEffect(() => { const preview = window.desktopBridge?.preview; if (!preview?.setForwardedShortcuts) return; + const options = { + context: { + previewFocus: true, + previewOpen: true, + terminalFocus: false, + terminalOpen, + editableFocus: false, + modelPickerOpen: false, + isDesktop: true, + isWeb: false, + }, + }; + // A focused browser page is a separate process, so these chords never reach + // the app's keydown listeners. The desktop claims them in the guest and + // sends each command back as a menu action. + const commands = [ + ...(hasHistory ? (["view.reopenClosed"] as const) : []), + ...PREVIEW_FORWARDED_LAYOUT_COMMANDS, + ]; void preview .setForwardedShortcuts( - hasHistory - ? effectiveShortcutsForCommand(keybindings, "view.reopenClosed", { - context: { - previewFocus: true, - previewOpen: true, - terminalFocus: false, - terminalOpen, - editableFocus: false, - modelPickerOpen: false, - isDesktop: true, - isWeb: false, - }, - }).map((shortcut) => ({ command: "view.reopenClosed", shortcut })) - : [], + commands.flatMap((command) => + effectiveShortcutsForCommand(keybindings, command, options).map((shortcut) => ({ + command, + shortcut, + })), + ), ) .catch(() => undefined); return () => { diff --git a/apps/web/src/components/composerContextPresentation.test.ts b/apps/web/src/components/composerContextPresentation.test.ts index b4eb4bbe0cb322b5274d0d1f251b4ea1e4d5c51a..4c82eb94ef2465e5c4814f3bb2075159e9741f0f 100644 --- a/apps/web/src/components/composerContextPresentation.test.ts +++ b/apps/web/src/components/composerContextPresentation.test.ts @@ -8,6 +8,44 @@ import { } from "./composerContextPresentation"; describe("composerContextRecordsFromDraft", () => { + it("pairs a picked element with its crop, which the draft keeps as an image", () => { + const annotation = { + id: "pick-1", + pageUrl: "http://localhost:3000/", + pageTitle: "Example", + comment: "", + elements: [], + regions: [], + strokes: [], + styleChanges: [], + // The draft stores annotations without their data URL. + screenshot: { + dataUrl: "", + width: 10, + height: 10, + cropRect: { x: 0, y: 0, width: 10, height: 10 }, + }, + createdAt: "2026-10-08T00:00:00.000Z", + }; + const crop = { + type: "image" as const, + id: "pick-1", + name: "preview-annotation-pick-1.png", + mimeType: "image/png", + sizeBytes: 1, + previewUrl: "blob:crop", + file: new File(["x"], "preview-annotation-pick-1.png"), + }; + + expect( + composerContextRecordsFromDraft({ + terminalContexts: [], + previewAnnotations: [annotation], + images: [crop], + }).get("preview-annotation_pick-1"), + ).toMatchObject({ kind: "preview-annotation", screenshot: { previewUrl: "blob:crop" } }); + }); + it("recovers the uploaded record when clipboard data points at an attachment already in the draft", () => { const file = { type: "file" as const, diff --git a/apps/web/src/components/composerContextPresentation.tsx b/apps/web/src/components/composerContextPresentation.tsx index 7e639e05793cefd95ed270c3dbd8f3be19ce3613..e8e493be73c67b5f52c611412de9879a54c4e7cf 100644 --- a/apps/web/src/components/composerContextPresentation.tsx +++ b/apps/web/src/components/composerContextPresentation.tsx @@ -55,7 +55,12 @@ import { export type ComposerDraftContextRecord = | { kind: "terminal"; record: TerminalContextDraft } | { kind: "review-comment"; record: ReviewCommentContext } - | { kind: "preview-annotation"; record: PreviewAnnotationPayload } + | { + kind: "preview-annotation"; + record: PreviewAnnotationPayload; + /** The crop, which the draft stores as an image sharing the annotation's id. */ + screenshot?: ComposerImageAttachment | undefined; + } | { kind: "image"; record: ComposerImageAttachment; upload?: AttachmentUploadState | undefined } | { kind: "file"; record: ComposerFileAttachment; upload?: AttachmentUploadState | undefined } | { kind: "thread"; record: ThreadContextRecord }; @@ -122,8 +127,13 @@ export function composerContextRecordsFromDraft(input: { for (const record of input.reviewComments ?? []) { records.set(reviewCommentContextId(record.id), { kind: "review-comment", record }); } + const imagesById = new Map((input.images ?? []).map((image) => [image.id, image])); for (const record of input.previewAnnotations ?? []) { - records.set(previewAnnotationContextId(record.id), { kind: "preview-annotation", record }); + records.set(previewAnnotationContextId(record.id), { + kind: "preview-annotation", + record, + screenshot: imagesById.get(record.id), + }); } for (const record of input.threadContexts ?? []) { records.set(record.contextId, { kind: "thread", record }); @@ -291,15 +301,19 @@ function ComposerReviewCommentDetails({ comment }: { comment: ReviewCommentConte function ComposerPreviewAnnotationDetails({ annotation, + screenshot, }: { annotation: PreviewAnnotationPayload; + screenshot: ComposerImageAttachment | undefined; }) { const summary = previewAnnotationTooltip(annotation); + // The draft drops the annotation's own data URL and keeps the crop as an image. + const screenshotUrl = annotation.screenshot?.dataUrl || screenshot?.previewUrl; return (
- {annotation.screenshot?.dataUrl ? ( + {screenshotUrl ? ( Annotated preview crop @@ -406,7 +420,12 @@ const composerContextPresentationRegistry = createContextPresentationRegistry< icon={} label={previewAnnotationContextLabel(entry.record)} kindLabel="Preview annotation" - details={} + details={ + + } detailsMode={definition.capabilities.details} kind="preview-annotation" /> diff --git a/apps/web/src/components/preview/openPreviewSession.test.ts b/apps/web/src/components/preview/openPreviewSession.test.ts index 70751aef8342d47642a382f79754867ad5008ca4..792a20b7c4aa1ca165f997f7cdfbb29d6eba8c10 100644 --- a/apps/web/src/components/preview/openPreviewSession.test.ts +++ b/apps/web/src/components/preview/openPreviewSession.test.ts @@ -13,7 +13,13 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vite-plus/test" import * as browserDefaults from "~/browser/browserDefaults"; import { BrowserSettingsReadError, openUrlInPreview } from "~/browser/openFileInPreview"; import { __setClientSettingsForTests } from "~/hooks/useSettings"; -import { readThreadPreviewState, resetPreviewStateForTests } from "~/previewStateStore"; +import { useRightPanelStore } from "~/rightPanelStore"; +import { + applyPreviewServerSnapshot, + readThreadPreviewState, + resetPreviewStateForTests, + setActivePreviewTab, +} from "~/previewStateStore"; import { openPreviewSession } from "./openPreviewSession"; @@ -37,6 +43,7 @@ const snapshot: PreviewSessionSnapshot = { beforeEach(() => { resetPreviewStateForTests(); + useRightPanelStore.setState({ byThreadKey: {}, threadPanelVisibilityByThreadKey: {} }); __setClientSettingsForTests(DEFAULT_CLIENT_SETTINGS); }); @@ -140,3 +147,82 @@ describe("openPreviewSession", () => { }, ); }); + +describe("openUrlInPreview from a link", () => { + it("opens under the source tab's profile instead of the default", async () => { + const openPreview = vi.fn(async (_arg: { input: PreviewOpenInput }) => + AsyncResult.success(snapshot), + ); + + await openUrlInPreview({ openPreview, threadRef, url: "https://t3.chat/", profileId: "work" }); + + expect(openPreview.mock.calls[0]?.[0].input.profileId).toBe("work"); + }); + + it("keeps the current tab active for a background open", async () => { + applyPreviewServerSnapshot(threadRef, { ...snapshot, tabId: "tab-current" }); + + await openUrlInPreview({ + openPreview: async () => AsyncResult.success(snapshot), + threadRef, + url: "https://t3.chat/", + background: true, + }); + + const state = readThreadPreviewState(threadRef); + expect(state.activeTabId).toBe("tab-current"); + expect(Object.keys(state.sessions).toSorted()).toEqual(["tab-1", "tab-current"]); + }); + + it("keeps a tab the user picked while a background open was in flight", async () => { + applyPreviewServerSnapshot(threadRef, { ...snapshot, tabId: "tab-current" }); + applyPreviewServerSnapshot(threadRef, { ...snapshot, tabId: "tab-other" }); + setActivePreviewTab(threadRef, "tab-current"); + + await openUrlInPreview({ + openPreview: async () => { + // The server activates the new tab, then the user selects another one + // before the open resolves. + applyPreviewServerSnapshot(threadRef, snapshot); + setActivePreviewTab(threadRef, "tab-other"); + return AsyncResult.success(snapshot); + }, + threadRef, + url: "https://t3.chat/", + background: true, + }); + + expect(readThreadPreviewState(threadRef).activeTabId).toBe("tab-other"); + }); + + it("keeps the new tab when the user picks it while a background open is in flight", async () => { + applyPreviewServerSnapshot(threadRef, { ...snapshot, tabId: "tab-current" }); + setActivePreviewTab(threadRef, "tab-current"); + + await openUrlInPreview({ + openPreview: async () => { + // The server activates the new tab, then the user clicks that same tab. + applyPreviewServerSnapshot(threadRef, snapshot); + useRightPanelStore.getState().openBrowser(threadRef, snapshot.tabId); + return AsyncResult.success(snapshot); + }, + threadRef, + url: "https://t3.chat/", + background: true, + }); + + expect(readThreadPreviewState(threadRef).activeTabId).toBe(snapshot.tabId); + }); + + it("activates the new tab for a foreground open", async () => { + applyPreviewServerSnapshot(threadRef, { ...snapshot, tabId: "tab-current" }); + + await openUrlInPreview({ + openPreview: async () => AsyncResult.success(snapshot), + threadRef, + url: "https://t3.chat/", + }); + + expect(readThreadPreviewState(threadRef).activeTabId).toBe("tab-1"); + }); +}); diff --git a/packages/contracts/src/ipc.ts b/packages/contracts/src/ipc.ts index 149f282194a59976ef5e4d3efd0d46822569649a..5cee1f9280a721a895553aeed511889ae40dc7b4 100644 --- a/packages/contracts/src/ipc.ts +++ b/packages/contracts/src/ipc.ts @@ -658,6 +658,14 @@ export interface DesktopPreviewPointerEvent { createdAt: string; } +/** A `target="_blank"` link the previewed page asked to open beside itself. */ +export interface DesktopPreviewOpenLinkEvent { + tabId: string; + url: string; + /** True for middle-click / Cmd-click, which should not take focus. */ + background: boolean; +} + /** Recording decorations are forwarded separately from the captured page pixels. */ export const DesktopPreviewRecordingInputSchema = Schema.Union([ Schema.Struct({ @@ -1331,6 +1339,7 @@ export interface DesktopPreviewBridge { }; onStateChange: (listener: (tabId: string, state: DesktopPreviewTabState) => void) => () => void; onPointerEvent: (listener: (event: DesktopPreviewPointerEvent) => void) => () => void; + onOpenLink: (listener: (event: DesktopPreviewOpenLinkEvent) => void) => () => void; } export type ConfirmDialogVariant = "default" | "destructive"; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b948baba7c65c30129e92c6c3cdd31884ad2cc09..edc6c3e9a99a562a9dec54c447f3db04179ddf4b 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -185,7 +185,7 @@ importers: dependencies: '@clerk/electron': specifier: 0.0.44 - version: 0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.2)(react-dom@19.3.0(react@19.3.0))(react@19.2.6) + version: 0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.5)(react-dom@19.3.0(react@19.3.0))(react@19.2.6) '@clerk/electron-passkeys': specifier: 0.0.3 version: 0.0.3 @@ -220,8 +220,8 @@ importers: specifier: 4.0.1 version: 4.0.1(patch_hash=a33cba07c41f32374c2aaa86ea4a84d3ab9a872a78c94b65e929b19f8361856f) electron: - specifier: 44.4.2 - version: 44.4.2 + specifier: 44.4.5 + version: 44.4.5 electron-store: specifier: ^8.2.0 version: 8.2.0 @@ -687,7 +687,7 @@ importers: version: 1.5.0(@date-fns/tz@1.5.0)(@types/react@19.3.0)(date-fns@4.4.0)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@clerk/electron': specifier: 0.0.44 - version: 0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.2)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) + version: 0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.5)(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@clerk/react': specifier: 6.16.1 version: 6.16.1(react-dom@19.2.6(react@19.2.6))(react@19.2.6) @@ -7727,8 +7727,8 @@ packages: resolution: {integrity: sha512-bO3y10YikuUwUuDUQRM4KfwNkKhnpVO7IPdbsrejwN9/AABJzzTQ4GeHwyzNSrVO+tEH3/Np255a3sVZpZDjvg==} engines: {node: '>=8.0.0'} - electron@44.4.2: - resolution: {integrity: sha512-872t/+8M8TyribWm2cYpRwHY2aDlyDHOn+CHnhN9kHQStVW8pAijIhFolYA/qQyDdri28Xp8Mi3HEj5kFvajCw==} + electron@44.4.5: + resolution: {integrity: sha512-SjgoaeYsSWZfJzubgQU7juvuXMTvn6/e1gAHdGFA/yuMbpF+I+skYqIJ6DdXBXHeWbkbpNpmwZCTpiivtlWZSw==} engines: {node: '>= 22.12.0'} hasBin: true @@ -12858,12 +12858,12 @@ snapshots: '@clerk/electron-passkeys-win32-arm64-msvc': 0.0.3 '@clerk/electron-passkeys-win32-x64-msvc': 0.0.3 - '@clerk/electron@0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.2)(react-dom@19.2.6(react@19.2.6))(react@19.2.6)': + '@clerk/electron@0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.5)(react-dom@19.2.6(react@19.2.6))(react@19.2.6)': dependencies: '@clerk/clerk-js': 6.32.1(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@clerk/react': 6.16.1(react-dom@19.2.6(react@19.2.6))(react@19.2.6) '@clerk/shared': 4.33.0(react-dom@19.2.6(react@19.2.6))(react@19.2.6) - electron: 44.4.2 + electron: 44.4.5 react: 19.2.6 tslib: 2.8.1 optionalDependencies: @@ -12871,12 +12871,12 @@ snapshots: electron-store: 8.2.0 react-dom: 19.2.6(react@19.2.6) - '@clerk/electron@0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.2)(react-dom@19.3.0(react@19.3.0))(react@19.2.6)': + '@clerk/electron@0.0.44(@clerk/electron-passkeys@0.0.3)(electron-store@8.2.0)(electron@44.4.5)(react-dom@19.3.0(react@19.3.0))(react@19.2.6)': dependencies: '@clerk/clerk-js': 6.32.1(react-dom@19.3.0(react@19.3.0))(react@19.2.6) '@clerk/react': 6.16.1(react-dom@19.3.0(react@19.3.0))(react@19.2.6) '@clerk/shared': 4.33.0(react-dom@19.3.0(react@19.3.0))(react@19.2.6) - electron: 44.4.2 + electron: 44.4.5 react: 19.2.6 tslib: 2.8.1 optionalDependencies: @@ -18180,7 +18180,7 @@ snapshots: transitivePeerDependencies: - supports-color - electron@44.4.2: + electron@44.4.5: dependencies: '@electron-internal/extract-zip': 1.0.5 '@electron/get': 5.1.0 diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index c7f0a786151a307184df1d722b377c17b95b5c5e..fe0ca51174b3c12451af49e7a8cd12583d91cdab 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -130,7 +130,7 @@ minimumReleaseAgeExclude: - expo-sharing@58.0.13 - expo-sqlite@58.0.8 - expo-symbols@58.0.3 - - electron@44.4.2 + - electron@44.4.5 - "@anthropic-ai/claude-agent-sdk@0.3.276" - "@alchemy.run/cloudflare-runtime@2.0.0-beta.80" - "@alchemy.run/floci@2.0.0-beta.80"