From 7e50c319863b830da17611de03badaa9847d132b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A8=8B=E5=BA=8F=E5=91=98=E9=98=BF=E6=B1=9F-Relakkes?= Date: Sun, 4 Oct 2026 03:24:50 +0800 Subject: [PATCH] fix(desktop): composite the workspace browser with the app UI (#1437) The workspace browser was a native WebContentsView, which always paints above the DOM. Switching to the skills or settings page left the page floating over it, and menus over the page needed a screenshot swap that showed up clipped and flashed white. Pages are now guests kept in a layer inside the session panel that is never unmounted, so other pages, menus and dialogs draw over them like any element. The main process adopts each guest and keeps all in-page behaviour: navigation, history, find, zoom, capture, PDF, downloads, shortcuts and annotation. - Guard every attach in the main window: browser partition only, start at about:blank, preload and sandbox pinned, reported ids validated. - Match the native page: drop the blank history entry, keep page zoom independent of app zoom, allow popups so they still become tabs. - Keep app drags working over a page and close menus on a click into it. - Tell the side dock it is off screen when the session page is hidden. - Remove the overlay snapshot machinery and native bounds syncing. --- desktop/electron/ipc/capabilities.test.ts | 24 +- desktop/electron/ipc/capabilities.ts | 17 +- desktop/electron/ipc/channels.ts | 2 - desktop/electron/main.security.test.ts | 29 +- desktop/electron/main.ts | 92 ++- .../services/workspaceBrowser.test.ts | 731 ++++++++++-------- desktop/electron/services/workspaceBrowser.ts | 383 +++++---- .../services/workspaceBrowserGuest.test.ts | 127 +++ .../services/workspaceBrowserGuest.ts | 122 +++ .../electron/services/workspaceBrowserMenu.ts | 2 +- .../browser/computeWebviewBounds.test.ts | 19 - .../browser/computeWebviewBounds.ts | 13 - .../chat/AssistantMessage.images.test.tsx | 2 - .../chat/ImageGalleryModal.test.tsx | 75 -- .../src/components/chat/ImageGalleryModal.tsx | 11 - .../components/layout/ContentRouter.test.tsx | 33 + .../src/components/layout/ContentRouter.tsx | 29 +- .../layout/OpenProjectMenu.test.tsx | 21 - .../src/components/layout/OpenProjectMenu.tsx | 10 - .../workbench/WorkspaceAddMenu.test.tsx | 11 +- .../components/workbench/WorkspaceAddMenu.tsx | 2 - .../WorkspaceBrowserAddressBar.test.tsx | 12 +- .../workbench/WorkspaceBrowserAddressBar.tsx | 2 - .../workbench/WorkspaceBrowserGuestLayer.tsx | 24 + .../workbench/WorkspaceBrowserTab.test.tsx | 303 +++----- .../workbench/WorkspaceBrowserTab.tsx | 194 ++--- .../workbench/WorkspaceSurface.test.tsx | 33 +- .../components/workbench/WorkspaceSurface.tsx | 4 + desktop/src/hooks/useDismissable.test.tsx | 33 + desktop/src/hooks/useDismissable.ts | 20 + desktop/src/lib/desktopHost/browserHost.ts | 2 - desktop/src/lib/desktopHost/contract.test.ts | 4 +- .../src/lib/desktopHost/electronHost.test.ts | 19 +- desktop/src/lib/desktopHost/electronHost.ts | 5 +- desktop/src/lib/desktopHost/types.ts | 8 +- .../src/lib/workspace/browserGuestContract.ts | 21 + .../src/lib/workspace/browserGuests.test.ts | 282 +++++++ desktop/src/lib/workspace/browserGuests.ts | 293 +++++++ desktop/src/lib/workspace/browserHost.test.ts | 32 +- desktop/src/lib/workspace/browserHost.ts | 14 +- desktop/src/pages/ActiveSession.test.tsx | 72 ++ desktop/src/pages/ActiveSession.tsx | 6 + desktop/src/preview-agent/zoomControls.ts | 2 +- desktop/src/stores/overlayStore.test.ts | 58 -- desktop/src/stores/overlayStore.ts | 44 -- desktop/src/test/fakeBrowserGuests.ts | 49 ++ 46 files changed, 2044 insertions(+), 1247 deletions(-) create mode 100644 desktop/electron/services/workspaceBrowserGuest.test.ts create mode 100644 desktop/electron/services/workspaceBrowserGuest.ts delete mode 100644 desktop/src/components/browser/computeWebviewBounds.test.ts delete mode 100644 desktop/src/components/browser/computeWebviewBounds.ts create mode 100644 desktop/src/components/workbench/WorkspaceBrowserGuestLayer.tsx create mode 100644 desktop/src/lib/workspace/browserGuestContract.ts create mode 100644 desktop/src/lib/workspace/browserGuests.test.ts create mode 100644 desktop/src/lib/workspace/browserGuests.ts delete mode 100644 desktop/src/stores/overlayStore.test.ts delete mode 100644 desktop/src/stores/overlayStore.ts create mode 100644 desktop/src/test/fakeBrowserGuests.ts diff --git a/desktop/electron/ipc/capabilities.test.ts b/desktop/electron/ipc/capabilities.test.ts index 5b42d190..8be829eb 100644 --- a/desktop/electron/ipc/capabilities.test.ts +++ b/desktop/electron/ipc/capabilities.test.ts @@ -56,16 +56,17 @@ describe('Electron IPC capabilities', () => { ]) expect(validateElectronIpcPayload(channel, { ...payload, ...patch })).toBe(false) }) - it('accepts optional initial browser visibility without widening the create payload', () => { + it('requires the renderer guest id on create and accepts no geometry', () => { const channel = ELECTRON_IPC_CHANNELS.workspaceBrowserCreate - const identity = { tabId: 'wb-1', storageId: 'store-1' } + const identity = { tabId: 'wb-1', storageId: 'store-1', webContentsId: 7 } expect(validateElectronIpcPayload(channel, identity)).toBe(true) - expect(validateElectronIpcPayload(channel, { ...identity, visible: false })).toBe(true) - expect(validateElectronIpcPayload(channel, { ...identity, visible: true })).toBe(true) - for (const visible of ['false', 0, null, {}]) { - expect(validateElectronIpcPayload(channel, { ...identity, visible })).toBe(false) + expect(validateElectronIpcPayload(channel, { ...identity, url: 'https://example.com/' })).toBe(true) + for (const webContentsId of [undefined, 0, -1, 1.5, '7', Number.NaN, Number.MAX_SAFE_INTEGER + 1]) { + expect(validateElectronIpcPayload(channel, { ...identity, webContentsId })).toBe(false) } - expect(validateElectronIpcPayload(channel, { ...identity, visible: false, unknown: true })).toBe(false) + // Pages are drawn by the renderer now; the old native-view options are gone. + expect(validateElectronIpcPayload(channel, { ...identity, visible: false })).toBe(false) + expect(validateElectronIpcPayload(channel, { ...identity, bounds: { x: 0, y: 0, width: 1, height: 1 } })).toBe(false) }) it('has a validator for every exposed invoke channel', () => { @@ -74,15 +75,6 @@ describe('Electron IPC capabilities', () => { ) }) - it('limits presentation snapshots to a single browser page id', () => { - const channel = ELECTRON_IPC_CHANNELS.workspaceBrowserSnapshot - expect(validateElectronIpcPayload(channel, { tabId: 'wb-1' })).toBe(true) - for (const payload of [{}, { tabId: '' }, { tabId: 'wb-1', kind: 'full' }, { tabId: 'wb-1', url: 'https://example.com' }]) { - expect(validateElectronIpcPayload(channel, payload)).toBe(false) - } - expect(isElectronIpcChannelAllowedForPetWindow(channel)).toBe(false) - }) - it('rejects channels outside the desktop host contract', () => { expect(isElectronIpcChannel(ELECTRON_IPC_CHANNELS.appGetVersion)).toBe(true) expect(isElectronIpcChannel(ELECTRON_IPC_CHANNELS.appGetLocalePreference)).toBe(true) diff --git a/desktop/electron/ipc/capabilities.ts b/desktop/electron/ipc/capabilities.ts index ccd71db1..f8755eae 100644 --- a/desktop/electron/ipc/capabilities.ts +++ b/desktop/electron/ipc/capabilities.ts @@ -215,14 +215,17 @@ const workspaceBrowserTab: Validator = value => && hasOnlyKeys(value, ['tabId']) && isWorkspaceBrowserId(value.tabId) +// `webContentsId` names the renderer's `` guest. It is only a claim: +// the main process adopts it solely if it is a browser guest of the main window. const workspaceBrowserCreate: Validator = value => isRecord(value) - && hasOnlyKeys(value, ['tabId', 'storageId', 'url', 'bounds', 'visible']) + && hasOnlyKeys(value, ['tabId', 'storageId', 'url', 'webContentsId']) && isWorkspaceBrowserId(value.tabId) && isWorkspaceBrowserId(value.storageId) && (value.url === undefined || (typeof value.url === 'string' && value.url.length <= 8_192)) - && (value.bounds === undefined || boundsPayload(value.bounds)) - && (value.visible === undefined || typeof value.visible === 'boolean') + && typeof value.webContentsId === 'number' + && Number.isSafeInteger(value.webContentsId) + && value.webContentsId > 0 const workspaceBrowserMenuLabelKeys = ['find', 'print', 'zoom', 'zoomIn', 'zoomOut', 'zoomReset', 'capture', 'pickElement', 'downloads', 'history', 'openExternal'] @@ -256,12 +259,6 @@ const workspaceBrowserReload: Validator = value => && isWorkspaceBrowserId(value.tabId) && (value.ignoreCache === undefined || typeof value.ignoreCache === 'boolean') -const workspaceBrowserSetBounds: Validator = value => - isRecord(value) - && hasOnlyKeys(value, ['tabId', 'bounds']) - && isWorkspaceBrowserId(value.tabId) - && boundsPayload(value.bounds) - const workspaceBrowserSetVisible: Validator = value => isRecord(value) && hasOnlyKeys(value, ['tabId', 'visible']) @@ -402,13 +399,11 @@ export const ELECTRON_IPC_VALIDATORS = { [ELECTRON_IPC_CHANNELS.workspaceBrowserGoForward]: workspaceBrowserTab, [ELECTRON_IPC_CHANNELS.workspaceBrowserReload]: workspaceBrowserReload, [ELECTRON_IPC_CHANNELS.workspaceBrowserStop]: workspaceBrowserTab, - [ELECTRON_IPC_CHANNELS.workspaceBrowserSetBounds]: workspaceBrowserSetBounds, [ELECTRON_IPC_CHANNELS.workspaceBrowserSetVisible]: workspaceBrowserSetVisible, [ELECTRON_IPC_CHANNELS.workspaceBrowserSetZoom]: workspaceBrowserSetZoom, [ELECTRON_IPC_CHANNELS.workspaceBrowserFind]: workspaceBrowserFind, [ELECTRON_IPC_CHANNELS.workspaceBrowserStopFind]: workspaceBrowserTab, [ELECTRON_IPC_CHANNELS.workspaceBrowserCapture]: workspaceBrowserCapture, - [ELECTRON_IPC_CHANNELS.workspaceBrowserSnapshot]: workspaceBrowserTab, [ELECTRON_IPC_CHANNELS.workspaceBrowserMessage]: workspaceBrowserMessage, [ELECTRON_IPC_CHANNELS.workspaceBrowserPrintToPdf]: workspaceBrowserTab, [ELECTRON_IPC_CHANNELS.workspaceBrowserClose]: workspaceBrowserTab, diff --git a/desktop/electron/ipc/channels.ts b/desktop/electron/ipc/channels.ts index 7d95ce84..2be2b5b4 100644 --- a/desktop/electron/ipc/channels.ts +++ b/desktop/electron/ipc/channels.ts @@ -71,13 +71,11 @@ export const ELECTRON_IPC_CHANNELS = { workspaceBrowserGoForward: 'desktop:workspace-browser:go-forward', workspaceBrowserReload: 'desktop:workspace-browser:reload', workspaceBrowserStop: 'desktop:workspace-browser:stop', - workspaceBrowserSetBounds: 'desktop:workspace-browser:set-bounds', workspaceBrowserSetVisible: 'desktop:workspace-browser:set-visible', workspaceBrowserSetZoom: 'desktop:workspace-browser:set-zoom', workspaceBrowserFind: 'desktop:workspace-browser:find', workspaceBrowserStopFind: 'desktop:workspace-browser:stop-find', workspaceBrowserCapture: 'desktop:workspace-browser:capture', - workspaceBrowserSnapshot: 'desktop:workspace-browser:snapshot', workspaceBrowserMessage: 'desktop:workspace-browser:message', workspaceBrowserPrintToPdf: 'desktop:workspace-browser:print-to-pdf', workspaceBrowserClose: 'desktop:workspace-browser:close', diff --git a/desktop/electron/main.security.test.ts b/desktop/electron/main.security.test.ts index cf2e25d1..a9f83bee 100644 --- a/desktop/electron/main.security.test.ts +++ b/desktop/electron/main.security.test.ts @@ -7,6 +7,11 @@ import { createPreviewSessionPartition, isAllowlistedMainRendererMediaRequest, } from './services/previewSession' +import { applyWorkspaceBrowserAttachPolicy } from './services/workspaceBrowserGuest' +import { + WORKSPACE_BROWSER_INITIAL_SRC, + WORKSPACE_BROWSER_PARTITION, +} from '../src/lib/workspace/browserGuestContract' const desktopRoot = existsSync(path.resolve(process.cwd(), 'electron', 'main.ts')) ? process.cwd() @@ -80,9 +85,15 @@ describe('Electron preview security boundary', () => { }) it('locks workspace browser sandboxing on', () => { - expect(workspaceBrowserServiceSource).toContain('sandbox: true') - expect(workspaceBrowserServiceSource).toContain('contextIsolation: true') - expect(workspaceBrowserServiceSource).toContain('nodeIntegration: false') + // Pages are `` guests now; their preferences are pinned by the + // attach policy, which the main window must install. + const preferences: Record = { sandbox: false, contextIsolation: false, nodeIntegration: true } + expect(applyWorkspaceBrowserAttachPolicy(preferences, { + partition: WORKSPACE_BROWSER_PARTITION, + src: WORKSPACE_BROWSER_INITIAL_SRC, + }, { preload: '/preview-preload.cjs' })).toBe(true) + expect(preferences).toMatchObject({ sandbox: true, contextIsolation: true, nodeIntegration: false }) + expect(mainWindowSource).toContain('installWorkspaceBrowserGuestPolicy(mainWindow.webContents') }) it('lets only the main window host workspace browser pages', () => { @@ -132,9 +143,15 @@ describe('Electron preview security boundary', () => { }) it('locks workspace browser sandboxing on', () => { - expect(workspaceBrowserServiceSource).toContain('sandbox: true') - expect(workspaceBrowserServiceSource).toContain('contextIsolation: true') - expect(workspaceBrowserServiceSource).toContain('nodeIntegration: false') + // Pages are `` guests now; their preferences are pinned by the + // attach policy, which the main window must install. + const preferences: Record = { sandbox: false, contextIsolation: false, nodeIntegration: true } + expect(applyWorkspaceBrowserAttachPolicy(preferences, { + partition: WORKSPACE_BROWSER_PARTITION, + src: WORKSPACE_BROWSER_INITIAL_SRC, + }, { preload: '/preview-preload.cjs' })).toBe(true) + expect(preferences).toMatchObject({ sandbox: true, contextIsolation: true, nodeIntegration: false }) + expect(mainWindowSource).toContain('installWorkspaceBrowserGuestPolicy(mainWindow.webContents') }) it('lets only the main window host workspace browser pages', () => { diff --git a/desktop/electron/main.ts b/desktop/electron/main.ts index fcd528b3..e1711326 100644 --- a/desktop/electron/main.ts +++ b/desktop/electron/main.ts @@ -1,5 +1,5 @@ import { PublicAccessManager } from './services/publicAccess' -import { app, BrowserWindow, clipboard, dialog, ipcMain, Menu, nativeImage, nativeTheme, Notification, screen, session, systemPreferences, WebContentsView } from 'electron' +import { app, BrowserWindow, clipboard, dialog, ipcMain, Menu, nativeImage, nativeTheme, Notification, screen, session, systemPreferences, WebContentsView, webContents } from 'electron' import { autoUpdater } from 'electron-updater' import path from 'node:path' import { ELECTRON_EVENT_CHANNELS, ELECTRON_INTERNAL_CHANNELS, ELECTRON_IPC_CHANNELS, type ElectronIpcChannel } from './ipc/channels' @@ -33,11 +33,15 @@ import { ElectronPreviewService, type PreviewBounds } from './services/preview' import { ElectronWorkspaceBrowserService, WORKSPACE_BROWSER_PARTITION, - type WorkspaceBrowserBounds, type WorkspaceBrowserCaptureKind, type WorkspaceBrowserCreateOptions, type WorkspaceBrowserFindOptions, + type WorkspaceBrowserWebContentsLike, } from './services/workspaceBrowser' +import { + installWorkspaceBrowserGuestPolicy, + resolveWorkspaceBrowserGuest, +} from './services/workspaceBrowserGuest' import { configureLocalServerRequestAuth, configurePreviewSessionPermissions, @@ -113,6 +117,7 @@ let updaterService: ElectronUpdaterService | null = null let terminalService: ElectronTerminalService | null = null let previewService: ElectronPreviewService | null = null let workspaceBrowserService: ElectronWorkspaceBrowserService | null = null +let workspaceBrowserSessionConfigured = false let petWindowController: PetWindowController | null = null const traceWindows = new Map() let isQuitting = false @@ -445,10 +450,6 @@ function getWorkspaceBrowserService() { emit: event => { mainWindow?.webContents.send(ELECTRON_EVENT_CHANNELS.workspaceBrowserEvent, event) }, - resolveScaleFactor: parent => { - const bounds = parent.getBounds?.() - return bounds ? screen.getDisplayMatching(bounds).scaleFactor : 1 - }, writePdf: async ({ data, filename }) => { return saveWorkspaceBrowserPdf(data, async () => { if (!mainWindow || mainWindow.isDestroyed()) return null @@ -459,31 +460,37 @@ function getWorkspaceBrowserService() { return result.canceled ? null : result.filePath ?? null }) }, - createView: () => { - const view = new WebContentsView({ - webPreferences: { - preload: previewPreloadPath(), - // One shared persistent partition for every workspace page: a login in - // one tab has to still be there in the next one. Per-tab partitions - // would turn every new tab into a fresh, logged-out browser. - partition: WORKSPACE_BROWSER_PARTITION, - contextIsolation: true, - nodeIntegration: false, - sandbox: true, - }, - }) - // Same boundary as the singleton preview: OS permissions are denied, and - // `configureLocalServerRequestAuth` is deliberately NOT installed here. - // These pages render arbitrary remote sites, so attaching the desktop's - // local access token to their loopback requests would hand any visited - // site the local API. - configurePreviewSessionPermissions(view.webContents.session) - return view - }, + // Pages are `` guests the renderer creates; their preferences are + // fixed by `installWorkspaceBrowserGuestPolicy` on the main window. The id + // the renderer reports is only adopted if it names one of those guests. + resolveGuest: webContentsId => resolveWorkspaceBrowserGuest(webContentsId, { + fromId: id => webContents.fromId(id), + host: mainWindow?.webContents, + session: workspaceBrowserSession(), + }) as WorkspaceBrowserWebContentsLike, }) return workspaceBrowserService } +/** + * One shared persistent partition for every workspace page: a login in one + * tab has to still be there in the next one. Per-tab partitions would turn + * every new tab into a fresh, logged-out browser. + */ +function workspaceBrowserSession() { + const browserSession = session.fromPartition(WORKSPACE_BROWSER_PARTITION) + // Same boundary as the singleton preview: OS permissions are denied, and + // `configureLocalServerRequestAuth` is deliberately NOT installed here. + // These pages render arbitrary remote sites, so attaching the desktop's + // local access token to their loopback requests would hand any visited + // site the local API. + if (!workspaceBrowserSessionConfigured) { + workspaceBrowserSessionConfigured = true + configurePreviewSessionPermissions(browserSession) + } + return browserSession +} + async function listCustomPets() { const { pets, errors } = await loadCustomPetCatalog() return { pets, errors } @@ -830,11 +837,10 @@ function registerIpcHandlers() { registerHandler(ELECTRON_IPC_CHANNELS.previewClose, () => getPreviewService().close()) registerHandler(ELECTRON_IPC_CHANNELS.previewMessage, (event, payload) => getPreviewService().message(payload, event.sender)) registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserCreate, (event, payload) => { - // The service keeps a single parent window, so whichever renderer calls - // `create` last owns where every page is attached and detached. Trace and - // pet windows load the same preload, so without this a secondary window - // could adopt the pages and strand them as unremovable children of the main - // window. Same guard shape as `appSetLocalePreference`. + // Pages are guests of the main window's renderer. Trace and pet windows + // load the same preload, so without this a secondary window could register + // pages it does not host — and own the menu parent of every page. Same + // guard shape as `appSetLocalePreference`. if (!mainWindow || currentWindow(event) !== mainWindow) { throw new Error('Only the main window can host workspace browser pages') } @@ -864,10 +870,6 @@ function registerIpcHandlers() { }) registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserStop, (_event, payload) => getWorkspaceBrowserService().stop((payload as { tabId: string }).tabId)) - registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserSetBounds, (_event, payload) => { - const { tabId, bounds } = payload as { tabId: string, bounds: WorkspaceBrowserBounds } - return getWorkspaceBrowserService().setBounds(tabId, bounds) - }) registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserSetVisible, (_event, payload) => { const { tabId, visible } = payload as { tabId: string, visible: boolean } return getWorkspaceBrowserService().setVisible(tabId, visible) @@ -890,8 +892,6 @@ function registerIpcHandlers() { const { tabId, kind } = payload as { tabId: string, kind: WorkspaceBrowserCaptureKind } return getWorkspaceBrowserService().capture(tabId, kind) }) - registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserSnapshot, (_event, payload) => - getWorkspaceBrowserService().snapshot((payload as { tabId: string }).tabId)) registerHandler(ELECTRON_IPC_CHANNELS.workspaceBrowserMessage, (_event, payload) => { const { tabId, payload: message } = payload as { tabId: string, payload: unknown } return getWorkspaceBrowserService().message(tabId, message) @@ -913,7 +913,13 @@ function registerIpcHandlers() { app.quit() }) registerHandler(ELECTRON_IPC_CHANNELS.adaptersRestartSidecar, () => getServerRuntime().restartAdaptersSidecars()) - registerHandler(ELECTRON_IPC_CHANNELS.zoomSet, (event, payload) => currentWindow(event).webContents.setZoomFactor(normalizeZoomFactor(payload))) + registerHandler(ELECTRON_IPC_CHANNELS.zoomSet, (event, payload) => { + const window = currentWindow(event) + window.webContents.setZoomFactor(normalizeZoomFactor(payload)) + // Electron pushes the embedder's zoom into every browser guest; app zoom + // must not change the pages' own zoom. + if (window === mainWindow) workspaceBrowserService?.restorePageZoom() + }) registerHandler(ELECTRON_IPC_CHANNELS.appearanceSetApplied, (_event, payload) => { if (!isAppliedAppearance(payload)) return lastAppliedAppearance = payload @@ -942,8 +948,15 @@ async function createMainWindow() { contextIsolation: true, nodeIntegration: false, sandbox: true, + // Only for the workspace browser, whose pages must be composited with + // the DOM. Every attach is decided by the guest policy installed below. + webviewTag: true, }, }) + // Before any guest can exist: its session denies OS permissions, and its + // preferences are replaced with the sandboxed set the policy pins. + workspaceBrowserSession() + installWorkspaceBrowserGuestPolicy(mainWindow.webContents, { preload: previewPreloadPath() }) configureLocalServerRequestAuth( mainWindow.webContents.session.webRequest, resolveMainRendererServerAccess, @@ -1051,7 +1064,6 @@ app.whenReady().then(async () => { screen.on('display-metrics-changed', (_event, _display, changedMetrics) => { if (changedMetrics.includes('scaleFactor') || changedMetrics.includes('bounds')) { previewService?.refreshBounds() - workspaceBrowserService?.refreshBounds() } }) try { diff --git a/desktop/electron/services/workspaceBrowser.test.ts b/desktop/electron/services/workspaceBrowser.test.ts index e072b714..ce2af417 100644 --- a/desktop/electron/services/workspaceBrowser.test.ts +++ b/desktop/electron/services/workspaceBrowser.test.ts @@ -11,7 +11,6 @@ import { workspaceBrowserPdfFilename, type WorkspaceBrowserDownloadItemLike, type WorkspaceBrowserSessionLike, - type WorkspaceBrowserViewLike, type WorkspaceBrowserWebContentsLike, } from './workspaceBrowser' @@ -79,6 +78,7 @@ class FakeDownloadItem implements WorkspaceBrowserDownloadItemLike { } class FakeWebContents implements WorkspaceBrowserWebContentsLike { + id = 0 loadedUrls: string[] = [] scripts: string[] = [] zoomFactors: number[] = [] @@ -91,7 +91,8 @@ class FakeWebContents implements WorkspaceBrowserWebContentsLike { focused = false closed = 0 title = 'Page' - url = '' + // A `` guest attaches on the blank document the renderer starts it on. + url = 'about:blank' loading = false loadResult?: Promise history = { @@ -105,6 +106,10 @@ class FakeWebContents implements WorkspaceBrowserWebContentsLike { this.forwardEntries -= 1 this.backEntries += 1 }), + clear: vi.fn(() => { + this.backEntries = 0 + this.forwardEntries = 0 + }), } backEntries = 0 forwardEntries = 0 @@ -190,7 +195,14 @@ class FakeWebContents implements WorkspaceBrowserWebContentsLike { close() { this.closed += 1 + this.destroy() + } + + /** What removing the guest's element does: the guest is gone, and says so. */ + destroy() { + if (this.destroyed) return this.destroyed = true + this.emit('destroyed') } isDestroyed() { @@ -208,30 +220,10 @@ class FakeWebContents implements WorkspaceBrowserWebContentsLike { } } -class FakeView implements WorkspaceBrowserViewLike { - webContents = new FakeWebContents() - bounds: Array<{ x: number, y: number, width: number, height: number }> = [] - visible: boolean[] = [] - - setBounds(bounds: { x: number, y: number, width: number, height: number }) { - this.bounds.push(bounds) - } - - setVisible(visible: boolean) { - this.visible.push(visible) - if (!visible) this.webContents.focused = false - } -} - function fakeParent() { return { - webContents: { focus: vi.fn(), isDestroyed: vi.fn(() => false) }, + webContents: { isDestroyed: vi.fn(() => false) }, isDestroyed: vi.fn(() => false), - contentView: { - addChildView: vi.fn(), - removeChildView: vi.fn(), - }, - getBounds: () => ({ x: 0, y: 0, width: 1440, height: 900 }), } } @@ -251,51 +243,90 @@ const browserControls = { colors: { background: 'white', foreground: 'black', muted: 'gray', border: 'gray', hover: 'white', focus: 'blue', shadow: 'none' }, } +type OpenOptions = { storageId?: string, url?: string } + type Harness = { service: ElectronWorkspaceBrowserService parent: ReturnType - views: FakeView[] + /** Every guest the "renderer" created, in creation order. */ + guests: FakeWebContents[] events: WorkspaceBrowserEvent[] sharedSession: FakeSession - partitions: string[] pdfWrites: Array<{ data: Uint8Array, filename: string }> + /** Work the service scheduled for after the current native dispatch. */ + deferred: Array<() => void> + /** Stands in for the renderer creating a `` and registering it. */ + open(tabId: string, options?: OpenOptions): Promise + newGuest(): FakeWebContents + flushDeferred(): void } -function createHarness(options?: { scaleFactor?: number, platform?: NodeJS.Platform, cancelPdf?: boolean, loadResult?: Promise, menuFactory?: WorkspaceBrowserMenuFactory }): Harness { - const views: FakeView[] = [] +function createHarness(options?: { platform?: NodeJS.Platform, cancelPdf?: boolean, loadResult?: Promise, menuFactory?: WorkspaceBrowserMenuFactory, captureTimeoutMs?: number }): Harness { + const guests: FakeWebContents[] = [] const events: WorkspaceBrowserEvent[] = [] - const partitions: string[] = [] const pdfWrites: Array<{ data: Uint8Array, filename: string }> = [] + const deferred: Array<() => void> = [] // One shared session object stands in for `session.fromPartition(...)`, which // hands back the same session for the same partition string. const sharedSession = new FakeSession() + let nextId = 100 const service = new ElectronWorkspaceBrowserService({ previewScriptPath: previewScript(), emit: event => events.push(event), platform: options?.platform, menuFactory: options?.menuFactory, - resolveScaleFactor: () => options?.scaleFactor ?? 1, + defer: task => { deferred.push(task) }, + captureTimeoutMs: options?.captureTimeoutMs, writePdf: async input => { if (options?.cancelPdf) return null pdfWrites.push(input) return `/downloads/${input.filename}` }, - createView: () => { - partitions.push(WORKSPACE_BROWSER_PARTITION) - const view = new FakeView() - view.webContents.session = sharedSession - view.webContents.loadResult = options?.loadResult - views.push(view) - return view + // The real resolver also checks the guest's type, host and session; here a + // guest is valid exactly when the harness created it. + resolveGuest: id => { + const guest = guests.find(candidate => candidate.id === id && !candidate.destroyed) + if (!guest) throw new Error('not a workspace browser guest of the main window') + return guest }, }) - return { service, parent: fakeParent(), views, events, sharedSession, partitions, pdfWrites } + const parent = fakeParent() + const newGuest = () => { + const guest = new FakeWebContents() + guest.id = nextId++ + guest.session = sharedSession + guest.loadResult = options?.loadResult + guests.push(guest) + return guest + } + return { + service, + parent, + guests, + events, + sharedSession, + pdfWrites, + deferred, + newGuest, + open: async (tabId, open = {}) => { + const guest = newGuest() + await service.create(parent, tabId, { + storageId: open.storageId ?? `store-${tabId}`, + webContentsId: guest.id, + ...(open.url === undefined ? {} : { url: open.url }), + }) + return guest + }, + flushDeferred: () => { + for (const task of deferred.splice(0)) task() + }, + } } -function requireView(harness: Harness, index: number): FakeView { - const view = harness.views[index] - if (!view) throw new Error(`no fake view at index ${index}`) - return view +function requireGuest(harness: Harness, index: number): FakeWebContents { + const guest = harness.guests[index] + if (!guest) throw new Error(`no fake guest at index ${index}`) + return guest } afterEach(() => { @@ -307,25 +338,46 @@ afterEach(() => { const desktopRoot = fs.existsSync(path.resolve(process.cwd(), 'electron', 'main.ts')) ? process.cwd() : path.resolve(process.cwd(), 'desktop') -const workspaceBrowserHostSource = (() => { - const mainSource = fs.readFileSync(path.join(desktopRoot, 'electron', 'main.ts'), 'utf8') - return mainSource - .slice( - mainSource.indexOf('function getWorkspaceBrowserService()'), - mainSource.indexOf('async function listCustomPets()'), - ) - // Comments explain what is deliberately absent, so they must not count as - // the code being present. - .replace(/^\s*\/\/.*$/gm, '') -})() +const mainSource = fs.readFileSync(path.join(desktopRoot, 'electron', 'main.ts'), 'utf8') +const workspaceBrowserHostSource = mainSource + .slice( + mainSource.indexOf('function getWorkspaceBrowserService()'), + mainSource.indexOf('async function listCustomPets()'), + ) + // Comments explain what is deliberately absent, so they must not count as + // the code being present. + .replace(/^\s*\/\/.*$/gm, '') +const mainWindowSource = mainSource.slice( + mainSource.indexOf('async function createMainWindow()'), + mainSource.indexOf('if (!acquireSingleInstanceLock'), +) describe('Electron workspace browser host wiring', () => { it('shares one persistent partition and denies OS permissions on it', () => { - expect(workspaceBrowserHostSource).toContain('partition: WORKSPACE_BROWSER_PARTITION') - expect(workspaceBrowserHostSource).toContain('configurePreviewSessionPermissions') - expect(workspaceBrowserHostSource).toContain('contextIsolation: true') - expect(workspaceBrowserHostSource).toContain('nodeIntegration: false') - expect(workspaceBrowserHostSource).toContain('sandbox: true') + expect(workspaceBrowserHostSource).toContain('session.fromPartition(WORKSPACE_BROWSER_PARTITION)') + expect(workspaceBrowserHostSource).toContain('configurePreviewSessionPermissions(browserSession)') + }) + + it('adopts only guests that pass the main-window attach policy', () => { + // Sandboxing is pinned by the policy (see workspaceBrowserGuest.test.ts); + // here the wiring must install it before any page can exist. + expect(workspaceBrowserHostSource).toContain('resolveWorkspaceBrowserGuest') + expect(mainWindowSource).toContain('webviewTag: true') + const policy = mainWindowSource.indexOf('installWorkspaceBrowserGuestPolicy(mainWindow.webContents') + const session = mainWindowSource.indexOf('workspaceBrowserSession()') + const load = mainWindowSource.indexOf('loadRendererEntry(mainWindow!)') + expect(policy).toBeGreaterThan(-1) + expect(session).toBeGreaterThan(-1) + expect(policy).toBeLessThan(load) + expect(session).toBeLessThan(load) + }) + + it('keeps app zoom from changing the pages\' own zoom', () => { + const zoomHandler = mainSource.slice( + mainSource.indexOf('ELECTRON_IPC_CHANNELS.zoomSet'), + mainSource.indexOf('ELECTRON_IPC_CHANNELS.appearanceSetApplied'), + ) + expect(zoomHandler.indexOf('setZoomFactor')).toBeLessThan(zoomHandler.indexOf('restorePageZoom()')) }) it('never authenticates loopback requests made by a visited page', () => { @@ -341,74 +393,59 @@ describe('Electron workspace browser service', () => { it('keeps two pages alive at once and navigates only the addressed one', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { + await harness.open('tab-a', { storageId: 'store-a', url: 'https://a.example', }) - await harness.service.create(harness.parent, 'tab-b', { + await harness.open('tab-b', { storageId: 'store-b', url: 'https://b.example', }) await harness.service.navigate('tab-a', 'https://a.example/second') - expect(harness.views).toHaveLength(2) - expect(requireView(harness, 0).webContents.loadedUrls).toEqual([ + expect(harness.guests).toHaveLength(2) + expect(requireGuest(harness, 0).loadedUrls).toEqual([ 'https://a.example', 'https://a.example/second', ]) - expect(requireView(harness, 1).webContents.loadedUrls).toEqual(['https://b.example']) - expect(requireView(harness, 1).webContents.destroyed).toBe(false) + expect(requireGuest(harness, 1).loadedUrls).toEqual(['https://b.example']) + expect(requireGuest(harness, 1).destroyed).toBe(false) }) - it('gives every page the one shared persistent partition', async () => { - const harness = createHarness() - - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) - + it('names the one shared persistent partition every guest must use', () => { + // The attach policy admits only this partition; a per-tab partition would + // be the bug: `storageId` restores a page, it never forks the cookie jar. expect(WORKSPACE_BROWSER_PARTITION).toBe('persist:cc-haha-browser-app') - expect(harness.partitions).toEqual([ - WORKSPACE_BROWSER_PARTITION, - WORKSPACE_BROWSER_PARTITION, - ]) - // A per-tab partition would be the bug: `storageId` restores a page, it - // never forks the cookie jar. - expect(harness.partitions.some(partition => partition.includes('store-a'))).toBe(false) }) - it('hides a page by detaching it and never destroys it', async () => { + it('hides a page without ever destroying it', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - harness.service.setVisible('tab-a', false) - - expect(harness.parent.contentView.removeChildView).toHaveBeenCalledTimes(1) - expect(requireView(harness, 0).visible.at(-1)).toBe(false) - expect(requireView(harness, 0).webContents.closed).toBe(0) - expect(requireView(harness, 0).webContents.destroyed).toBe(false) - + await harness.open('tab-a') harness.service.setVisible('tab-a', true) - expect(harness.parent.contentView.addChildView).toHaveBeenCalledTimes(2) - expect(requireView(harness, 0).webContents.destroyed).toBe(false) + harness.service.setVisible('tab-a', false) + harness.service.setVisible('tab-a', true) + + expect(requireGuest(harness, 0).closed).toBe(0) + expect(requireGuest(harness, 0).destroyed).toBe(false) + await expect(harness.service.navigate('tab-a', 'https://a.example/next')).resolves.toBeUndefined() }) - it('accepts a redundant post-close hide but still rejects show and bounds for an absent page', async () => { + it('accepts a redundant post-close hide but still rejects showing an absent page', async () => { const h = createHarness() - await h.service.create(h.parent, 'closed', { storageId: 'closed' }) + await h.open('closed', { storageId: 'closed' }) h.service.close('closed') expect(() => h.service.setVisible('closed', false)).not.toThrow() expect(() => h.service.setVisible('closed', true)).toThrow('workspace browser tab not open') - expect(() => h.service.setBounds('closed', { x: 0, y: 0, width: 100, height: 100 })).toThrow('workspace browser tab not open') }) - it('registers an initially hidden page without attaching or obscuring the visible page', async () => { + it('registers a page as soon as its guest is adopted and reports the blank document as no page', async () => { const h = createHarness() - await h.service.create(h.parent, 'shown', { storageId: 'shown' }) - await h.service.create(h.parent, 'pending', { storageId: 'pending', visible: false }) - expect(h.parent.contentView.addChildView).toHaveBeenCalledTimes(1) - expect(requireView(h, 0).visible.at(-1)).toBe(true) - expect(requireView(h, 1).visible.at(-1)).toBe(false) - expect(h.events).toContainEqual(expect.objectContaining({ type: 'state', tabId: 'pending' })) + await h.open('pending', { storageId: 'pending' }) + // Until the first real navigation a tab has no address and no title; the + // attach document must not become a URL the tab would then "load". + expect(h.events).toContainEqual(expect.objectContaining({ type: 'state', tabId: 'pending', url: '', title: '' })) + expect(requireGuest(h, 0).loadedUrls).toEqual([]) }) it.each(['resolve', 'reject'] as const)('never resurrects a closed page when its initial load later %ss', async (outcome) => { @@ -416,105 +453,76 @@ describe('Electron workspace browser service', () => { let reject!: (error: Error) => void const loadResult = new Promise((done, fail) => { resolve = done; reject = fail }) const h = createHarness({ loadResult }) - const creation = h.service.create(h.parent, 'pending', { storageId: 'pending', url: 'https://slow.test/', visible: false }) - // Registration occurs before load settles, and Stop/geometry already work. + // Registration does not wait for the first load; Stop already works. + await h.open('pending', { storageId: 'pending', url: 'https://slow.test/' }) expect(h.events).toContainEqual(expect.objectContaining({ type: 'state', tabId: 'pending' })) - h.service.setBounds('pending', { x: 0, y: 0, width: 100, height: 100 }) h.service.stop('pending') h.service.close('pending') const eventCount = h.events.length - if (outcome === 'resolve') { - resolve() - await creation - } else { - reject(new Error('initial navigation failed')) - await expect(creation).rejects.toThrow('initial navigation failed') - } + if (outcome === 'resolve') resolve() + else reject(new Error('initial navigation failed')) + await new Promise(done => setImmediate(done)) expect(h.events).toHaveLength(eventCount) - expect(requireView(h, 0).webContents.closed).toBe(1) - expect(h.parent.contentView.addChildView).not.toHaveBeenCalled() + expect(requireGuest(h, 0).closed).toBe(1) expect(() => h.service.setVisible('pending', false)).not.toThrow() expect(() => h.service.setVisible('pending', true)).toThrow('workspace browser tab not open') }) - it('rejects a malformed initial visibility option before constructing any resource', async () => { + it('adopts nothing when the reported guest is not one it may adopt', async () => { const h = createHarness() - await expect(h.service.create(h.parent, 'bad', { storageId: 'bad', visible: 'no' as unknown as boolean })).rejects.toThrow('visible must be a boolean') - expect(h.views).toHaveLength(0) + await expect(h.service.create(h.parent, 'foreign', { storageId: 'foreign', webContentsId: 9_999 })) + .rejects.toThrow('not a workspace browser guest') + expect(h.events).toEqual([]) + expect(() => h.service.stop('foreign')).toThrow('workspace browser tab not open') }) - it('keeps registered resources retryable while propagating the initial navigation failure', async () => { + it('refuses to give one guest to two tabs or a second guest to a live tab', async () => { + const h = createHarness() + const guest = await h.open('tab-a') + await expect(h.service.create(h.parent, 'tab-b', { storageId: 'b', webContentsId: guest.id })) + .rejects.toThrow('already belongs to tab-a') + const other = h.newGuest() + await expect(h.service.create(h.parent, 'tab-a', { storageId: 'a', webContentsId: other.id })) + .rejects.toThrow('already has a live page') + expect(() => h.service.stop('tab-b')).toThrow('workspace browser tab not open') + }) + + it('keeps an adopted page retryable when its initial navigation fails', async () => { const h = createHarness({ loadResult: Promise.reject(new Error('initial navigation denied')) }) - await expect(h.service.create(h.parent, 'retry', { storageId: 'retry', url: 'https://retry.test/', visible: false })).rejects.toThrow('initial navigation denied') + // The failure reaches the tab as a `failed` event, like any navigation; + // `create` answers only whether the page was adopted. + await expect(h.open('retry', { storageId: 'retry', url: 'https://retry.test/' })).resolves.toBeDefined() expect(h.events).toContainEqual(expect.objectContaining({ type: 'state', tabId: 'retry' })) expect(() => h.service.reload('retry', { ignoreCache: true })).not.toThrow() - expect(requireView(h, 0).webContents.reloads).toEqual(['reload-ignoring-cache']) - expect(requireView(h, 0).webContents.destroyed).toBe(false) + expect(requireGuest(h, 0).reloads).toEqual(['reload-ignoring-cache']) + expect(requireGuest(h, 0).destroyed).toBe(false) }) - it('attaches only one page at a time so a shown page cannot sit under another', async () => { - const harness = createHarness() - - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) - - expect(harness.parent.contentView.removeChildView).toHaveBeenCalledWith(requireView(harness, 0)) - expect(requireView(harness, 0).visible.at(-1)).toBe(false) - expect(requireView(harness, 1).visible.at(-1)).toBe(true) - expect(requireView(harness, 0).webContents.destroyed).toBe(false) - }) - - it.each(['hide', 'close'] as const)('returns native input focus to the host before %s leaves no page responder', async (action) => { - const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - const contents = requireView(harness, 0).webContents - contents.focused = true - - if (action === 'hide') harness.service.setVisible('tab-a', false) - else harness.service.close('tab-a') - - // Hiding drops the native responder before the renderer's DOM focus can - // receive its next shortcut; test ownership before that native transition. - expect(contents.focused).toBe(false) - expect(harness.parent.webContents.focus).toHaveBeenCalledTimes(1) - }) - - it('does not steal native page B focus when page A is hidden or closed later', async () => { - const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) - requireView(harness, 1).webContents.focused = true - - harness.service.setVisible('tab-a', false) - harness.service.close('tab-a') - - expect(requireView(harness, 1).webContents.focused).toBe(true) - expect(harness.parent.webContents.focus).not.toHaveBeenCalled() - }) - - it('does not refocus a host-owned input or a destroyed parent during detach', async () => { - const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - harness.service.setVisible('tab-a', false) - expect(harness.parent.webContents.focus).not.toHaveBeenCalled() - - harness.service.setVisible('tab-a', true) - requireView(harness, 0).webContents.focused = true - harness.parent.isDestroyed.mockReturnValue(true) - harness.service.close('tab-a') - expect(harness.parent.webContents.focus).not.toHaveBeenCalled() + it('treats a guest destroyed behind its back as a closed page with a retry', async () => { + const h = createHarness() + const guest = await h.open('lost', { url: 'https://lost.test/' }) + await h.open('kept') + h.events.length = 0 + // The renderer lost the element (e.g. its layer was unmounted). + guest.destroy() + expect(h.events).toEqual([{ type: 'destroyed', tabId: 'lost', reason: 'closed' }]) + expect(() => h.service.stop('lost')).toThrow('workspace browser tab not open') + // A retry builds a new page for the same tab. + await h.open('lost', { url: 'https://lost.test/' }) + expect(requireGuest(h, 2).loadedUrls).toEqual(['https://lost.test/']) + expect(() => h.service.stop('kept')).not.toThrow() }) it('destroys only the closed page and leaves its neighbour intact', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) harness.service.close('tab-a') - expect(requireView(harness, 0).webContents.closed).toBe(1) - expect(requireView(harness, 1).webContents.closed).toBe(0) - expect(requireView(harness, 1).webContents.destroyed).toBe(false) + expect(requireGuest(harness, 0).closed).toBe(1) + expect(requireGuest(harness, 1).closed).toBe(0) + expect(requireGuest(harness, 1).destroyed).toBe(false) await expect(harness.service.navigate('tab-b', 'https://b.example/next')).resolves.toBeUndefined() expect(() => harness.service.stop('tab-a')).toThrow('workspace browser tab not open') }) @@ -522,9 +530,9 @@ describe('Electron workspace browser service', () => { it('drops native events that arrive after the tab was closed', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) - const closedContents = requireView(harness, 0).webContents + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) + const closedContents = requireGuest(harness, 0) harness.service.close('tab-a') harness.events.length = 0 @@ -535,15 +543,15 @@ describe('Electron workspace browser service', () => { expect(harness.events).toEqual([]) - requireView(harness, 1).webContents.emit('did-stop-loading') + requireGuest(harness, 1).emit('did-stop-loading') expect(harness.events.map(event => event.tabId)).toEqual(['tab-b']) }) it('denies native popups and reports them as new-window events instead', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - const result = requireView(harness, 0).webContents.windowOpenHandler?.({ + await harness.open('tab-a', { storageId: 'store-a' }) + const result = requireGuest(harness, 0).windowOpenHandler?.({ url: 'https://popup.example/page', }) @@ -558,42 +566,29 @@ describe('Electron workspace browser service', () => { it('blocks non-http navigation and rejects non-http loads outright', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) + await harness.open('tab-a', { storageId: 'store-a' }) const preventDefault = vi.fn() - requireView(harness, 0).webContents.emit('will-navigate', { preventDefault }, 'file:///etc/passwd') - requireView(harness, 0).webContents.emit('will-navigate', { preventDefault }, 'https://ok.example') + requireGuest(harness, 0).emit('will-navigate', { preventDefault }, 'file:///etc/passwd') + requireGuest(harness, 0).emit('will-navigate', { preventDefault }, 'https://ok.example') expect(preventDefault).toHaveBeenCalledTimes(1) await expect(harness.service.navigate('tab-a', 'javascript:alert(1)')).rejects.toThrow( 'unsupported url scheme', ) - expect(requireView(harness, 0).webContents.windowOpenHandler?.({ url: 'file:///etc/passwd' })) + expect(requireGuest(harness, 0).windowOpenHandler?.({ url: 'file:///etc/passwd' })) .toEqual({ action: 'deny' }) expect(harness.events.some(event => event.type === 'new-window')).toBe(false) }) - it('snaps bounds to physical pixels at fractional scale factors', async () => { - const harness = createHarness({ scaleFactor: 2.25 }) - - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - harness.service.setBounds('tab-a', { x: 1.1, y: 2.2, width: 10.3, height: 4.4 }) - - expect(requireView(harness, 0).bounds.at(-1)).toEqual({ - x: 0.888889, - y: 2.222222, - width: 10.666667, - height: 4.444444, - }) - }) - it('reads back and forward from the native navigation history', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { + await harness.open('tab-a', { storageId: 'store-a', url: 'https://a.example', }) - const webContents = requireView(harness, 0).webContents + const webContents = requireGuest(harness, 0) + webContents.emit('did-navigate', {}, 'https://a.example') // first commit webContents.backEntries = 1 harness.events.length = 0 webContents.emit('did-navigate', {}, 'https://a.example/second') @@ -614,8 +609,8 @@ describe('Electron workspace browser service', () => { it('reports main-frame load failures and crashes on the owning tab', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - const webContents = requireView(harness, 0).webContents + await harness.open('tab-a', { storageId: 'store-a' }) + const webContents = requireGuest(harness, 0) harness.events.length = 0 webContents.emit('did-fail-load', {}, -6, 'FILE_NOT_FOUND', 'https://a.example/missing', false) @@ -640,19 +635,19 @@ describe('Electron workspace browser service', () => { it('maps find and stop-find onto the native page search', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) + await harness.open('tab-a', { storageId: 'store-a' }) harness.service.find('tab-a', ' invoice ', { matchCase: true, findNext: true }) harness.service.stopFind('tab-a') harness.events.length = 0 - requireView(harness, 0).webContents.emit('found-in-page', {}, { + requireGuest(harness, 0).emit('found-in-page', {}, { activeMatchOrdinal: 2, matches: 7, }) - expect(requireView(harness, 0).webContents.finds).toEqual([ + expect(requireGuest(harness, 0).finds).toEqual([ { text: 'invoice', options: { matchCase: true, findNext: true } }, ]) - expect(requireView(harness, 0).webContents.stopFinds).toEqual(['clearSelection']) + expect(requireGuest(harness, 0).stopFinds).toEqual(['clearSelection']) expect(harness.events).toEqual([ { type: 'found', tabId: 'tab-a', activeMatchOrdinal: 2, matches: 7 }, ]) @@ -661,8 +656,8 @@ describe('Electron workspace browser service', () => { it('captures the viewport natively and the full page over CDP', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - const webContents = requireView(harness, 0).webContents + await harness.open('tab-a', { storageId: 'store-a' }) + const webContents = requireGuest(harness, 0) webContents.debugger = { isAttached: vi.fn(() => false), attach: vi.fn(), @@ -698,12 +693,12 @@ describe('Electron workspace browser service', () => { it('reports shared-session downloads on the tab that started them', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) harness.events.length = 0 const item = new FakeDownloadItem('report.pdf', 2_048) - harness.sharedSession.startDownload(item, requireView(harness, 1).webContents) + harness.sharedSession.startDownload(item, requireGuest(harness, 1)) item.advance(2_048, 'completed') expect(harness.events).toEqual([ @@ -737,11 +732,11 @@ describe('Electron workspace browser service', () => { it('exports a PDF through the host and reports it as a finished download', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { + await harness.open('tab-a', { storageId: 'store-a', url: 'https://a.example/report', }) - requireView(harness, 0).webContents.title = 'Quarterly Report' + requireGuest(harness, 0).title = 'Quarterly Report' harness.events.length = 0 await harness.service.printToPdf('tab-a') @@ -767,14 +762,14 @@ describe('Electron workspace browser service', () => { it('routes agent messages to the page that sent them and injects the agent after load', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) - requireView(harness, 1).webContents.emit('did-finish-load') + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) + requireGuest(harness, 1).emit('did-finish-load') await Promise.resolve() harness.events.length = 0 const owned = harness.service.handleMessageFromView( - requireView(harness, 1).webContents, + requireGuest(harness, 1), JSON.stringify({ v: 1, type: 'ready' }), ) const foreign = harness.service.handleMessageFromView({}, JSON.stringify({ v: 1, type: 'ready' })) @@ -785,31 +780,33 @@ describe('Electron workspace browser service', () => { expect(harness.events).toEqual([ { type: 'agent', tabId: 'tab-b', message: { v: 1, type: 'ready' } }, ]) - expect(requireView(harness, 1).webContents.scripts.some(script => + expect(requireGuest(harness, 1).scripts.some(script => script.includes('window.__previewInjected = true'))).toBe(true) }) it('forwards host messages only to the addressed page', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) await harness.service.message('tab-a', { v: 1, type: 'enter-picker' }) - expect(requireView(harness, 0).webContents.scripts.some(script => + expect(requireGuest(harness, 0).scripts.some(script => script.includes('enter-picker'))).toBe(true) - expect(requireView(harness, 1).webContents.scripts).toEqual([]) + expect(requireGuest(harness, 1).scripts).toEqual([]) }) it('keeps native floating zoom controls synchronized through menus, page changes and tab activation', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - await harness.service.create(harness.parent, 'b', { storageId: 'b' }) + await harness.open('a', { storageId: 'a' }) + await harness.open('b', { storageId: 'b' }) await harness.service.message('a', browserControls) await harness.service.message('b', browserControls) - const a = requireView(harness, 0).webContents - const b = requireView(harness, 1).webContents + // Only the page on screen may act as browser chrome. + harness.service.setVisible('b', true) + const a = requireGuest(harness, 0) + const b = requireGuest(harness, 1) const readConfig = (script: string) => JSON.parse(JSON.parse(script.slice(script.indexOf('(') + 1, -1))) as { type: string, zoomFactor: number } a.scripts.length = 0 b.scripts.length = 0 @@ -836,8 +833,8 @@ describe('Electron workspace browser service', () => { it('hides the native zoom capsule for a capture and restores it after a capture failure', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) page.capturePage.mockRejectedValueOnce(new Error('capture failed')) await expect(harness.service.capture('a', 'viewport')).rejects.toThrow('capture failed') expect(page.scripts).toEqual([ @@ -846,38 +843,22 @@ describe('Electron workspace browser service', () => { ]) }) - it('returns a presentation snapshot without emitting a composer screenshot or changing page lifetime', async () => { - const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents - harness.events.length = 0 - expect(await harness.service.snapshot('a')).toBe('data:image/png;base64,VIEWPORT') - expect(harness.events).toEqual([]) - expect(page.scripts).toEqual([ - 'globalThis.__PREVIEW_AGENT_SET_CHROME_HIDDEN__?.(true)', - 'globalThis.__PREVIEW_AGENT_SET_CHROME_HIDDEN__?.(false)', - ]) - expect(page.capturePage).toHaveBeenCalledTimes(1) - }) - - it('discards a presentation snapshot when its page navigates during capture', async () => { - const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents - let finish!: (image: Awaited>) => void - page.capturePage.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) - const snapshot = harness.service.snapshot('a') - await Promise.resolve() - page.emit('did-start-navigation', {}, 'https://next.example/', false, true) - finish({ toDataURL: () => 'data:image/png;base64,VIEWPORT' }) - await expect(snapshot).rejects.toThrow('changed during snapshot') + it('gives up on a viewport capture the compositor never answers', async () => { + // A guest that is not on screen has no frame; Chromium then takes ~30s to + // fail, and the page's zoom chrome would stay hidden all that time. + const harness = createHarness({ captureTimeoutMs: 5 }) + await harness.open('a') + const page = requireGuest(harness, 0) + page.capturePage.mockReturnValueOnce(new Promise(() => {})) + await expect(harness.service.capture('a', 'viewport')).rejects.toThrow('timed out') + expect(page.scripts.at(-1)).toBe('globalThis.__PREVIEW_AGENT_SET_CHROME_HIDDEN__?.(false)') expect(harness.events.some(event => event.type === 'screenshot')).toBe(false) }) it('keeps zoom chrome hidden until all overlapping native captures finish', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) let resolveFirst!: (image: Awaited>) => void page.capturePage.mockReturnValueOnce(new Promise(resolve => { resolveFirst = resolve })) const first = harness.service.capture('a', 'viewport') @@ -896,8 +877,8 @@ describe('Electron workspace browser service', () => { it.each([false, true])('exits the live page picker on same-document navigation (capture pending: %s)', async (capturing) => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) let finish!: (image: Awaited>) => void await harness.service.message('a', { v: 1, type: 'enter-picker', persistent: true }) if (capturing) { @@ -917,8 +898,8 @@ describe('Electron workspace browser service', () => { it('rejects delayed exits and selections after a newer generation, including missing identity after negotiation', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) const commandGeneration = () => { const script = page.scripts.filter(script => script.includes('enter-picker') || script.includes('exit-picker')).at(-1)! return JSON.parse(JSON.parse(script.match(/handleHostRaw\((.*)\)$/)![1]!)).generation as number @@ -942,8 +923,8 @@ describe('Electron workspace browser service', () => { it('preserves legacy v1 single selection and cancellation without a generation handshake', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) const select = () => harness.service.handleMessageFromView(page, '{"v":1,"type":"selection","payload":{"element":{"tag":"h1"}}}') await harness.service.message('a', { v: 1, type: 'enter-picker' }) select() @@ -959,8 +940,8 @@ describe('Electron workspace browser service', () => { it('does not let a delayed failed exit or its old event clear a newer mode', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) await harness.service.message('a', { v: 1, type: 'enter-picker', persistent: true }) const oldGeneration = pickerCommandGeneration(page) let rejectExit!: (error: Error) => void @@ -975,8 +956,8 @@ describe('Electron workspace browser service', () => { it('clears annotation mode if the page rejects a picker command', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) vi.spyOn(page, 'executeJavaScript').mockRejectedValueOnce(new Error('page unavailable')) await expect(harness.service.message('a', { v: 1, type: 'enter-picker', persistent: true })).rejects.toThrow('page unavailable') expect(harness.events.filter(event => event.type === 'state').at(-1)).toMatchObject({ annotationActive: false }) @@ -987,8 +968,8 @@ describe('Electron workspace browser service', () => { it('rearms persistent annotations after capture but keeps legacy picking one-shot', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) await harness.service.message('a', { v: 1, type: 'enter-picker', persistent: true, label: 1 }) const select = () => harness.service.handleMessageFromView(page, JSON.stringify({ v: 1, type: 'selection', generation: pickerCommandGeneration(page), payload: { element: { tag: 'h1' }, screenshot: { kind: 'region', captureId: 1 } } })) select() @@ -1011,8 +992,8 @@ describe('Electron workspace browser service', () => { it.each(['exit', 'navigate', 'new-picker'] as const)('does not rearm an old annotation after %s during capture', async (action) => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) let finish!: (image: Awaited>) => void page.capturePage.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) await harness.service.message('a', { v: 1, type: 'enter-picker', persistent: true }) @@ -1030,8 +1011,8 @@ describe('Electron workspace browser service', () => { it('returns the selection capture id to its own cleanup even when captures finish out of order', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) let resolveFirst!: (image: Awaited>) => void page.capturePage.mockReturnValueOnce(new Promise(resolve => { resolveFirst = resolve })) const select = async (captureId: number) => { @@ -1056,8 +1037,8 @@ describe('Electron workspace browser service', () => { it('does not clean a new document when an old selection capture finishes after navigation', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'a', { storageId: 'a' }) - const page = requireView(harness, 0).webContents + await harness.open('a', { storageId: 'a' }) + const page = requireGuest(harness, 0) let finishCapture!: (image: Awaited>) => void page.capturePage.mockReturnValueOnce(new Promise(resolve => { finishCapture = resolve })) await harness.service.message('a', { v: 1, type: 'enter-picker' }) @@ -1075,8 +1056,8 @@ describe('Electron workspace browser service', () => { it('records a visit log without standing in for the native back stack', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - const webContents = requireView(harness, 0).webContents + await harness.open('tab-a', { storageId: 'store-a' }) + const webContents = requireGuest(harness, 0) harness.events.length = 0 webContents.emit('did-navigate', {}, 'https://a.example/one') webContents.emit('did-navigate', {}, 'https://a.example/one') @@ -1096,84 +1077,73 @@ describe('Electron workspace browser service', () => { it('releases every page when the host tears the workspace down', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) harness.service.closeAll() - expect(requireView(harness, 0).webContents.closed).toBe(1) - expect(requireView(harness, 1).webContents.closed).toBe(1) + expect(requireGuest(harness, 0).closed).toBe(1) + expect(requireGuest(harness, 1).closed).toBe(1) expect(() => harness.service.stop('tab-b')).toThrow('workspace browser tab not open') }) - it('never re-navigates a live page when its tab is re-created', async () => { + it('never re-navigates a live page when its tab is registered again', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { + const guest = await harness.open('tab-a', { storageId: 'store-a', url: 'https://a.example', }) await harness.service.navigate('tab-a', 'https://a.example/checkout') - // The renderer re-mounts its surface every time the tab is re-activated and - // re-issues `create` with the tab's last known URL. Honouring that would be - // a hard navigation: the half-filled form, the scroll position and the real - // back stack would all be lost — which is the one thing keeping the page - // alive is supposed to prevent. + // The renderer registers a page again whenever it re-establishes it, with + // the tab's last known URL. Honouring that would be a hard navigation: the + // half-filled form, the scroll position and the real back stack would all + // be lost — which is the one thing keeping the page alive is supposed to + // prevent. await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a', url: 'https://a.example/checkout', + webContentsId: guest.id, }) - expect(harness.views).toHaveLength(1) - expect(requireView(harness, 0).webContents.loadedUrls).toEqual([ + expect(harness.guests).toHaveLength(1) + expect(requireGuest(harness, 0).loadedUrls).toEqual([ 'https://a.example', 'https://a.example/checkout', ]) }) - it('re-attaches a re-created page without loading anything', async () => { - const harness = createHarness() - - await harness.service.create(harness.parent, 'tab-a', { - storageId: 'store-a', - url: 'https://a.example', - }) - await harness.service.setVisible('tab-a', false) - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - - expect(requireView(harness, 0).visible).toEqual([true, false, true]) - expect(requireView(harness, 0).webContents.loadedUrls).toEqual(['https://a.example']) - }) - it('registers nothing when create is given arguments it cannot use', async () => { const harness = createHarness() + const guest = harness.newGuest() await expect(harness.service.create(harness.parent, 'tab-bad', { storageId: 'store-bad', url: 'file:///etc/passwd', + webContentsId: guest.id, })).rejects.toThrow() - // A view constructed before validation would be a `webContents` with no - // owner and no way to address it for closing. - expect(harness.views).toHaveLength(0) + // A page registered before validation would have no owner that could + // address it for closing. + expect(guest.loadedUrls).toEqual([]) expect(() => harness.service.stop('tab-bad')).toThrow('workspace browser tab not open') }) it('applies zoom and reload modes to the addressed page only', async () => { const harness = createHarness() - await harness.service.create(harness.parent, 'tab-a', { storageId: 'store-a' }) - await harness.service.create(harness.parent, 'tab-b', { storageId: 'store-b' }) + await harness.open('tab-a', { storageId: 'store-a' }) + await harness.open('tab-b', { storageId: 'store-b' }) harness.service.setZoom('tab-a', 1.25) harness.service.reload('tab-a', { ignoreCache: true }) harness.service.reload('tab-b') harness.service.stop('tab-b') - expect(requireView(harness, 0).webContents.zoomFactors).toEqual([1.25]) - expect(requireView(harness, 1).webContents.zoomFactors).toEqual([]) - expect(requireView(harness, 0).webContents.reloads).toEqual(['reload-ignoring-cache']) - expect(requireView(harness, 1).webContents.reloads).toEqual(['reload']) - expect(requireView(harness, 1).webContents.stops).toBe(1) + expect(requireGuest(harness, 0).zoomFactors).toEqual([1.25]) + expect(requireGuest(harness, 1).zoomFactors).toEqual([]) + expect(requireGuest(harness, 0).reloads).toEqual(['reload-ignoring-cache']) + expect(requireGuest(harness, 1).reloads).toEqual(['reload']) + expect(requireGuest(harness, 1).stops).toBe(1) }) }) @@ -1182,8 +1152,8 @@ describe('browser recovery boundaries', () => { it.each(['completed', 'cancelled', 'interrupted'])('reports %s after closing the source page or session', async (state) => { for (const closeAll of [false, true]) { const h = createHarness() - await h.service.create(h.parent, 'source', { storageId: 'source' }) - const contents = requireView(h, 0).webContents + await h.open('source', { storageId: 'source' }) + const contents = requireGuest(h, 0) const item = new FakeDownloadItem('large.zip', 2000) h.sharedSession.startDownload(item, contents) item.advance(100, 'progressing') @@ -1202,7 +1172,7 @@ describe('browser recovery boundaries', () => { it('does not report a completed PDF when the Save dialog is cancelled', async () => { const h = createHarness({ cancelPdf: true }) - await h.service.create(h.parent, 'source', { storageId: 'source' }) + await h.open('source', { storageId: 'source' }) await h.service.printToPdf('source') expect(h.pdfWrites).toHaveLength(0) expect(h.events.filter(event => event.type === 'download')).toHaveLength(0) @@ -1210,8 +1180,8 @@ describe('browser recovery boundaries', () => { it.each(['goBack', 'goForward', 'reload', 'navigate'] as const)('marks successful %s after an error with a new generation', async (operation) => { const h = createHarness() - await h.service.create(h.parent, 'source', { storageId: 'source' }) - const contents = requireView(h, 0).webContents + await h.open('source', { storageId: 'source' }) + const contents = requireGuest(h, 0) contents.emit('did-start-navigation', {}, 'https://bad.test/', false, true) contents.emit('did-fail-load', {}, -105, 'NAME_NOT_RESOLVED', 'https://bad.test/', true) contents.emit('did-stop-loading') @@ -1232,8 +1202,8 @@ describe('browser recovery boundaries', () => { it('ignores cancelled/subframe loads and tracks redirected navigation failure', async () => { const h = createHarness() - await h.service.create(h.parent, 'source', { storageId: 'source' }) - const contents = requireView(h, 0).webContents + await h.open('source', { storageId: 'source' }) + const contents = requireGuest(h, 0) contents.emit('did-start-navigation', {}, 'https://old.test/', false, true) contents.emit('did-start-navigation', {}, 'https://frame.test/', false, false) contents.emit('did-redirect-navigation', {}, 'https://new.test/', false, true) @@ -1245,8 +1215,12 @@ describe('browser recovery boundaries', () => { it.each(['darwin', 'win32', 'linux'] as const)('routes focused native shortcuts on %s without consuming page editing', async (platform) => { const h = createHarness({ platform }) - await h.service.create(h.parent, 'source', { storageId: 'source' }) - const contents = requireView(h, 0).webContents + await h.open('source', { storageId: 'source' }) + const contents = requireGuest(h, 0) + const unshown = vi.fn() + contents.emit('before-input-event', { preventDefault: unshown }, { type: 'keyDown', key: 'w', meta: platform === 'darwin', control: platform !== 'darwin', shift: false, alt: false }) + expect(unshown).not.toHaveBeenCalled() + h.service.setVisible('source', true) const input = { type: 'keyDown', key: 'w', meta: platform === 'darwin', control: platform !== 'darwin', shift: false, alt: false } for (const [key, action] of [['w', 'close-tab'], ['t', 'new-browser-tab'], ['j', 'toggle-bottom-panel']]) { const preventDefault = vi.fn() @@ -1270,10 +1244,10 @@ describe('browser recovery boundaries', () => { it('reports actual zoom after reattachment and across same-origin pages', async () => { const h = createHarness() - await h.service.create(h.parent, 'a', { storageId: 'a', url: 'https://same.test/a' }) - await h.service.create(h.parent, 'b', { storageId: 'b', url: 'https://same.test/b' }) - const a = requireView(h, 0).webContents - const b = requireView(h, 1).webContents + await h.open('a', { storageId: 'a', url: 'https://same.test/a' }) + await h.open('b', { storageId: 'b', url: 'https://same.test/b' }) + const a = requireGuest(h, 0) + const b = requireGuest(h, 1) // Emulate Chromium applying shared-origin zoom outside this service. a.zoomFactor = 1.5 b.zoomFactor = 1.5 @@ -1310,26 +1284,23 @@ function chooseMenu(h: ReturnType, index: number, action: st } describe('native browser popup lifetime', () => { - it('opens a native menu without hiding, resizing, navigating or replacing its live view', async () => { + it('opens a native menu without hiding, navigating or replacing its live page', async () => { const h = menuHarness() - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu', url: 'https://example.test/' }) - const view = requireView(h, 0) - const visible = [...view.visible] - const bounds = [...view.bounds] + await h.open('wb-menu', { storageId: 'menu', url: 'https://example.test/' }) + h.service.setVisible('wb-menu', true) + const guest = requireGuest(h, 0) const pending = h.service.showMenu(h.parent, 'wb-menu', menuOptions) expect(h.menus).toHaveLength(1) - expect(view.visible).toEqual(visible) - expect(view.bounds).toEqual(bounds) - expect(h.parent.contentView.removeChildView).not.toHaveBeenCalled() - expect(view.webContents.loadedUrls).toEqual(['https://example.test/']) - expect(h.views).toHaveLength(1) + expect(guest.loadedUrls).toEqual(['https://example.test/']) + expect(guest.destroyed).toBe(false) + expect(h.guests).toHaveLength(1) chooseMenu(h, 0, 'find') await expect(pending).resolves.toBe('find') }) it.each(['hide', 'close', 'closeAll'] as const)('cancels a pending popup on %s and ignores late selection', async action => { const h = menuHarness() - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu' }) + await h.open('wb-menu', { storageId: 'menu' }) const pending = h.service.showMenu(h.parent, 'wb-menu', menuOptions) if (action === 'hide') h.service.setVisible('wb-menu', false) else if (action === 'close') h.service.close('wb-menu') @@ -1342,7 +1313,7 @@ describe('native browser popup lifetime', () => { it('cancels the previous popup before opening another without letting its callback cancel the new popup', async () => { const h = menuHarness() - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu' }) + await h.open('wb-menu', { storageId: 'menu' }) const first = h.service.showMenu(h.parent, 'wb-menu', menuOptions) const second = h.service.showMenu(h.parent, 'wb-menu', menuOptions) await expect(first).resolves.toBeNull() @@ -1352,19 +1323,18 @@ describe('native browser popup lifetime', () => { await expect(second).resolves.toBe('history') }) - it('allows a registered hidden page to open its menu without attaching it', async () => { + it('allows a registered page that is not shown yet to open its menu', async () => { const h = menuHarness() - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu', visible: false }) + await h.open('wb-menu', { storageId: 'menu' }) const pending = h.service.showMenu(h.parent, 'wb-menu', menuOptions) - expect(h.parent.contentView.addChildView).not.toHaveBeenCalled() chooseMenu(h, 0, 'downloads') await expect(pending).resolves.toBe('downloads') }) it('uses the current native zoom for menu bounds and percentage instead of stale renderer state', async () => { const h = menuHarness() - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu' }) - requireView(h, 0).webContents.zoomFactor = 2 + await h.open('wb-menu', { storageId: 'menu' }) + requireGuest(h, 0).zoomFactor = 2 h.events.length = 0 const pending = h.service.showMenu(h.parent, 'wb-menu', menuOptions) expect(h.events).toEqual([expect.objectContaining({ type: 'state', tabId: 'wb-menu', zoomFactor: 2 })]) @@ -1377,10 +1347,89 @@ describe('native browser popup lifetime', () => { it('rejects missing pages, foreign owners and destroyed parents without constructing a menu', async () => { const h = menuHarness() await expect(h.service.showMenu(h.parent, 'missing', menuOptions)).rejects.toThrow('tab not open') - await h.service.create(h.parent, 'wb-menu', { storageId: 'menu' }) + await h.open('wb-menu', { storageId: 'menu' }) await expect(h.service.showMenu(fakeParent(), 'wb-menu', menuOptions)).rejects.toThrow('window') h.parent.isDestroyed.mockReturnValue(true) await expect(h.service.showMenu(h.parent, 'wb-menu', menuOptions)).rejects.toThrow('window') expect(h.menus).toHaveLength(0) }) }) + +// A `` guest differs from the old native page in three ways Electron +// imposes; each would be a visible regression if left alone. +describe('webview guest parity with the native page', () => { + it('does not let the first page go back to the blank document it attached with', async () => { + const h = createHarness() + const guest = await h.open('tab-a', { url: 'https://a.example/' }) + guest.backEntries = 1 // the attach document is a real history entry + guest.url = 'https://a.example/' + guest.emit('did-navigate', {}, guest.url) + expect(guest.history.clear).toHaveBeenCalledTimes(1) + expect(h.events.filter(event => event.type === 'state').at(-1)).toMatchObject({ canGoBack: false }) + + // Later navigations keep their real back stack. + guest.backEntries = 1 + guest.url = 'https://a.example/next' + guest.emit('did-navigate', {}, guest.url) + expect(guest.history.clear).toHaveBeenCalledTimes(1) + expect(h.events.filter(event => event.type === 'state').at(-1)).toMatchObject({ canGoBack: true }) + }) + + it('starts a page at 100% even though Electron attached it at the app zoom', async () => { + const h = createHarness() + const guest = h.newGuest() + guest.zoomFactor = 1.25 + await h.service.create(h.parent, 'tab-a', { storageId: 'a', webContentsId: guest.id }) + expect(guest.zoomFactor).toBe(1) + }) + + it('puts every page back at its own zoom when app zoom changes', async () => { + const h = createHarness() + const a = await h.open('a', { url: 'https://a.test/' }) + const b = await h.open('b', { url: 'https://b.test/' }) + a.url = 'https://a.test/' + b.url = 'https://b.test/' + h.service.setZoom('a', 1.5) + // Electron copies the embedder's zoom into every guest. + a.zoomFactor = 1.1 + b.zoomFactor = 1.1 + h.service.restorePageZoom() + expect(a.zoomFactor).toBe(1.5) + expect(b.zoomFactor).toBe(1) + }) + + it('restores the zoom Chromium would give a host after Electron resets it on navigation', async () => { + const h = createHarness() + const guest = await h.open('tab-a', { url: 'https://zoomed.test/' }) + guest.url = 'https://zoomed.test/' + h.service.setZoom('tab-a', 1.5) + + const navigate = (url: string, electronZoom: number) => { + guest.url = url + guest.emit('did-navigate', {}, url) + // Electron applies the embedder's zoom after `did-navigate` listeners + // return, which is why the service defers its correction. + guest.zoomFactor = electronZoom + h.flushDeferred() + } + navigate('https://other.test/', 1.1) + expect(guest.zoomFactor).toBe(1) + // Per host, not per page: a host's zoom survives leaving and returning, + // and does not depend on port or scheme. + navigate('http://zoomed.test:8080/again', 1.1) + expect(guest.zoomFactor).toBe(1.5) + expect(h.events.filter(event => event.type === 'state').at(-1)).toMatchObject({ zoomFactor: 1.5 }) + }) + + it('does not correct the zoom of a page closed before the deferred correction ran', async () => { + const h = createHarness() + const guest = await h.open('tab-a') + guest.url = 'https://late.test/' + guest.emit('did-navigate', {}, guest.url) + guest.zoomFactor = 1.1 + h.service.close('tab-a') + const before = guest.zoomFactors.length + h.flushDeferred() + expect(guest.zoomFactors).toHaveLength(before) + }) +}) diff --git a/desktop/electron/services/workspaceBrowser.ts b/desktop/electron/services/workspaceBrowser.ts index 8cc4f3a5..cb57842f 100644 --- a/desktop/electron/services/workspaceBrowser.ts +++ b/desktop/electron/services/workspaceBrowser.ts @@ -12,38 +12,29 @@ import type { } from '../../src/lib/desktopHost/types' import { parsePreviewAgentMessage, type PreviewAgentMessage } from '../ipc/previewMessage' import { parseHostMessage, type HostMessage } from '../../src/preview-agent/protocol' +import { WORKSPACE_BROWSER_INITIAL_SRC } from '../../src/lib/workspace/browserGuestContract' import { isHttpUrl } from './navigationGuards' -import { - normalizePreviewBounds, - normalizePreviewUrl, - resolvePreviewScriptPath, - snapPreviewBoundsToScaleFactor, - type PreviewBounds, -} from './preview' +import { normalizePreviewUrl, resolvePreviewScriptPath } from './preview' import { normalizeZoomFactor } from './zoom' import { WorkspaceBrowserMenuController, type WorkspaceBrowserMenuFactory } from './workspaceBrowserMenu' export type { WorkspaceBrowserCaptureKind, WorkspaceBrowserEvent, WorkspaceBrowserFindOptions } - -/** - * One persistent partition for every workspace browser page of this user. - * - * Codex does the same with `persist:codex-browser-app`: a login performed in - * one tab has to be there in the next one. The per-tab `storageId` is a *page - * restore identity* — which page to reopen after a restart — and must never be - * turned into a partition name, or every tab would get its own cookie jar. - */ -export const WORKSPACE_BROWSER_PARTITION = 'persist:cc-haha-browser-app' +export { WORKSPACE_BROWSER_PARTITION } from '../../src/lib/workspace/browserGuestContract' /** Mirrors the preview capture guard rails; a page controls these dimensions. */ const FULL_CAPTURE_MAX_EDGE = 16_384 const FULL_CAPTURE_MAX_PIXELS = 32_000_000 +/** + * A guest that is not on screen has no compositor frame, and `capturePage` + * then waits ~30s before failing. Without a bound, a capture racing a tab + * switch would keep the page's zoom chrome hidden for that long. + */ +const VIEWPORT_CAPTURE_TIMEOUT_MS = 5_000 + /** Visit log bound. The native back/forward stack is unaffected by this cap. */ const MAX_HISTORY_ENTRIES = 200 -export type WorkspaceBrowserBounds = PreviewBounds - type WorkspaceBrowserDebuggerLike = { isAttached(): boolean attach(protocolVersion?: string): void @@ -77,6 +68,7 @@ type WorkspaceBrowserNavigationHistoryLike = { canGoForward(): boolean goBack(): void goForward(): void + clear?(): void } export type WorkspaceBrowserWebContentsLike = { @@ -124,6 +116,7 @@ export type WorkspaceBrowserWebContentsLike = { event: 'will-navigate', handler: (event: { preventDefault: () => void }, url: string) => void, ): unknown + on(event: 'destroyed', handler: () => void): unknown navigationHistory?: WorkspaceBrowserNavigationHistoryLike canGoBack?(): boolean canGoForward?(): boolean @@ -140,49 +133,50 @@ export type WorkspaceBrowserWebContentsLike = { isFocused?(): boolean } -export type WorkspaceBrowserViewLike = { - webContents: WorkspaceBrowserWebContentsLike - setBounds(bounds: PreviewBounds): void - setVisible?(visible: boolean): void -} - export type WorkspaceBrowserParentWindowLike = { webContents?: { - focus(): void isDestroyed?(): boolean } isDestroyed?(): boolean - contentView: { - addChildView(view: unknown): void - removeChildView(view: unknown): void - } - getBounds?(): PreviewBounds } export type WorkspaceBrowserCreateOptions = { storageId: string url?: string - bounds?: WorkspaceBrowserBounds - visible?: boolean + /** The `` guest the renderer created for this page. */ + webContentsId: number } export type ElectronWorkspaceBrowserServiceOptions = { - createView: () => WorkspaceBrowserViewLike + /** + * Turns a renderer-reported id into the guest it names. It must throw for + * anything that is not a workspace browser webview of the main window. + */ + resolveGuest: (webContentsId: number) => WorkspaceBrowserWebContentsLike previewScriptPath: string emit: (event: WorkspaceBrowserEvent) => void - resolveScaleFactor?: (parent: WorkspaceBrowserParentWindowLike) => number /** Writes an exported PDF and resolves with the path it landed on. */ writePdf?: (input: { data: Uint8Array, filename: string }) => Promise platform?: NodeJS.Platform menuFactory?: WorkspaceBrowserMenuFactory + /** + * Runs a task after the current native dispatch. Electron applies its own + * navigation zoom after `did-navigate` listeners return, so a correction made + * inside the listener would be overwritten. + */ + defer?: (task: () => void) => void + /** Bounds a viewport capture; a hidden guest otherwise stalls for ~30s. */ + captureTimeoutMs?: number } type WorkspaceBrowserPage = { tabId: string storageId: string - view: WorkspaceBrowserViewLike - attached: boolean - requestedBounds: PreviewBounds | null + webContents: WorkspaceBrowserWebContentsLike + /** Whether the renderer currently shows this page. Only it can take input. */ + presented: boolean + /** The guest starts on a blank document whose entry must not be "Back". */ + initialEntryCleared: boolean zoomFactor: number controls: PreviewBrowserControlsMessage | null controlsSignature: string | null @@ -234,32 +228,45 @@ export function workspaceBrowserPdfFilename(url: string, title: string): string /** * Multi-page browser host. * - * The whole point of this service is that a page's lifetime is decided by - * `close(tabId)` and nothing else. Hiding a tab, re-bounding it, moving the - * panel or unmounting the React surface only change where — or whether — a page - * is drawn; the `webContents` behind it keeps its form state, scroll position - * and navigation history the entire time. + * Each page is a `` guest the renderer keeps in a layer that is never + * unmounted, so the page is composited with the DOM: menus, dialogs and other + * pages draw over it like over any element. This service adopts the guest and + * owns everything that happens *inside* it — navigation, history, find, zoom, + * capture, downloads and the annotation agent. + * + * A page's lifetime is still decided by `close(tabId)` and nothing else. + * Hiding a tab, moving the panel or unmounting the React surface only change + * whether a page is drawn; the guest keeps its form state, scroll position and + * navigation history the entire time. */ export class ElectronWorkspaceBrowserService { - private readonly createView: () => WorkspaceBrowserViewLike + private readonly resolveGuest: (webContentsId: number) => WorkspaceBrowserWebContentsLike private readonly previewScriptPath: string private readonly emit: (event: WorkspaceBrowserEvent) => void - private readonly resolveScaleFactor?: (parent: WorkspaceBrowserParentWindowLike) => number private readonly writePdf?: (input: { data: Uint8Array, filename: string }) => Promise private readonly platform: NodeJS.Platform + private readonly defer: (task: () => void) => void + private readonly captureTimeoutMs: number private readonly pages = new Map() private readonly hookedSessions = new Set() + /** + * Page zoom as Chromium keeps it: per host, shared by every page of the + * partition. Electron overwrites a guest's zoom with its embedder's whenever + * app zoom changes or the guest navigates, so this is the value to restore. + */ + private readonly zoomByHost = new Map() private parent: WorkspaceBrowserParentWindowLike | null = null private downloadSequence = 0 private readonly menu?: WorkspaceBrowserMenuController constructor(options: ElectronWorkspaceBrowserServiceOptions) { - this.createView = options.createView + this.resolveGuest = options.resolveGuest this.previewScriptPath = options.previewScriptPath this.emit = options.emit - this.resolveScaleFactor = options.resolveScaleFactor this.writePdf = options.writePdf this.platform = options.platform ?? process.platform + this.defer = options.defer ?? (task => { setImmediate(task) }) + this.captureTimeoutMs = options.captureTimeoutMs ?? VIEWPORT_CAPTURE_TIMEOUT_MS if (options.menuFactory) this.menu = new WorkspaceBrowserMenuController(options.menuFactory) } @@ -268,35 +275,38 @@ export class ElectronWorkspaceBrowserService { tabId: string, options: WorkspaceBrowserCreateOptions, ): Promise { - // Validate before anything is constructed or registered: `openPage` inserts - // a live view into `this.pages`, and a throw after that point strands a - // `webContents` that was never attached and can never be addressed again. - const bounds = options.bounds ? normalizePreviewBounds(options.bounds) : null + // Validate before anything is registered: a throw after `openPage` would + // leave a page this service can neither address nor release. const url = options.url ? normalizePreviewUrl(options.url) : null - if (options.visible !== undefined && typeof options.visible !== 'boolean') throw new Error('visible must be a boolean') + const webContents = this.resolveGuest(options.webContentsId) this.parent = parent const existing = this.pages.get(tabId) - // Re-creating a live id would strand its `webContents` with no way to close - // it, so an already-known tab keeps its page. - const page = existing ?? this.openPage(tabId, options) - if (bounds) page.requestedBounds = bounds - if (options.visible === false) { - this.detach(page) - // Registration is complete even while the first navigation is pending. - // The renderer may now safely send geometry, visibility and Stop. - this.emitState(page) - } else this.showExclusively(page) - // A live page is NEVER re-navigated from `create`. The renderer re-mounts - // this component every time its tab is re-activated, and `loadURL` on an - // existing `webContents` is a hard navigation: it would wipe the form the - // user had filled in, reset the scroll position and push a duplicate entry - // onto the native back stack — destroying exactly the state that keeping - // the page alive exists to preserve. Navigation is `navigate()`'s job. - if (url && !existing) { - await page.view.webContents.loadURL(url) + if (existing) { + // A live page is NEVER re-navigated from `create`. The renderer asks + // again whenever it has to re-establish a page, and `loadURL` on a live + // guest is a hard navigation: it would wipe the form the user filled in, + // reset the scroll position and push a duplicate back entry. + if (existing.webContents !== webContents) { + throw new Error(`workspace browser tab already has a live page: ${tabId}`) + } + this.emitState(existing) + return } + const owner = this.findPageByWebContents(webContents) + if (owner) throw new Error(`workspace browser guest already belongs to ${owner.tabId}`) + + const page = this.openPage(tabId, options.storageId, webContents) + // Resolving means "registered": the renderer may now send visibility, + // Stop and navigation. The first load is an ordinary navigation, so its + // failure reaches the tab as a `failed` event like any other — answering + // `create` with it would make an adopted page look unregistered. this.emitState(page) + if (url) { + void webContents.loadURL(url).catch(() => { + // Reported through `did-fail-load`, or superseded by a newer navigation. + }) + } } async navigate(tabId: string, url: string): Promise { @@ -304,7 +314,7 @@ export class ElectronWorkspaceBrowserService { page.pickerArmed = false page.persistentPicker = null page.pickerGeneration += 1 - await page.view.webContents.loadURL(normalizePreviewUrl(url)) + await page.webContents.loadURL(normalizePreviewUrl(url)) } async showMenu( @@ -317,7 +327,7 @@ export class ElectronWorkspaceBrowserService { throw new Error('Workspace browser menu requires its live owner window') } if (!this.menu) throw new Error('Workspace browser native menu unavailable') - const nativeZoom = page.view.webContents.getZoomFactor?.() + const nativeZoom = page.webContents.getZoomFactor?.() if (nativeZoom !== undefined && Number.isFinite(nativeZoom) && nativeZoom > 0 && nativeZoom !== page.zoomFactor) { page.zoomFactor = nativeZoom // Menu actions return to the renderer. Its next zoom step must start @@ -328,7 +338,7 @@ export class ElectronWorkspaceBrowserService { } goBack(tabId: string): void { - const webContents = this.requirePage(tabId).view.webContents + const webContents = this.requirePage(tabId).webContents if (webContents.navigationHistory) { webContents.navigationHistory.goBack() return @@ -337,7 +347,7 @@ export class ElectronWorkspaceBrowserService { } goForward(tabId: string): void { - const webContents = this.requirePage(tabId).view.webContents + const webContents = this.requirePage(tabId).webContents if (webContents.navigationHistory) { webContents.navigationHistory.goForward() return @@ -346,55 +356,69 @@ export class ElectronWorkspaceBrowserService { } reload(tabId: string, options?: { ignoreCache?: boolean }): void { - const webContents = this.requirePage(tabId).view.webContents + const webContents = this.requirePage(tabId).webContents if (options?.ignoreCache) webContents.reloadIgnoringCache() else webContents.reload() } stop(tabId: string): void { - this.requirePage(tabId).view.webContents.stop() - } - - setBounds(tabId: string, bounds: WorkspaceBrowserBounds): void { - const page = this.requirePage(tabId) - page.requestedBounds = normalizePreviewBounds(bounds) - this.applyBounds(page) + this.requirePage(tabId).webContents.stop() } /** - * Hiding detaches the native view from the window so it cannot cover a modal - * or steal clicks — it never destroys the page. + * Records whether the renderer shows this page. Drawing is the renderer's + * job; this decides which page may act as browser chrome (shortcuts, zoom + * capsule) and closes a native menu that belonged to a page going away. */ setVisible(tabId: string, visible: boolean): void { // Controller close may precede the component's passive unmount cleanup. // Only hide is an idempotent teardown; showing a missing page is still an error. if (!visible && !this.pages.has(tabId)) return const page = this.requirePage(tabId) - if (visible) this.showExclusively(page) - else this.detach(page) + page.presented = visible + if (!visible) { + this.menu?.cancel(page.tabId) + return + } + // Chromium may have changed this page's zoom while it was off screen (a + // same-host page zoomed); the controls it shows must read the real value. + this.emitState(page) } setZoom(tabId: string, factor: unknown): void { const page = this.requirePage(tabId) page.zoomFactor = normalizeZoomFactor(factor) - page.view.webContents.setZoomFactor?.(page.zoomFactor) + this.zoomByHost.set(zoomHostKey(page.webContents.getURL()), page.zoomFactor) + page.webContents.setZoomFactor?.(page.zoomFactor) // Chromium may apply zoom to another live page on the same origin. // Read every native value so controls never report an invented factor. for (const current of this.pages.values()) this.emitState(current) } + /** + * Electron copies the embedder's zoom into every guest when app zoom + * changes. App zoom never scaled the old native page, so put each page back + * at the zoom it had. Must run right after the embedder zoom changes. + */ + restorePageZoom(): void { + for (const page of this.pages.values()) { + if (page.closed || page.webContents.isDestroyed?.()) continue + page.webContents.setZoomFactor?.(page.zoomFactor) + } + } + find(tabId: string, text: string, options?: WorkspaceBrowserFindOptions): void { const trimmed = text.trim() const page = this.requirePage(tabId) if (!trimmed) { - page.view.webContents.stopFindInPage('clearSelection') + page.webContents.stopFindInPage('clearSelection') return } - page.view.webContents.findInPage(trimmed, options) + page.webContents.findInPage(trimmed, options) } stopFind(tabId: string): void { - this.requirePage(tabId).view.webContents.stopFindInPage('clearSelection') + this.requirePage(tabId).webContents.stopFindInPage('clearSelection') } async capture(tabId: string, kind: WorkspaceBrowserCaptureKind): Promise { @@ -403,17 +427,6 @@ export class ElectronWorkspaceBrowserService { this.emitFor(page, { type: 'screenshot', tabId: page.tabId, dataUrl, kind }) } - /** Presentation-only image: never enters the screenshot/chat event stream. */ - async snapshot(tabId: string): Promise { - const page = this.requirePage(tabId) - const navigationId = page.navigationId - const dataUrl = await this.captureDataUrl(page, 'viewport') - if (page.closed || navigationId !== page.navigationId) { - throw new Error('Browser page changed during snapshot') - } - return dataUrl - } - async message(tabId: string, payload: unknown): Promise { const page = this.requirePage(tabId) const controls = parseHostMessage(JSON.stringify(payload)) @@ -436,7 +449,7 @@ export class ElectronWorkspaceBrowserService { const raw = JSON.stringify(isHostPickerMessage(payload) ? { ...payload, generation: page.pickerGeneration } : payload) const generation = page.pickerGeneration try { - await page.view.webContents.executeJavaScript( + await page.webContents.executeJavaScript( `globalThis.__PREVIEW_BRIDGE__?.handleHostRaw(${JSON.stringify(raw)})`, ) } catch (error) { @@ -452,7 +465,7 @@ export class ElectronWorkspaceBrowserService { async printToPdf(tabId: string): Promise { const page = this.requirePage(tabId) - const webContents = page.view.webContents + const webContents = page.webContents if (!webContents.printToPDF || !this.writePdf) throw new Error('pdf export unavailable') const filename = workspaceBrowserPdfFilename(webContents.getURL(), webContents.getTitle()) const data = await webContents.printToPDF({ printBackground: true }) @@ -472,15 +485,19 @@ export class ElectronWorkspaceBrowserService { }) } - /** The only call that ends a page's life. */ + /** + * The only call that ends a page's life. The renderer removes the guest's + * element as well; closing here too means a page can never outlive its tab + * even if that removal never happens. + */ close(tabId: string): void { const page = this.pages.get(tabId) if (!page) return this.pages.delete(tabId) page.closed = true - this.detach(page) - if (!page.view.webContents.isDestroyed?.()) { - page.view.webContents.close?.() + this.menu?.cancel(page.tabId) + if (!page.webContents.isDestroyed?.()) { + page.webContents.close?.() } } @@ -490,11 +507,6 @@ export class ElectronWorkspaceBrowserService { this.parent = null } - /** Re-snaps every live page after a display scale-factor or bounds change. */ - refreshBounds(): void { - for (const page of this.pages.values()) this.applyBounds(page) - } - /** * Routes an in-page agent message. Returns whether this service owns the * sender, so the caller can fall through to the legacy singleton preview. @@ -506,14 +518,17 @@ export class ElectronWorkspaceBrowserService { return true } - private openPage(tabId: string, options: WorkspaceBrowserCreateOptions): WorkspaceBrowserPage { - const view = this.createView() + private openPage( + tabId: string, + storageId: string, + webContents: WorkspaceBrowserWebContentsLike, + ): WorkspaceBrowserPage { const page: WorkspaceBrowserPage = { tabId, - storageId: options.storageId, - view, - attached: false, - requestedBounds: null, + storageId, + webContents, + presented: false, + initialEntryCleared: false, zoomFactor: 1, controls: null, controlsSignature: null, @@ -532,12 +547,15 @@ export class ElectronWorkspaceBrowserService { } this.pages.set(tabId, page) this.installPageListeners(page) - this.hookSession(view.webContents.session) + this.hookSession(webContents.session) + // The guest attached at its embedder's zoom; a page starts at 100% or at + // whatever its host was last zoomed to, exactly as a fresh native page did. + this.restoreNavigationZoom(page) return page } private installPageListeners(page: WorkspaceBrowserPage): void { - const webContents = page.view.webContents + const webContents = page.webContents // Popups become tabs in this window instead of native child windows, which // would escape the workspace and the preload/permission boundary with it. @@ -548,9 +566,19 @@ export class ElectronWorkspaceBrowserService { webContents.on('will-navigate', (event, url) => { if (!isHttpUrl(url)) event.preventDefault() }) + webContents.on('destroyed', () => { + // A guest dies with its element. Unless the tab closed it, the renderer + // lost the page (its layer went away) and the tab must offer a retry + // instead of addressing a page that no longer exists. + if (page.closed || this.pages.get(page.tabId) !== page) return + this.pages.delete(page.tabId) + page.closed = true + this.menu?.cancel(page.tabId) + this.emit({ type: 'destroyed', tabId: page.tabId, reason: 'closed' }) + }) webContents.on('before-input-event', (event, input) => { - if (input.type !== 'keyDown' || !page.attached || page.closed) return + if (input.type !== 'keyDown' || !page.presented || page.closed) return const action = matchWorkspaceShortcut({ key: input.key, code: input.code, metaKey: input.meta, ctrlKey: input.control, shiftKey: input.shift, altKey: input.alt, @@ -597,8 +625,17 @@ export class ElectronWorkspaceBrowserService { page.pickerGeneration += 1 page.navigationUrl = url page.navigationCommitted = true + if (!page.initialEntryCleared && url !== WORKSPACE_BROWSER_INITIAL_SRC) { + // The blank document the guest attached with is a real history entry; + // a native page had none, so the first page must not be able to go + // "Back" to it. + page.initialEntryCleared = true + webContents.navigationHistory?.clear?.() + } this.recordVisit(page, url) this.emitState(page) + // Electron re-applies the embedder's zoom once these listeners return. + this.defer(() => this.restoreNavigationZoom(page)) }) webContents.on('did-navigate-in-page', (_event, url, isMainFrame) => { if (isMainFrame === false) return @@ -688,8 +725,8 @@ export class ElectronWorkspaceBrowserService { if (message.type === 'browser-zoom') { // Only the attached page can act as browser chrome. Background pages // cannot modify another tab, and no zoom event enters the chat pipeline. - if (page.attached && page.controls) { - const current = page.view.webContents.getZoomFactor?.() ?? page.zoomFactor + if (page.presented && page.controls) { + const current = page.webContents.getZoomFactor?.() ?? page.zoomFactor this.setZoom(page.tabId, message.action === 'reset' ? 1 : Math.round((current + (message.action === 'in' ? 0.1 : -0.1)) * 10) / 10) } return @@ -753,7 +790,7 @@ export class ElectronWorkspaceBrowserService { } private async clearSelectionOverlay(page: WorkspaceBrowserPage, captureId?: number): Promise { - const webContents = page.view.webContents + const webContents = page.webContents if (page.closed || webContents.isDestroyed?.()) return try { await webContents.executeJavaScript(`globalThis.__PREVIEW_AGENT_CLEAR_SELECTION_OVERLAY__?.(${captureId === undefined ? '' : JSON.stringify(captureId)})`) @@ -763,7 +800,7 @@ export class ElectronWorkspaceBrowserService { } private async injectPreviewAgent(page: WorkspaceBrowserPage): Promise { - const webContents = page.view.webContents + const webContents = page.webContents if (page.closed || webContents.isDestroyed?.()) return page.pickerArmed = false page.persistentPicker = null @@ -777,12 +814,12 @@ export class ElectronWorkspaceBrowserService { } private async syncBrowserControls(page: WorkspaceBrowserPage): Promise { - if (!page.controls || page.closed || page.view.webContents.isDestroyed?.()) return + if (!page.controls || page.closed || page.webContents.isDestroyed?.()) return const raw = JSON.stringify({ ...page.controls, zoomFactor: page.zoomFactor }) if (page.controlsSignature === raw) return page.controlsSignature = raw try { - await page.view.webContents.executeJavaScript(`globalThis.__PREVIEW_BRIDGE__?.handleHostRaw(${JSON.stringify(raw)})`) + await page.webContents.executeJavaScript(`globalThis.__PREVIEW_BRIDGE__?.handleHostRaw(${JSON.stringify(raw)})`) } catch (error) { if (page.controlsSignature === raw) page.controlsSignature = null throw error @@ -793,7 +830,7 @@ export class ElectronWorkspaceBrowserService { page: WorkspaceBrowserPage, kind: WorkspaceBrowserCaptureKind, ): Promise { - const webContents = page.view.webContents + const webContents = page.webContents const hideChrome = async (hidden: boolean) => { if (page.closed || webContents.isDestroyed?.()) return await webContents.executeJavaScript(`globalThis.__PREVIEW_AGENT_SET_CHROME_HIDDEN__?.(${hidden})`) @@ -803,7 +840,11 @@ export class ElectronWorkspaceBrowserService { await hideChrome(true) if (kind === 'full') return await this.captureFullPageDataUrl(page) if (!webContents.capturePage) throw new Error('native browser capture unavailable') - const image = await webContents.capturePage() + const image = await withTimeout( + webContents.capturePage(), + this.captureTimeoutMs, + 'browser capture timed out', + ) return image.toDataURL() } finally { page.captureCount -= 1 @@ -825,7 +866,7 @@ export class ElectronWorkspaceBrowserService { } private async captureFullPageDataUrlOnce(page: WorkspaceBrowserPage): Promise { - const debuggerApi = page.view.webContents.debugger + const debuggerApi = page.webContents.debugger if (!debuggerApi) throw new Error('full browser capture unavailable') let attachedHere = false @@ -876,42 +917,22 @@ export class ElectronWorkspaceBrowserService { } } - private showExclusively(page: WorkspaceBrowserPage): void { - // A second attached view would sit on top of the first and swallow its - // input, so exactly one page occupies the window's single browser slot. - for (const other of this.pages.values()) { - if (other !== page) this.detach(other) - } - if (this.parent && !page.attached) { - this.parent.contentView.addChildView(page.view) - page.attached = true - } - page.view.setVisible?.(true) - this.applyBounds(page) + /** + * Put a page back at the zoom Chromium would have given it as a native + * page: whatever its host was last zoomed to, otherwise 100%. Electron + * instead resets a guest to its embedder's zoom, i.e. the app zoom. + */ + private restoreNavigationZoom(page: WorkspaceBrowserPage): void { + const webContents = page.webContents + if (page.closed || webContents.isDestroyed?.() || !webContents.setZoomFactor) return + const wanted = this.zoomByHost.get(zoomHostKey(webContents.getURL())) ?? 1 + const actual = webContents.getZoomFactor?.() + // Factors round-trip through zoom levels, so compare with a tolerance. + if (actual !== undefined && Math.abs(actual - wanted) < 0.001) return + webContents.setZoomFactor(wanted) this.emitState(page) } - private detach(page: WorkspaceBrowserPage): void { - this.menu?.cancel(page.tabId) - // A DOM focus request cannot move macOS's native responder out of a - // WebContentsView. Capture ownership before hiding/removing the view drops - // it, and do not steal focus when a newer page or a host input already owns it. - const returnFocus = page.attached && !page.view.webContents.isDestroyed?.() && page.view.webContents.isFocused?.() - page.view.setVisible?.(false) - if (!page.attached) return - this.parent?.contentView.removeChildView(page.view) - page.attached = false - if (returnFocus && !this.parent?.isDestroyed?.() && !this.parent?.webContents?.isDestroyed?.()) { - this.parent?.webContents?.focus() - } - } - - private applyBounds(page: WorkspaceBrowserPage): void { - if (!page.requestedBounds || !this.parent) return - const scaleFactor = this.resolveScaleFactor?.(this.parent) ?? 1 - page.view.setBounds(snapPreviewBoundsToScaleFactor(page.requestedBounds, scaleFactor)) - } - private recordVisit(page: WorkspaceBrowserPage, url: string): void { // A visit log, not a back/forward stack: Electron's navigation entries carry // no timestamps, and the native history stays the source of truth for @@ -919,24 +940,27 @@ export class ElectronWorkspaceBrowserService { if (!isHttpUrl(url)) return const last = page.history[page.history.length - 1] if (last?.url === url) return - page.history.push({ url, title: page.view.webContents.getTitle(), visitedAt: Date.now() }) + page.history.push({ url, title: page.webContents.getTitle(), visitedAt: Date.now() }) if (page.history.length > MAX_HISTORY_ENTRIES) page.history.shift() this.emitFor(page, { type: 'history', tabId: page.tabId, entries: [...page.history] }) } private emitState(page: WorkspaceBrowserPage): void { - const webContents = page.view.webContents + const webContents = page.webContents if (webContents.isDestroyed?.()) return const actualZoom = webContents.getZoomFactor?.() if (actualZoom !== undefined && Number.isFinite(actualZoom) && actualZoom > 0) page.zoomFactor = actualZoom void this.syncBrowserControls(page).catch(error => { if (!page.closed) console.error('Failed to update workspace browser controls', error) }) + // Until its first real navigation the guest shows the blank document it + // attached with. A tab without a page has no address and no title. + const blank = webContents.getURL() === WORKSPACE_BROWSER_INITIAL_SRC this.emitFor(page, { type: 'state', tabId: page.tabId, - url: webContents.getURL(), - title: webContents.getTitle(), + url: blank ? '' : webContents.getURL(), + title: blank ? '' : webContents.getTitle(), canGoBack: readCanGoBack(webContents), canGoForward: readCanGoForward(webContents), loading: webContents.isLoading(), @@ -960,7 +984,7 @@ export class ElectronWorkspaceBrowserService { private findPageByWebContents(webContents: unknown): WorkspaceBrowserPage | null { if (!webContents) return null for (const page of this.pages.values()) { - if (page.view.webContents === webContents) return page + if (page.webContents === webContents) return page } return null } @@ -977,6 +1001,29 @@ export class ElectronWorkspaceBrowserService { } } +/** + * Chromium keys page zoom by host name alone — port and scheme do not count — + * or by the whole URL when there is no host (`net::GetHostOrSpecFromURL`). + */ +function zoomHostKey(url: string): string { + try { + const { hostname } = new URL(url) + return hostname ? hostname.replace(/\.$/, '') : url + } catch { + return url + } +} + +function withTimeout(promise: Promise, ms: number, message: string): Promise { + let timer: ReturnType | undefined + return Promise.race([ + promise, + new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(message)), ms) + }), + ]).finally(() => clearTimeout(timer)) +} + function readCanGoBack(webContents: WorkspaceBrowserWebContentsLike): boolean { return webContents.navigationHistory?.canGoBack() ?? webContents.canGoBack?.() ?? false } diff --git a/desktop/electron/services/workspaceBrowserGuest.test.ts b/desktop/electron/services/workspaceBrowserGuest.test.ts new file mode 100644 index 00000000..5ac816c0 --- /dev/null +++ b/desktop/electron/services/workspaceBrowserGuest.test.ts @@ -0,0 +1,127 @@ +import { describe, expect, it, vi } from 'vitest' +import { + WORKSPACE_BROWSER_INITIAL_SRC, + WORKSPACE_BROWSER_PARTITION, +} from '../../src/lib/workspace/browserGuestContract' +import { + applyWorkspaceBrowserAttachPolicy, + installWorkspaceBrowserGuestPolicy, + resolveWorkspaceBrowserGuest, +} from './workspaceBrowserGuest' + +const PRELOAD = '/app/electron-dist/preview-preload.cjs' +const browserAttach = { partition: WORKSPACE_BROWSER_PARTITION, src: WORKSPACE_BROWSER_INITIAL_SRC } + +describe('workspace browser attach policy', () => { + it('pins the sandboxed preferences and the preview preload, whatever the element asked for', () => { + // Everything a `` attribute or `webpreferences` string could + // request on its way here. + const requested: Record = { + preload: '/tmp/evil.js', + preloadURL: 'file:///tmp/evil.js', + nodeIntegration: true, + nodeIntegrationInSubFrames: true, + nodeIntegrationInWorker: true, + contextIsolation: false, + sandbox: false, + webSecurity: false, + allowRunningInsecureContent: true, + experimentalFeatures: true, + enableBlinkFeatures: 'Foo', + disableBlinkFeatures: 'Bar', + additionalArguments: ['--evil'], + plugins: true, + webviewTag: true, + zoomFactor: 3, + } + expect(applyWorkspaceBrowserAttachPolicy(requested, browserAttach, { preload: PRELOAD })).toBe(true) + expect(requested).toEqual({ + preload: PRELOAD, + nodeIntegration: false, + nodeIntegrationInSubFrames: false, + contextIsolation: true, + sandbox: true, + webSecurity: true, + allowRunningInsecureContent: false, + experimentalFeatures: false, + plugins: false, + webviewTag: false, + zoomFactor: 1, + }) + }) + + it.each([ + ['the renderer session', { partition: undefined, src: WORKSPACE_BROWSER_INITIAL_SRC }], + ['another partition', { partition: 'persist:other', src: WORKSPACE_BROWSER_INITIAL_SRC }], + ['a page the renderer picked', { partition: WORKSPACE_BROWSER_PARTITION, src: 'https://example.com/' }], + ['a local file', { partition: WORKSPACE_BROWSER_PARTITION, src: 'file:///etc/passwd' }], + ])('denies a guest for %s', (_name, params) => { + const preferences: Record = { nodeIntegration: true } + expect(applyWorkspaceBrowserAttachPolicy(preferences, params, { preload: PRELOAD })).toBe(false) + }) + + it('cancels a denied attach and confines every attached guest until it is adopted', () => { + const handlers = new Map void>() + installWorkspaceBrowserGuestPolicy({ + on: (event, handler) => { handlers.set(event, handler) }, + }, { preload: PRELOAD }) + const willAttach = handlers.get('will-attach-webview') as unknown as ( + event: { preventDefault(): void }, preferences: Record, params: unknown, + ) => void + const denied = { preventDefault: vi.fn() } + willAttach(denied, {}, { partition: 'persist:other', src: WORKSPACE_BROWSER_INITIAL_SRC }) + expect(denied.preventDefault).toHaveBeenCalledTimes(1) + const allowed = { preventDefault: vi.fn() } + willAttach(allowed, {}, browserAttach) + expect(allowed.preventDefault).not.toHaveBeenCalled() + + let openHandler: ((details: { url: string }) => unknown) | undefined + const navigateHandlers: Array<(event: { preventDefault(): void }, url: string) => void> = [] + const didAttach = handlers.get('did-attach-webview') as unknown as (event: unknown, guest: unknown) => void + didAttach({}, { + setWindowOpenHandler: (handler: (details: { url: string }) => unknown) => { openHandler = handler }, + on: (_event: string, handler: (event: { preventDefault(): void }, url: string) => void) => { navigateHandlers.push(handler) }, + }) + expect(openHandler?.({ url: 'https://popup.example/' })).toEqual({ action: 'deny' }) + const blocked = { preventDefault: vi.fn() } + navigateHandlers[0]!(blocked, 'file:///etc/passwd') + const allowedNavigation = { preventDefault: vi.fn() } + navigateHandlers[0]!(allowedNavigation, 'https://example.com/') + expect(blocked.preventDefault).toHaveBeenCalledTimes(1) + expect(allowedNavigation.preventDefault).not.toHaveBeenCalled() + }) +}) + +describe('workspace browser guest resolution', () => { + const host = { id: 1 } + const browserSession = { name: 'browser' } + const guest = (overrides: Partial<{ type: string, host: unknown, session: unknown, destroyed: boolean }> = {}) => ({ + isDestroyed: () => overrides.destroyed ?? false, + getType: () => overrides.type ?? 'webview', + hostWebContents: 'host' in overrides ? overrides.host : host, + session: 'session' in overrides ? overrides.session : browserSession, + }) + + it('adopts a webview of the main window in the browser partition', () => { + const page = guest() + expect(resolveWorkspaceBrowserGuest(7, { fromId: () => page, host, session: browserSession })).toBe(page) + }) + + it.each([ + ['a malformed id', 0, guest()], + ['a fractional id', 1.5, guest()], + ['a string id', '7', guest()], + ['a missing guest', 7, null], + ['a destroyed guest', 7, guest({ destroyed: true })], + // The renderer's own webContents carries the local access token. + ['the renderer itself', 7, guest({ type: 'window', host: null })], + ['another window’s webview', 7, guest({ host: { id: 2 } })], + ['a webview in another session', 7, guest({ session: { name: 'renderer' } })], + ])('refuses %s', (_name, id, candidate) => { + expect(() => resolveWorkspaceBrowserGuest(id, { + fromId: () => candidate, + host, + session: browserSession, + })).toThrow() + }) +}) diff --git a/desktop/electron/services/workspaceBrowserGuest.ts b/desktop/electron/services/workspaceBrowserGuest.ts new file mode 100644 index 00000000..4349c106 --- /dev/null +++ b/desktop/electron/services/workspaceBrowserGuest.ts @@ -0,0 +1,122 @@ +import { + WORKSPACE_BROWSER_INITIAL_SRC, + WORKSPACE_BROWSER_PARTITION, +} from '../../src/lib/workspace/browserGuestContract' +import { isHttpUrl } from './navigationGuards' + +/** + * Attach boundary for workspace browser `` guests. + * + * The main window enables `webviewTag` only so the workspace browser can be + * composited with the DOM: a native `WebContentsView` always paints above the + * page, which is what made menus, dialogs and other pages flicker behind or + * underneath it. In exchange, the renderer may now ask for a guest, so every + * attach is decided here, deny by default: + * + * - only the shared browser partition, starting from the fixed blank document; + * - every preference the element could have requested is replaced with the + * sandboxed set the old `WebContentsView` used, plus the preview preload the + * annotation agent relies on. + */ + +/** Keys the embedding element may set that must never reach a guest. */ +const STRIPPED_PREFERENCE_KEYS = [ + 'preloadURL', + 'enableBlinkFeatures', + 'disableBlinkFeatures', + 'additionalArguments', + 'nodeIntegrationInWorker', +] as const + +export type WorkspaceBrowserAttachParams = { + partition?: unknown + src?: unknown +} + +export function applyWorkspaceBrowserAttachPolicy( + webPreferences: Record, + params: WorkspaceBrowserAttachParams, + options: { preload: string }, +): boolean { + if (params.partition !== WORKSPACE_BROWSER_PARTITION) return false + if (params.src !== WORKSPACE_BROWSER_INITIAL_SRC) return false + for (const key of STRIPPED_PREFERENCE_KEYS) delete webPreferences[key] + Object.assign(webPreferences, { + preload: options.preload, + contextIsolation: true, + nodeIntegration: false, + nodeIntegrationInSubFrames: false, + sandbox: true, + webSecurity: true, + allowRunningInsecureContent: false, + experimentalFeatures: false, + plugins: false, + // A page cannot nest a guest of its own. + webviewTag: false, + // Electron starts a guest at its embedder's zoom. App zoom never applied to + // the old native view, so a page still starts at 100%. + zoomFactor: 1, + }) + return true +} + +export type WorkspaceBrowserGuestLike = { + isDestroyed(): boolean + getType(): string + hostWebContents?: unknown + session?: unknown +} + +export function resolveWorkspaceBrowserGuest( + webContentsId: unknown, + options: { + fromId: (id: number) => Guest | undefined | null + host: unknown + session: unknown + }, +): Guest { + if (typeof webContentsId !== 'number' || !Number.isSafeInteger(webContentsId) || webContentsId <= 0) { + throw new Error('invalid workspace browser guest id') + } + const guest = options.fromId(webContentsId) + if (!guest || guest.isDestroyed()) throw new Error('workspace browser guest is gone') + // The id travels through the renderer, so it is only a claim. Adopt nothing + // that is not a webview of the main window in the browser's own partition — + // least of all the renderer itself, whose session carries the local token. + if (guest.getType() !== 'webview' || guest.hostWebContents !== options.host || guest.session !== options.session) { + throw new Error('not a workspace browser guest of the main window') + } + return guest +} + +type AttachEvent = { preventDefault(): void } +type GuestBaseline = { + setWindowOpenHandler(handler: (details: { url: string }) => { action: 'deny' }): void + on(event: 'will-navigate', handler: (event: AttachEvent, url: string) => void): unknown +} + +/** Electron's `WebContents` declares each event as its own overload. */ +export type WorkspaceBrowserGuestHostLike = { + on(event: string, handler: (...args: never[]) => void): unknown +} + +export function installWorkspaceBrowserGuestPolicy( + host: WorkspaceBrowserGuestHostLike, + options: { preload: string }, +): void { + host.on('will-attach-webview', ( + event: AttachEvent, + webPreferences: Record, + params: WorkspaceBrowserAttachParams, + ) => { + if (!applyWorkspaceBrowserAttachPolicy(webPreferences, params, options)) event.preventDefault() + }) + host.on('did-attach-webview', (_event: unknown, guest: GuestBaseline) => { + // Until the browser service adopts it, a guest can neither open a window + // nor leave http(s). Adoption replaces the popup handler with its own. + guest.setWindowOpenHandler(() => ({ action: 'deny' })) + guest.on('will-navigate', (event, url) => { + if (!isHttpUrl(url)) event.preventDefault() + }) + }) +} diff --git a/desktop/electron/services/workspaceBrowserMenu.ts b/desktop/electron/services/workspaceBrowserMenu.ts index a98048d2..b4456e85 100644 --- a/desktop/electron/services/workspaceBrowserMenu.ts +++ b/desktop/electron/services/workspaceBrowserMenu.ts @@ -52,7 +52,7 @@ export function buildWorkspaceBrowserMenuTemplate( ] } -/** One popup belongs to one live page; dismissal never hides its WebContentsView. */ +/** One popup belongs to one live page; dismissal never hides or reloads that page. */ export class WorkspaceBrowserMenuController { private pending: { tabId: string; cancel: () => void } | null = null diff --git a/desktop/src/components/browser/computeWebviewBounds.test.ts b/desktop/src/components/browser/computeWebviewBounds.test.ts deleted file mode 100644 index 71762095..00000000 --- a/desktop/src/components/browser/computeWebviewBounds.test.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { describe, expect, it } from 'vitest' -import { computeWebviewBounds } from './computeWebviewBounds' - -describe('computeWebviewBounds', () => { - it('maps a DOMRect to logical bounds without rounding away high-DPI fractions', () => { - const rect = { left: 100.4, top: 50.6, width: 800.2, height: 600.9 } as DOMRect - expect(computeWebviewBounds(rect)).toEqual({ x: 100.4, y: 50.6, width: 800.2, height: 600.9 }) - }) - - it('maps zoomed CSS coordinates back to the native parent coordinate space', () => { - const rect = { left: 180, top: 150, width: 420, height: 300 } as DOMRect - expect(computeWebviewBounds(rect, 1.25)).toEqual({ x: 225, y: 187.5, width: 525, height: 375 }) - }) - - it('clamps negative/zero sizes to 0', () => { - const rect = { left: -5, top: -5, width: -10, height: 0 } as DOMRect - expect(computeWebviewBounds(rect)).toEqual({ x: -5, y: -5, width: 0, height: 0 }) - }) -}) diff --git a/desktop/src/components/browser/computeWebviewBounds.ts b/desktop/src/components/browser/computeWebviewBounds.ts deleted file mode 100644 index bb0f10a4..00000000 --- a/desktop/src/components/browser/computeWebviewBounds.ts +++ /dev/null @@ -1,13 +0,0 @@ -export type WebviewBounds = { x: number; y: number; width: number; height: number } - -export function computeWebviewBounds( - rect: Pick, - appZoom = 1, -): WebviewBounds { - return { - x: rect.left * appZoom, - y: rect.top * appZoom, - width: Math.max(0, rect.width * appZoom), - height: Math.max(0, rect.height * appZoom), - } -} diff --git a/desktop/src/components/chat/AssistantMessage.images.test.tsx b/desktop/src/components/chat/AssistantMessage.images.test.tsx index 25953277..fb42e95e 100644 --- a/desktop/src/components/chat/AssistantMessage.images.test.tsx +++ b/desktop/src/components/chat/AssistantMessage.images.test.tsx @@ -5,7 +5,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type { WorkspaceStatusResult } from '../../api/sessions' import { browserHost } from '../../lib/desktopHost/browserHost' import { openLocalFileWithSystem } from '../../lib/systemFileOpen' -import { useOverlayStore } from '../../stores/overlayStore' import { useSettingsStore } from '../../stores/settingsStore' import { useWorkspaceContentStore } from '../../stores/workspaceContentStore' import { AssistantMessage } from './AssistantMessage' @@ -50,7 +49,6 @@ beforeEach(() => { vi.spyOn(window, 'open').mockImplementation(() => null) apiGetBlob.mockReset().mockRejectedValue(new Error('404')) useSettingsStore.setState({ locale: 'en' }) - useOverlayStore.setState(useOverlayStore.getInitialState(), true) withWorkDir('/repo') vi.mocked(openLocalFileWithSystem).mockReset().mockResolvedValue(undefined) Reflect.deleteProperty(window, 'desktopHost') diff --git a/desktop/src/components/chat/ImageGalleryModal.test.tsx b/desktop/src/components/chat/ImageGalleryModal.test.tsx index eb899ae4..c0460aaa 100644 --- a/desktop/src/components/chat/ImageGalleryModal.test.tsx +++ b/desktop/src/components/chat/ImageGalleryModal.test.tsx @@ -5,7 +5,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { ImageGalleryModal } from './ImageGalleryModal' import { browserHost } from '../../lib/desktopHost/browserHost' import { openLocalFileWithSystem, reportOpenFailure } from '../../lib/systemFileOpen' -import { useOverlayStore } from '../../stores/overlayStore' import { useSettingsStore } from '../../stores/settingsStore' vi.mock('../../lib/systemFileOpen', () => ({ @@ -22,86 +21,12 @@ const gallery = [ ] const reset = () => { - useOverlayStore.setState(useOverlayStore.getInitialState(), true) useSettingsStore.setState({ locale: 'en' }) } beforeEach(reset) afterEach(reset) -describe('ImageGalleryModal · overlay suppression', () => { - it('increments overlay count while open and decrements on unmount', () => { - expect(useOverlayStore.getState().count).toBe(0) - - const { unmount } = render( - {}} - onSelect={() => {}} - />, - ) - expect(useOverlayStore.getState().count).toBe(1) - - unmount() - expect(useOverlayStore.getState().count).toBe(0) - }) - - it('does not increment when rendered with open=false', () => { - const { unmount } = render( - {}} - onSelect={() => {}} - />, - ) - expect(useOverlayStore.getState().count).toBe(0) - unmount() - expect(useOverlayStore.getState().count).toBe(0) - }) - - it('toggles count when open prop flips closed → open → closed', () => { - const { rerender, unmount } = render( - {}} - onSelect={() => {}} - />, - ) - expect(useOverlayStore.getState().count).toBe(0) - - rerender( - {}} - onSelect={() => {}} - />, - ) - expect(useOverlayStore.getState().count).toBe(1) - - rerender( - {}} - onSelect={() => {}} - />, - ) - expect(useOverlayStore.getState().count).toBe(0) - - unmount() - expect(useOverlayStore.getState().count).toBe(0) - }) -}) - describe('ImageGalleryModal · navigation', () => { it('names both arrows, which an icon-only control otherwise lacks', () => { render( {}} onSelect={() => {}} />) diff --git a/desktop/src/components/chat/ImageGalleryModal.tsx b/desktop/src/components/chat/ImageGalleryModal.tsx index a21569ad..c7cb1679 100644 --- a/desktop/src/components/chat/ImageGalleryModal.tsx +++ b/desktop/src/components/chat/ImageGalleryModal.tsx @@ -8,7 +8,6 @@ import { AuthedImage } from './AuthedImage' import { getDesktopHost } from '@/lib/desktopHost' import { isRootedLocalPath } from '@/lib/handlePreviewLink' import { openLocalFileWithSystem, reportOpenFailure } from '@/lib/systemFileOpen' -import { useOverlayStore } from '../../stores/overlayStore' import { useTranslation } from '../../i18n' type GalleryImage = { @@ -39,16 +38,6 @@ export function ImageGalleryModal({ open, images, activeIndex, onClose, onSelect const t = useTranslation() const activeImage = images[activeIndex] - // Native child webviews (e.g. the in-app browser preview) always render - // ABOVE the DOM, so this fullscreen overlay would be partially covered. - // Bump the overlay count while open so BrowserSurface can hide the webview. - useEffect(() => { - if (!open) return - const { push, pop } = useOverlayStore.getState() - push() - return () => pop() - }, [open]) - useEffect(() => { if (!open || images.length <= 1) return const handleKeyDown = (event: KeyboardEvent) => { diff --git a/desktop/src/components/layout/ContentRouter.test.tsx b/desktop/src/components/layout/ContentRouter.test.tsx index dcf4feb2..04aa8558 100644 --- a/desktop/src/components/layout/ContentRouter.test.tsx +++ b/desktop/src/components/layout/ContentRouter.test.tsx @@ -97,6 +97,39 @@ describe('ContentRouter tab surfaces', () => { expect(screen.getByTestId('terminal-host-__terminal__1')).toHaveAttribute('data-runtime-id', '__session_terminal__session-1') }) + it('keeps one browser page layer for the life of the window, inside the session panel', () => { + // A workspace browser page dies when its element leaves the DOM and + // reloads when it moves, so the layer holding them must never be + // re-created — not on a page switch, a task switch, or with no task open. + useTabStore.setState({ + tabs: [ + { sessionId: 'session-1', title: 'One', type: 'session', status: 'idle' }, + { sessionId: 'session-2', title: 'Two', type: 'session', status: 'idle' }, + { sessionId: MARKET_TAB_ID, title: 'Market', type: 'market', status: 'idle' }, + ], + activeTabId: 'session-1', + }) + render() + const layer = screen.getByTestId('workspace-browser-guest-layer') + // Same stacking context as the session content: it fades and goes inert + // with the panel when another page takes the window. + expect(screen.getByTestId('session-tab-panel')).toContainElement(layer) + + act(() => useTabStore.getState().setActiveTab(MARKET_TAB_ID)) + expect(screen.getByTestId('session-tab-panel')).toHaveAttribute('inert') + expect(screen.getByTestId('workspace-browser-guest-layer')).toBe(layer) + + act(() => useTabStore.getState().setActiveTab('session-2')) + expect(screen.getByTestId('workspace-browser-guest-layer')).toBe(layer) + + act(() => useTabStore.setState({ + tabs: [{ sessionId: MARKET_TAB_ID, title: 'Market', type: 'market', status: 'idle' }], + activeTabId: MARKET_TAB_ID, + })) + expect(screen.queryByTestId('active-session')).toBeNull() + expect(screen.getByTestId('workspace-browser-guest-layer')).toBe(layer) + }) + it('keeps terminal tabs mounted while chat content is active', () => { useTabStore.setState({ tabs: [ diff --git a/desktop/src/components/layout/ContentRouter.tsx b/desktop/src/components/layout/ContentRouter.tsx index 8e52ba2d..41c50fe3 100644 --- a/desktop/src/components/layout/ContentRouter.tsx +++ b/desktop/src/components/layout/ContentRouter.tsx @@ -10,6 +10,7 @@ import { TraceList } from '../../pages/TraceList' import { TraceSession } from '../../pages/TraceSession' import { SubagentRunPage, TeamMemberRunPage } from '../../pages/SubagentRunPage' import { AgentTeamsWorkbenchTab } from '../agentTeams/AgentTeamsWorkbenchTab' +import { WorkspaceBrowserGuestLayer } from '../workbench/WorkspaceBrowserGuestLayer' import { returnToTraceList } from '../../lib/traceNavigation' export function ContentRouter() { @@ -97,18 +98,24 @@ export function ContentRouter() { return (
- {retainedSessionId && ( -
+ {/* + Always rendered, even with no task to retain: the browser page layer + inside it must outlive every task switch, because a page whose element + is removed is destroyed and one that is moved reloads. + */} +
+ {retainedSessionId ? ( -
- )} + ) : null} + +
{page && (
{page} diff --git a/desktop/src/components/layout/OpenProjectMenu.test.tsx b/desktop/src/components/layout/OpenProjectMenu.test.tsx index 3f9d2c55..1b5ba1e2 100644 --- a/desktop/src/components/layout/OpenProjectMenu.test.tsx +++ b/desktop/src/components/layout/OpenProjectMenu.test.tsx @@ -50,11 +50,9 @@ vi.mock('../../stores/openTargetStore', () => ({ })) import { OpenProjectMenu } from './OpenProjectMenu' -import { useOverlayStore } from '../../stores/overlayStore' describe('OpenProjectMenu', () => { beforeEach(() => { - useOverlayStore.setState(useOverlayStore.getInitialState(), true) storeMocks.ensureTargets.mockReset() storeMocks.openTarget.mockReset() storeMocks.state = { @@ -109,25 +107,6 @@ describe('OpenProjectMenu', () => { expect(storeMocks.openTarget).toHaveBeenCalledWith('finder', '/repo') }) - it('suppresses the native browser preview while the target menu is open', async () => { - storeMocks.state.targets = [ - { id: 'vscode', kind: 'ide', label: 'VS Code', icon: 'vscode', platform: 'darwin' }, - { id: 'finder', kind: 'file_manager', label: 'Finder', icon: 'finder', platform: 'darwin' }, - ] - storeMocks.state.primaryTargetId = 'vscode' - - const { unmount } = render() - - expect(useOverlayStore.getState().count).toBe(0) - await act(async () => { - fireEvent.click(screen.getByRole('button', { name: 'Open project' })) - }) - expect(useOverlayStore.getState().count).toBe(1) - - unmount() - expect(useOverlayStore.getState().count).toBe(0) - }) - it('does not render without a path', () => { const { container } = render() expect(container).toBeEmptyDOMElement() diff --git a/desktop/src/components/layout/OpenProjectMenu.tsx b/desktop/src/components/layout/OpenProjectMenu.tsx index 02007c20..9c18c05e 100644 --- a/desktop/src/components/layout/OpenProjectMenu.tsx +++ b/desktop/src/components/layout/OpenProjectMenu.tsx @@ -3,7 +3,6 @@ import { createPortal } from 'react-dom' import { ChevronDown } from 'lucide-react' import { useTranslation } from '../../i18n' import { useOpenTargetStore } from '../../stores/openTargetStore' -import { useOverlayStore } from '../../stores/overlayStore' import { useDismissable } from '@/hooks/useDismissable' import { TargetIcon } from '@/components/composite/TargetIcon' @@ -29,15 +28,6 @@ export function OpenProjectMenu({ path }: Props) { void ensureTargets() }, [ensureTargets, path]) - // The native browser preview always renders above DOM portals. Suppress it - // while this menu is open so targets below the browser toolbar stay visible. - useEffect(() => { - if (!open) return - const { push, pop } = useOverlayStore.getState() - push() - return () => pop() - }, [open]) - const handleDismiss = useCallback(() => setOpen(false), []) useDismissable({ diff --git a/desktop/src/components/workbench/WorkspaceAddMenu.test.tsx b/desktop/src/components/workbench/WorkspaceAddMenu.test.tsx index 8ac4809f..31f6baf7 100644 --- a/desktop/src/components/workbench/WorkspaceAddMenu.test.tsx +++ b/desktop/src/components/workbench/WorkspaceAddMenu.test.tsx @@ -2,7 +2,6 @@ import { StrictMode, useRef, useState } from 'react' import { cleanup, fireEvent, render, screen, within } from '@testing-library/react' import '@testing-library/jest-dom' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { useOverlayStore } from '@/stores/overlayStore' import { WorkspaceAddMenu } from './WorkspaceAddMenu' const selected = vi.fn() @@ -21,7 +20,6 @@ function Harness({ last = false, disabled = false }: { last?: boolean; disabled? beforeEach(() => { selected.mockClear() - useOverlayStore.setState({ count: 0, snapshotCount: 0 }) }) afterEach(() => { cleanup(); vi.restoreAllMocks() }) @@ -120,16 +118,11 @@ describe('WorkspaceAddMenu', () => { expect(selected).not.toHaveBeenCalled() }) - it('selects one resource and balances native snapshot suppression in StrictMode', () => { - const { unmount } = render() + it('selects one resource exactly once in StrictMode', () => { + render() fireEvent.click(screen.getByText('Add')) - expect(useOverlayStore.getState()).toMatchObject({ count: 1, snapshotCount: 1 }) fireEvent.click(screen.getByTestId('workspace-menu-browser')) expect(selected).toHaveBeenCalledExactlyOnceWith('browser') expect(screen.queryByRole('menu')).toBeNull() - expect(useOverlayStore.getState()).toMatchObject({ count: 0, snapshotCount: 0 }) - fireEvent.click(screen.getByText('Add')) - unmount() - expect(useOverlayStore.getState()).toMatchObject({ count: 0, snapshotCount: 0 }) }) }) diff --git a/desktop/src/components/workbench/WorkspaceAddMenu.tsx b/desktop/src/components/workbench/WorkspaceAddMenu.tsx index c2dfed90..2252ecb2 100644 --- a/desktop/src/components/workbench/WorkspaceAddMenu.tsx +++ b/desktop/src/components/workbench/WorkspaceAddMenu.tsx @@ -4,7 +4,6 @@ import { useAnchoredPosition } from '@/hooks/useAnchoredPosition' import { useDismissable } from '@/hooks/useDismissable' import { useTranslation } from '@/i18n' import type { WorkspaceDock, WorkspaceTabKind } from '@/lib/workspace/types' -import { useSuppressBrowserOverlay } from '@/stores/overlayStore' import { useMenuKeyboard } from './menuKeyboard' import { WorkspaceLauncher } from './WorkspaceLauncher' @@ -24,7 +23,6 @@ export function WorkspaceAddMenu({ }: WorkspaceAddMenuProps) { const t = useTranslation() const menuRef = useRef(null) - useSuppressBrowserOverlay({ preserveSnapshot: true }) const position = useAnchoredPosition({ open: true, anchorRef, floatingRef: menuRef, placement: 'bottom-start', offset: 1, viewportMargin: 6, clampHeight: true, diff --git a/desktop/src/components/workbench/WorkspaceBrowserAddressBar.test.tsx b/desktop/src/components/workbench/WorkspaceBrowserAddressBar.test.tsx index 268ffafa..e22f584e 100644 --- a/desktop/src/components/workbench/WorkspaceBrowserAddressBar.test.tsx +++ b/desktop/src/components/workbench/WorkspaceBrowserAddressBar.test.tsx @@ -4,7 +4,6 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { createRef } from 'react' import { WorkspaceBrowserAddressBar } from '@/components/workbench/WorkspaceBrowserAddressBar' import { normalizeBrowserAddress } from '@/lib/workspace/browserAddress' -import { useOverlayStore } from '@/stores/overlayStore' import { useSettingsStore } from '@/stores/settingsStore' const visits = Array.from({ length: 8 }, (_, index) => ({ @@ -19,7 +18,6 @@ const props = { beforeEach(() => { vi.clearAllMocks() useSettingsStore.setState({ locale: 'en' }) - useOverlayStore.setState({ count: 0, snapshotCount: 0 }) }) afterEach(cleanup) @@ -63,7 +61,6 @@ describe('WorkspaceBrowserAddressBar', () => { fireEvent.change(address, { target: { value: 'in progress' } }) rerender() expect(address).toHaveValue('in progress') - expect(useOverlayStore.getState().count).toBe(0) act(() => address.blur()) rerender() act(() => address.focus()) @@ -73,19 +70,16 @@ describe('WorkspaceBrowserAddressBar', () => { expect(onNavigate).not.toHaveBeenCalled() }) - it('does not create actionable suggestions for an unsupported scheme and balances overlays on unmount', () => { - const { unmount } = render() + it('does not create actionable suggestions for an unsupported scheme', () => { + render() const address = screen.getByRole('combobox') act(() => address.focus()) - expect(useOverlayStore.getState().count).toBe(1) fireEvent.change(address, { target: { value: 'javascript:alert(1)' } }) expect(screen.queryByRole('listbox')).toBeNull() fireEvent.keyDown(address, { key: 'Enter' }) expect(onNavigate).not.toHaveBeenCalled() fireEvent.change(address, { target: { value: 'fixture' } }) - expect(useOverlayStore.getState().snapshotCount).toBe(1) - unmount() - expect(useOverlayStore.getState().count).toBe(0) + expect(screen.getByRole('listbox')).toBeInTheDocument() }) it('shows the new address only after navigation is accepted, allowing selection-discard confirmation first', () => { diff --git a/desktop/src/components/workbench/WorkspaceBrowserAddressBar.tsx b/desktop/src/components/workbench/WorkspaceBrowserAddressBar.tsx index 1fef10df..dad3ddf1 100644 --- a/desktop/src/components/workbench/WorkspaceBrowserAddressBar.tsx +++ b/desktop/src/components/workbench/WorkspaceBrowserAddressBar.tsx @@ -4,7 +4,6 @@ import { IconButton } from '@/components/ui/IconButton' import { useDismissable } from '@/hooks/useDismissable' import { useTranslation } from '@/i18n' import { normalizeBrowserAddress } from '@/lib/workspace/browserAddress' -import { useSuppressBrowserOverlay } from '@/stores/overlayStore' import type { WorkspaceBrowserVisit } from '@/stores/workspaceBrowserStore' type AddressSuggestion = { input: string; title: string; url?: string; action?: 'search' | 'visit' } @@ -186,7 +185,6 @@ function AddressSuggestions({ id, items, query, selectedIndex, onHighlight, onSe }) { const t = useTranslation() const listRef = useRef(null) - useSuppressBrowserOverlay({ preserveSnapshot: true }) useEffect(() => { listRef.current?.children[selectedIndex]?.scrollIntoView?.({ block: 'nearest' }) }, [selectedIndex, items]) diff --git a/desktop/src/components/workbench/WorkspaceBrowserGuestLayer.tsx b/desktop/src/components/workbench/WorkspaceBrowserGuestLayer.tsx new file mode 100644 index 00000000..db27f549 --- /dev/null +++ b/desktop/src/components/workbench/WorkspaceBrowserGuestLayer.tsx @@ -0,0 +1,24 @@ +import { useCallback } from 'react' +import { setWorkspaceBrowserGuestLayer } from '@/lib/workspace/browserGuests' + +/** + * Where every workspace browser page lives, for the life of the window. + * + * It must be mounted once and never re-parented: a `` reloads when + * moved and dies when removed. It sits after the session content in the same + * stacking context, so a page paints over the workspace like the placeholder it + * covers and under every dropdown, dialog and toast, and it fades with the + * session panel when another page takes the window. + */ +export function WorkspaceBrowserGuestLayer() { + const register = useCallback((element: HTMLDivElement | null) => { + setWorkspaceBrowserGuestLayer(element) + }, []) + return ( +
+ ) +} diff --git a/desktop/src/components/workbench/WorkspaceBrowserTab.test.tsx b/desktop/src/components/workbench/WorkspaceBrowserTab.test.tsx index b1e29198..1ee42bc9 100644 --- a/desktop/src/components/workbench/WorkspaceBrowserTab.test.tsx +++ b/desktop/src/components/workbench/WorkspaceBrowserTab.test.tsx @@ -19,13 +19,11 @@ const { host, isAvailable, releaseTab, openExternal, openPath } = vi.hoisted(() goForward: resolved(), reload: resolved(), stop: resolved(), - setBounds: resolved(), setVisible: resolved(), setZoom: resolved(), find: resolved(), stopFind: resolved(), capture: resolved(), - snapshot: vi.fn().mockResolvedValue('data:image/png;base64,BACKDROP'), message: resolved(), close: resolved(), printToPdf: resolved(), @@ -57,7 +55,7 @@ vi.mock('../../lib/desktopHost', async (importOriginal) => { }) import { WorkspaceBrowserTab } from './WorkspaceBrowserTab' -import { useOverlayStore } from '../../stores/overlayStore' +import { installFakeBrowserGuests, isBrowserPageShown } from '../../test/fakeBrowserGuests' import { useSettingsStore } from '../../stores/settingsStore' import { useWorkspaceBrowserStore } from '../../stores/workspaceBrowserStore' import { useWorkspaceStore } from '../../stores/workspaceStore' @@ -125,23 +123,34 @@ async function openMenuItem(action: WorkspaceBrowserMenuAction) { await act(async () => { fireEvent.click(screen.getByTestId('workspace-browser-menu-trigger')) }) } +function pageShown(tab: WorkspaceBrowserTabModel): boolean { + return isBrowserPageShown(tab.browserTabId) +} + +/** Lets an element attach, register with the host and report back. */ +async function flushGuests() { + for (let tick = 0; tick < 6; tick += 1) await act(async () => {}) +} + +let disposeGuests: () => void + beforeEach(() => { useSettingsStore.setState({ locale: 'en', uiZoom: 1 }) useWorkspaceStore.setState({ bySession: {}, sideWidth: 860, bottomHeight: 420 }) useWorkspaceBrowserStore.setState({ pageByTabId: {}, historyByTabId: {}, downloads: [] }) - useOverlayStore.setState({ count: 0, snapshotCount: 0 }) usePreviewSelectionStore.setState({ bySession: {} }) isAvailable.mockReturnValue(true) for (const mock of Object.values(host)) mock.mockReset().mockResolvedValue({ ok: true }) - host.snapshot.mockResolvedValue('data:image/png;base64,BACKDROP') host.showMenu.mockResolvedValue(null) releaseTab.mockClear() openExternal.mockClear() openPath.mockClear() + disposeGuests = installFakeBrowserGuests() }) afterEach(() => { cleanup() + disposeGuests() vi.unstubAllGlobals() vi.restoreAllMocks() }) @@ -166,14 +175,16 @@ describe('lifecycle readiness', () => { ) }) - it('focuses a newly created empty address bar once its native page is ready', async () => { + it('focuses a newly created empty address bar once its page is ready', async () => { const tab = openBrowserTab(null) const create = deferredCreate() host.create.mockReturnValueOnce(create.promise) render() const address = screen.getByTestId('workspace-browser-address') expect(address).toBeDisabled() + await flushGuests() await act(async () => { create.resolve({ ok: true }) }) + await flushGuests() expect(address).toBeEnabled() expect(address).toHaveFocus() }) @@ -185,61 +196,62 @@ describe('lifecycle readiness', () => { render(<>) const composer = screen.getByRole('textbox', { name: 'Chat composer' }) act(() => { composer.focus() }) + await flushGuests() await act(async () => { create.resolve({ ok: true }) }) + await flushGuests() expect(composer).toHaveFocus() }) - it('does not focus an existing loaded page address bar when snapshots open and close', async () => { + it('does not focus an existing loaded page address bar when it is hidden and shown again', async () => { const tab = openBrowserTab() - await renderReady(<>) + const view = await renderReady(<>) const composer = screen.getByRole('textbox', { name: 'Chat composer' }) act(() => { composer.focus() }) - await act(async () => { useOverlayStore.getState().push(true) }) - act(() => { useOverlayStore.getState().pop(true) }) + view.rerender(<>) + view.rerender(<>) expect(composer).toHaveFocus() }) - it('waits for registration before bounds, visibility or user commands', async () => { + it('registers the guest it created with the host, then shows it and takes commands', async () => { const creation = deferredCreate() host.create.mockReturnValueOnce(creation.promise) const tab = openBrowserTab() render() fireEvent.click(screen.getByRole('button', { name: 'Reload' })) - expect(host.setBounds).not.toHaveBeenCalled() + await flushGuests() + expect(host.create).toHaveBeenCalledWith(tab.browserTabId, { + storageId: tab.storageId, + webContentsId: expect.any(Number), + url: 'https://example.test/', + }) + expect(pageShown(tab)).toBe(false) expect(host.setVisible).not.toHaveBeenCalled() expect(host.reload).not.toHaveBeenCalled() await act(async () => creation.resolve({ ok: true })) - expect(host.setBounds).toHaveBeenCalledWith(tab.browserTabId, expect.any(Object)) + await flushGuests() + expect(pageShown(tab)).toBe(true) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) }) - it('uses registered state before a slow initial navigation finishes', async () => { - const creation = deferredCreate() - host.create.mockReturnValueOnce(creation.promise) - const tab = openBrowserTab() - render() - expect(host.setVisible).not.toHaveBeenCalled() - emit(pageState(tab.browserTabId, { loading: true })) - fireEvent.click(screen.getByRole('button', { name: 'Stop loading' })) - expect(host.stop).toHaveBeenCalledWith(tab.browserTabId) - await act(async () => creation.resolve({ ok: true })) - }) - it('surfaces failed creation and retries create before accepting navigation', async () => { const creation = deferredCreate() host.create.mockReturnValueOnce(creation.promise) const tab = openBrowserTab() const view = render() + await flushGuests() await act(async () => creation.reject(new Error('native creation denied'))) + await flushGuests() view.rerender() expect(screen.getByRole('alert')).toHaveTextContent('native creation denied') - expect(host.setBounds).not.toHaveBeenCalled() + expect(pageShown(tab)).toBe(false) expect(host.setVisible).not.toHaveBeenCalled() fireEvent.click(screen.getByRole('button', { name: 'Try again' })) - await waitFor(() => expect(host.create).toHaveBeenCalledTimes(2)) + await flushGuests() + expect(host.create).toHaveBeenCalledTimes(2) + // The refused guest was dropped; the retry registered a fresh one. + expect(host.create.mock.calls[1]![1].webContentsId).not.toBe(host.create.mock.calls[0]![1].webContentsId) expect(host.reload).not.toHaveBeenCalled() - await act(async () => {}) const address = screen.getByTestId('workspace-browser-address') fireEvent.change(address, { target: { value: 'https://retry.test/' } }) fireEvent.submit(address.closest('form')!) @@ -251,14 +263,15 @@ describe('lifecycle readiness', () => { host.create.mockReturnValueOnce(creation.promise) const tab = openBrowserTab() const view = render() - host.setBounds.mockClear() + await flushGuests() host.setVisible.mockClear() act(() => useWorkspaceStore.getState().closeTab(SESSION, tab.id)) await act(async () => { if (result === 'resolve') creation.resolve({ ok: true }) else creation.reject(new Error('closed while loading')) }) - expect(host.setBounds).not.toHaveBeenCalled() + await flushGuests() + expect(pageShown(tab)).toBe(false) expect(host.setVisible).not.toHaveBeenCalled() expect(useWorkspaceStore.getState().findBrowserTabOwner(tab.browserTabId)).toBeNull() view.unmount() @@ -271,16 +284,18 @@ describe('lifecycle readiness', () => { const first = openBrowserTab() const second = openBrowserTab('https://second.test/') const view = render() + await flushGuests() view.rerender() - await act(async () => {}) - host.setBounds.mockClear() + await flushGuests() host.setVisible.mockClear() await act(async () => { if (result === 'resolve') creation.resolve({ ok: true }) else creation.reject(new Error('old page failed')) }) - expect(host.setBounds).not.toHaveBeenCalled() - expect(host.setVisible).not.toHaveBeenCalled() + await flushGuests() + expect(pageShown(first)).toBe(false) + expect(pageShown(second)).toBe(true) + expect(host.setVisible).not.toHaveBeenCalledWith(first.browserTabId, expect.anything()) expect(currentTab(first.id).loadError).toBeNull() expect(currentTab(second.id).loadError).toBeNull() }) @@ -290,57 +305,43 @@ describe('lifecycle readiness', () => { host.create.mockReturnValueOnce(creation.promise) const tab = openBrowserTab() const view = render() - expect(host.create).toHaveBeenCalledWith(tab.browserTabId, expect.objectContaining({ visible: false })) + await flushGuests() view.unmount() await act(async () => creation.resolve({ ok: true })) - expect(host.setBounds).not.toHaveBeenCalled() + await flushGuests() + expect(pageShown(tab)).toBe(false) expect(host.setVisible).not.toHaveBeenCalled() expect(host.close).not.toHaveBeenCalled() }) - it('does not replay old resize callbacks into the next page identity', async () => { - const callbacks: ResizeObserverCallback[] = [] - vi.stubGlobal('ResizeObserver', class { - constructor(callback: ResizeObserverCallback) { callbacks.push(callback) } - observe() {} - disconnect() {} - }) + it('parks the previous page and draws the next one when the surface switches tabs', async () => { const first = openBrowserTab() const second = openBrowserTab('https://second.test/') - const view = render() - await act(async () => {}) - const oldCallback = callbacks[0]! + const view = await renderReady() + expect(pageShown(first)).toBe(true) view.rerender() - await act(async () => {}) - host.setBounds.mockClear() - act(() => oldCallback([], {} as ResizeObserver)) - expect(host.setBounds).not.toHaveBeenCalled() + await flushGuests() + expect(pageShown(first)).toBe(false) + expect(pageShown(second)).toBe(true) + // Parking is not closing: both pages are still alive. + expect(document.querySelectorAll('webview')).toHaveLength(2) + expect(host.close).not.toHaveBeenCalled() }) it('keeps an initial navigation failure retryable after registration', async () => { - const creation = deferredCreate() - host.create.mockReturnValueOnce(creation.promise) const tab = openBrowserTab() - const view = render() - emit(pageState(tab.browserTabId)) - await act(async () => creation.reject(new Error('ERR_CONNECTION_REFUSED'))) + const view = await renderReady() + // The host reports a failed first load as a `failed` event, which the + // event bridge writes onto the tab. + act(() => useWorkspaceStore.getState().updateBrowserTab(SESSION, tab.browserTabId, { loadError: 'ERR_CONNECTION_REFUSED' })) view.rerender() expect(screen.getByRole('alert')).toHaveTextContent('ERR_CONNECTION_REFUSED') + expect(pageShown(tab)).toBe(false) fireEvent.click(screen.getByRole('button', { name: 'Try again' })) expect(host.reload).toHaveBeenCalledWith(tab.browserTabId, { ignoreCache: true }) expect(host.create).toHaveBeenCalledTimes(1) }) - it('does not replace a completed initial navigation with its late promise rejection', async () => { - const creation = deferredCreate() - host.create.mockReturnValueOnce(creation.promise) - const tab = openBrowserTab() - render() - emit({ ...pageState(tab.browserTabId), navigationId: 1, navigationOutcome: 'succeeded' } as WorkspaceBrowserEvent) - await act(async () => creation.reject(new Error('late initial load rejection'))) - expect(currentTab(tab.id).loadError).toBeNull() - }) - it('does not replace successful navigation B with late rejected navigation A', async () => { const navigation = deferredCreate() host.navigate.mockReturnValueOnce(navigation.promise) @@ -357,7 +358,7 @@ describe('lifecycle readiness', () => { expect(currentTab(tab.id).loadError).toBeNull() }) - it('reports real navigation and geometry errors for a live page', async () => { + it('reports real navigation errors for a live page', async () => { const tab = openBrowserTab() await renderReady() host.navigate.mockRejectedValueOnce(new Error('navigation denied')) @@ -365,15 +366,12 @@ describe('lifecycle readiness', () => { fireEvent.change(address, { target: { value: 'https://denied.test/' } }) fireEvent.submit(address.closest('form')!) await waitFor(() => expect(currentTab(tab.id).loadError).toBe('navigation denied')) - host.setBounds.mockRejectedValueOnce(new Error('native geometry failed')) - act(() => window.dispatchEvent(new Event('resize'))) - await waitFor(() => expect(currentTab(tab.id).loadError).toBe('native geometry failed')) }) }) async function renderReady(ui: Parameters[0]) { const view = render(ui) - await act(async () => {}) + await flushGuests() return view } @@ -478,7 +476,7 @@ describe('native toolbar menu', () => { expect(screen.getByTestId('workspace-browser-find')).toBeInTheDocument() }) - it.each(['unmount', 'inactive', 'reactivate', 'close', 'replace', 'overlay'] as const)( + it.each(['unmount', 'inactive', 'reactivate', 'close', 'replace'] as const)( 'ignores a late menu action after %s', async (transition) => { const tab = openBrowserTab() const view = await renderReady() @@ -496,7 +494,6 @@ describe('native toolbar menu', () => { const next = openBrowserTab('https://replacement.test/') view.rerender() } - if (transition === 'overlay') act(() => { useOverlayStore.getState().push() }) await act(async () => { select('print') }) expect(host.printToPdf).not.toHaveBeenCalled() if (transition === 'replace' || transition === 'reactivate') { @@ -586,8 +583,7 @@ describe('page lifetime', () => { // the entire page. Native menus can cover the live guest without hiding it. expect(host.setVisible).not.toHaveBeenCalledWith(tab.browserTabId, false) expect(host.showMenu).toHaveBeenCalledWith(tab.browserTabId, expect.any(Object)) - expect(host.snapshot).not.toHaveBeenCalled() - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() + expect(pageShown(tab)).toBe(true) expect(screen.getByTestId('workspace-browser-menu-trigger')).toHaveAttribute('aria-expanded', 'true') await act(async () => { dismiss(null) }) expect(screen.getByTestId('workspace-browser-menu-trigger')).toHaveAttribute('aria-expanded', 'false') @@ -664,148 +660,63 @@ describe('page lifetime', () => { }) describe('visibility', () => { - it('captures a presentation backdrop before hiding the native page for a plus menu and restores the same page', async () => { + // The page is composited with the DOM, so nothing that draws over it has to + // hide it, snapshot it, or ask it to move — the bug class the native view had. + it('keeps the live page drawn under a dialog instead of hiding or snapshotting it', async () => { const tab = openBrowserTab() await renderReady() - let finish!: (url: string) => void - host.snapshot.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) + act(() => { + usePreviewSelectionStore.setState({ + bySession: { [tab.browserTabId]: { items: [{ id: 'one' }], nextNumber: 2 } }, + } as never) + }) host.setVisible.mockClear() - const createCount = host.create.mock.calls.length - - act(() => { useOverlayStore.getState().push(true) }) - expect(host.snapshot).toHaveBeenCalledWith(tab.browserTabId) - expect(host.setVisible).not.toHaveBeenCalledWith(tab.browserTabId, false) - await act(async () => { finish('data:image/png;base64,BACKDROP') }) - expect(screen.getByTestId('workspace-browser-backdrop')).toHaveAttribute('src', 'data:image/png;base64,BACKDROP') - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - expect(host.capture).not.toHaveBeenCalled() - - act(() => { useOverlayStore.getState().pop(true) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - expect(host.create).toHaveBeenCalledTimes(createCount) - expect(host.close).not.toHaveBeenCalled() + const address = screen.getByTestId('workspace-browser-address') + fireEvent.change(address, { target: { value: 'https://elsewhere.test/' } }) + fireEvent.submit(address.closest('form')!) + // Navigating away would discard annotations, so the page asks first. + expect(screen.getByRole('dialog')).toBeInTheDocument() + expect(pageShown(tab)).toBe(true) + expect(host.setVisible).not.toHaveBeenCalled() expect(host.navigate).not.toHaveBeenCalled() }) - it('does not hide or paint a stale capture after the plus menu has already closed', async () => { - const tab = openBrowserTab() - await renderReady() - let finish!: (url: string) => void - host.snapshot.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) - act(() => { useOverlayStore.getState().push(true) }) - act(() => { useOverlayStore.getState().pop(true) }) - host.setVisible.mockClear() - await act(async () => { finish('data:image/png;base64,STALE') }) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - expect(host.setVisible).not.toHaveBeenCalledWith(tab.browserTabId, false) - }) - - it('keeps menus usable after capture failure without turning it into a page load error', async () => { - const tab = openBrowserTab() - await renderReady() - host.snapshot.mockRejectedValueOnce(new Error('capture unavailable')) - await act(async () => { useOverlayStore.getState().push(true) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - expect(currentTab(tab.id).loadError).toBeFalsy() - act(() => { useOverlayStore.getState().pop(true) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) - }) - - it('lets an ordinary modal hide immediately while a snapshot is pending', async () => { - const tab = openBrowserTab() - await renderReady() - let finish!: (url: string) => void - host.snapshot.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) - act(() => { useOverlayStore.getState().push(true) }) - act(() => { useOverlayStore.getState().push() }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - await act(async () => { finish('data:image/png;base64,STALE') }) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - }) - - it('bounds capture waiting so a stalled native snapshot cannot trap the menu', async () => { - const tab = openBrowserTab() - await renderReady() - let finish!: (url: string) => void - host.snapshot.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) - vi.useFakeTimers() - try { - act(() => { useOverlayStore.getState().push(true) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) - await act(async () => { await vi.advanceTimersByTimeAsync(800) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - await act(async () => { finish('data:image/png;base64,TOO_LATE') }) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - act(() => { useOverlayStore.getState().pop(true) }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) - } finally { - vi.useRealTimers() - } - }) - - it('discards a pending backdrop when the browser tab is deactivated', async () => { - const tab = openBrowserTab() - const { rerender } = await renderReady() - let finish!: (url: string) => void - host.snapshot.mockReturnValueOnce(new Promise(resolve => { finish = resolve })) - act(() => { useOverlayStore.getState().push(true) }) - rerender() - await act(async () => { finish('data:image/png;base64,OTHER_TAB') }) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - expect(host.close).not.toHaveBeenCalled() - }) - - it('clears an already displayed backdrop when the underlying page navigates', async () => { - const tab = openBrowserTab() - await renderReady() - await act(async () => { useOverlayStore.getState().push(true) }) - expect(screen.getByTestId('workspace-browser-backdrop')).toBeInTheDocument() - emit({ type: 'state', tabId: tab.browserTabId, navigationId: 2, url: 'https://next.example/', title: '', loading: true, canGoBack: false, canGoForward: false }) - expect(screen.queryByTestId('workspace-browser-backdrop')).toBeNull() - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - }) - - it('attaches the page only while its tab is the active one', async () => { + it('draws the page only while its tab is the active one', async () => { const tab = openBrowserTab() const { rerender } = await renderReady() + expect(pageShown(tab)).toBe(true) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) rerender() + expect(pageShown(tab)).toBe(false) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) rerender() + expect(pageShown(tab)).toBe(true) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) + expect(host.create).toHaveBeenCalledTimes(1) }) - it('hides the page while a fullscreen DOM overlay is up', async () => { - // A native view always paints above the DOM, so an image modal opened over - // the workspace would otherwise be covered by the page. - const tab = openBrowserTab() - await renderReady() - - act(() => { useOverlayStore.getState().push() }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) - - act(() => { useOverlayStore.getState().pop() }) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) - }) - - it('hides the page while its own downloads overlay is open', async () => { - // Same reason, for the overlays this component draws itself: the downloads - // and history sheets are DOM, and the page would paint straight over them. + it('replaces the page with its own downloads sheet while that is open', async () => { const tab = openBrowserTab() await renderReady() await openMenuItem('downloads') expect(screen.getByTestId('workspace-browser-panel-downloads')).toBeInTheDocument() + expect(pageShown(tab)).toBe(false) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false) fireEvent.click(screen.getByRole('button', { name: 'Close' })) + expect(pageShown(tab)).toBe(true) expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) }) + + it('shows the empty state, not a blank page, for a tab without an address', async () => { + const tab = openBrowserTab(null) + await renderReady() + expect(pageShown(tab)).toBe(false) + expect(document.querySelectorAll('webview')).toHaveLength(1) + }) }) describe('navigation controls', () => { @@ -1123,7 +1034,6 @@ describe('browser address suggestions', () => { expect(options[0]).toHaveTextContent('ChatCut latest') expect(options[1]).toHaveTextContent('Chain Sheet') expect(screen.getByTestId('workspace-browser-address')).toHaveAttribute('aria-expanded', 'true') - expect(useOverlayStore.getState().snapshotCount).toBe(1) }) it('filters by title and URL, starts with search, and uses ArrowDown plus Enter to open a history item', async () => { @@ -1154,7 +1064,6 @@ describe('browser address suggestions', () => { fireEvent.pointerDown(option) fireEvent.click(option) expect(host.navigate).toHaveBeenCalledWith(tab.browserTabId, 'https://chain.test/') - expect(useOverlayStore.getState().count).toBe(0) }) it('submits the search row but lets IME composition finish first', async () => { @@ -1169,20 +1078,22 @@ describe('browser address suggestions', () => { expect(host.navigate).toHaveBeenCalledWith(tab.browserTabId, 'https://www.google.com/search?q=%E7%95%8C%E9%9D%A2%E8%AE%BE%E8%AE%A1') }) - it('keeps the native page behind a presentation snapshot while suggestions are open and restores it on Escape', async () => { + it('drops its suggestions over the live page and closes them on Escape without touching the page', async () => { seedVisits() const tab = openBrowserTab() await renderReady() + host.setVisible.mockClear() const address = screen.getByTestId('workspace-browser-address') act(() => address.focus()) - await waitFor(() => expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, false)) - expect(host.snapshot).toHaveBeenCalledWith(tab.browserTabId) + expect(screen.getByRole('listbox')).toBeInTheDocument() + // The list paints over the page; the page itself never had to move. + expect(pageShown(tab)).toBe(true) fireEvent.change(address, { target: { value: 'unfinished' } }) fireEvent.keyDown(address, { key: 'Escape' }) expect(address).toHaveValue(tab.url) expect(screen.queryByRole('listbox')).toBeNull() - expect(useOverlayStore.getState().count).toBe(0) - expect(host.setVisible).toHaveBeenLastCalledWith(tab.browserTabId, true) + expect(pageShown(tab)).toBe(true) + expect(host.setVisible).not.toHaveBeenCalled() }) }) diff --git a/desktop/src/components/workbench/WorkspaceBrowserTab.tsx b/desktop/src/components/workbench/WorkspaceBrowserTab.tsx index f08fab6b..7c9accd4 100644 --- a/desktop/src/components/workbench/WorkspaceBrowserTab.tsx +++ b/desktop/src/components/workbench/WorkspaceBrowserTab.tsx @@ -1,5 +1,4 @@ import { useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react' -import { flushSync } from 'react-dom' import { ArrowLeft, ArrowRight, @@ -18,7 +17,6 @@ import { WorkspaceBrowserAddressBar } from '@/components/workbench/WorkspaceBrow import { WorkspaceBrowserSelectionBar } from '@/components/workbench/WorkspaceBrowserSelectionBar' import { useDismissable } from '@/hooks/useDismissable' import { useTranslation } from '../../i18n' -import { computeWebviewBounds } from '../browser/computeWebviewBounds' import { getDesktopHost } from '../../lib/desktopHost' import { getServerBaseUrl } from '../../lib/desktopRuntime' import { formatBytes } from '../../lib/formatBytes' @@ -28,7 +26,11 @@ import { isWorkspaceBrowserAvailable, workspaceBrowserHost, } from '../../lib/workspace/browserHost' -import { useOverlayStore } from '../../stores/overlayStore' +import { + ensureWorkspaceBrowserGuest, + isWorkspaceBrowserGuestRegistered, + placeWorkspaceBrowserGuest, +} from '../../lib/workspace/browserGuests' import { useSettingsStore } from '../../stores/settingsStore' import { useUIStore } from '@/stores/uiStore' import { useWorkspaceBrowserStore } from '../../stores/workspaceBrowserStore' @@ -43,7 +45,6 @@ import type { WorkspaceBrowserTab as WorkspaceBrowserTabModel } from '../../lib/ const MIN_ZOOM = MIN_APP_ZOOM const MAX_ZOOM = MAX_APP_ZOOM const ZOOM_STEP = 0.1 -const PRESENTATION_SNAPSHOT_TIMEOUT_MS = 800 type BrowserPanel = 'downloads' | 'history' | null @@ -69,10 +70,11 @@ function resolveNavigationUrl(input: string, sessionId: string): string { /** * One page, addressed by its own `browserTabId`. * - * React here only decides *where the page is drawn*. Creating it, navigating it - * and destroying it are the controller's and the host's business — which is why - * unmounting this component (hiding the panel, switching tab, switching task) - * hides the view and nothing more. + * React here only decides *where the page is drawn*: the stage below is the + * placeholder the page's `` is positioned over. Creating it, + * navigating it and destroying it are the controller's and the host's business + * — which is why unmounting this component (hiding the panel, switching tab, + * switching task) parks the page and nothing more. */ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowserTabProps) { const t = useTranslation() @@ -91,20 +93,19 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser const panelRef = useRef(null) const appZoom = useSettingsStore((state) => state.uiZoom) const theme = useUIStore((state) => state.theme) - const overlayCount = useOverlayStore((state) => state.count) - const snapshotOverlayCount = useOverlayStore((state) => state.snapshotCount) - const nativePresentedRef = useRef(false) - const [presentationSnapshot, setPresentationSnapshot] = useState(null) const available = useMemo(() => isWorkspaceBrowserAvailable(), []) const browserTabId = tab.browserTabId - const [registeredId, setRegisteredId] = useState(null) + // Bumped when this page's registration settles; readiness itself is read + // from the guest registry, which also knows when a page was lost. + const [, setRegistrationSettled] = useState(0) const [createAttempt, setCreateAttempt] = useState(0) const lifetimeRef = useRef<{ id: string; ready: boolean; cancelled: boolean } | null>(null) const navigationRequestRef = useRef(0) - const appZoomRef = useRef(appZoom) - appZoomRef.current = appZoom const page = useWorkspaceBrowserStore((state) => state.pageByTabId[browserTabId]) - const ready = page?.registered === true || registeredId === browserTabId + // Only a page the host adopted can take commands. A page registered by an + // earlier mount is ready at once, so re-activating a tab does not flash a + // disabled toolbar. + const ready = isWorkspaceBrowserGuestRegistered(browserTabId) const initialAddressFocusRef = useRef({ browserTabId, pending: !tab.url && !ready, activated: false }) if (initialAddressFocusRef.current.browserTabId !== browserTabId) { initialAddressFocusRef.current = { browserTabId, pending: !tab.url && !ready, activated: false } @@ -116,7 +117,7 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser const visits = useMemo(() => Object.values(historyByTabId).flatMap((entries) => entries ?? []), [historyByTabId]) const downloads = useWorkspaceBrowserStore((state) => state.downloads) const loading = page?.loading ?? false - menuAllowedRef.current = active && overlayCount === 0 && !pendingNavigation + menuAllowedRef.current = active && !pendingNavigation const currentAddress = page?.url || tab.url || '' const annotationActive = page?.annotationActive ?? false @@ -151,13 +152,13 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser menuRequestRef.current = null setMenuOpen(false) } - }, [active, overlayCount, pendingNavigation]) + }, [active, pendingNavigation]) const stillOwned = useCallback(() => useWorkspaceStore.getState().findBrowserTabOwner(browserTabId)?.sessionId === sessionId, [browserTabId, sessionId]) const canCommand = useCallback(() => { const lifetime = lifetimeRef.current return lifetime?.id === browserTabId && !lifetime.cancelled && stillOwned() && - (lifetime.ready || useWorkspaceBrowserStore.getState().pageByTabId[browserTabId]?.registered === true) + (lifetime.ready || isWorkspaceBrowserGuestRegistered(browserTabId)) }, [browserTabId, stillOwned]) const reportHostError = useCallback((error: unknown) => { const lifetime = lifetimeRef.current @@ -180,18 +181,9 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser reportHostError(error) }) } - const reportBounds = useCallback(() => { - if (!canCommand()) return - const element = stageRef.current - if (!element) return - void workspaceBrowserHost.setBounds( - browserTabId, - computeWebviewBounds(element.getBoundingClientRect(), appZoomRef.current), - ).catch(reportHostError) - }, [browserTabId, canCommand, reportHostError]) - // Create once per page identity. `storageId` travels with it so a restored - // tab reopens the same page rather than a blank one. + // tab reopens the same page rather than a blank one. A page that already + // exists is only re-registered, never re-navigated. useEffect(() => { if (!available || !stillOwned()) return menuRequestRef.current = null @@ -200,32 +192,25 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser const lifetime = { id: browserTabId, ready: false, cancelled: false } lifetimeRef.current = lifetime const request = ++navigationRequestRef.current - const navigationId = useWorkspaceBrowserStore.getState().pageByTabId[browserTabId]?.navigationId ?? 0 - const element = stageRef.current - void workspaceBrowserHost.create(browserTabId, { - storageId: tab.storageId, - // A delayed completion must never attach an abandoned tab over its replacement. - visible: false, - ...(tab.url ? { url: tab.url } : {}), - ...(element - ? { bounds: computeWebviewBounds(element.getBoundingClientRect(), appZoom) } - : {}), - }).then((result) => { - if (lifetime.cancelled || !stillOwned() || !result.ok) return + void ensureWorkspaceBrowserGuest(browserTabId, async (webContentsId) => { + const result = await workspaceBrowserHost.create(browserTabId, { + storageId: tab.storageId, + webContentsId, + ...(tab.url ? { url: tab.url } : {}), + }) + if (!result.ok) throw new Error(t('workspace.browser.unavailableTitle')) + }).then(() => { + if (lifetime.cancelled || !stillOwned()) return lifetime.ready = true - setRegisteredId(browserTabId) - reportBounds() + setRegistrationSettled((count) => count + 1) }).catch((error: unknown) => { if (lifetime.cancelled || !stillOwned() || request !== navigationRequestRef.current) return - const current = useWorkspaceBrowserStore.getState().pageByTabId[browserTabId] - // The initial load may reject after the user has already navigated again. - if (current && (current.navigationId > navigationId + 1 || current.navigationOutcome === 'succeeded')) return reportHostError(error) }) // Deliberately NOT closing on unmount: the page belongs to the tab, and the // tab outlives this component. `closeTab` is the only thing that ends it. return () => { - const registered = lifetime.ready || useWorkspaceBrowserStore.getState().pageByTabId[browserTabId]?.registered === true + const registered = lifetime.ready || isWorkspaceBrowserGuestRegistered(browserTabId) lifetime.cancelled = true if (registered && stillOwned()) { void workspaceBrowserHost.setVisible(browserTabId, false).catch((error: unknown) => { @@ -235,98 +220,43 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser }) } } - // The URL/bounds are initial inputs, not a reason to recreate a live page. + // The URL is an initial input, not a reason to recreate a live page. // eslint-disable-next-line react-hooks/exhaustive-deps }, [browserTabId, createAttempt]) /* - A `WebContentsView` always paints above the DOM, so "is this page on screen" - has to account for everything the app might want to draw over it — and the - teardown is load-bearing. + The page is a `` composited with the DOM, so menus, dialogs and + other pages draw over it on their own. What remains is deciding when this + surface shows it at all — and the teardown is load-bearing. - Without the cleanup, unmounting leaves the page attached at its last bounds: - switching to a file tab, opening the `+` picker, hiding the workspace or - switching tasks would each leave a live page floating over whatever replaced - it. The surface renders only the active tab, so unmount is the *normal* way - a browser tab goes off screen, not an edge case. + The surface renders only the active tab, so unmount is the *normal* way a + browser tab goes off screen: switching to a file tab, hiding the workspace + or switching tasks. Releasing the placement parks the page (it keeps its + state); without that it would stay drawn over whatever replaced it. - Full-page panels and the retry overlay still replace the guest. The toolbar - menu uses the host's native popup layer, so opening it must not detach or - hide the live page. + Full-page panels and the retry overlay replace the page, and a tab with no + address shows its empty state instead. */ - const pageCanBePresented = active && + const pageVisible = active && panel === null && - !pendingNavigation && !tab.loadError && Boolean(tab.url) - const pageVisible = pageCanBePresented && overlayCount === 0 - const snapshotRequested = pageCanBePresented && overlayCount > 0 && overlayCount === snapshotOverlayCount - const navigationId = page?.navigationId ?? 0 + useLayoutEffect(() => { + const stage = stageRef.current + if (!available || !stage) return + return placeWorkspaceBrowserGuest(browserTabId, stage, pageVisible && ready) + }, [available, browserTabId, pageVisible, ready]) + + // Only the shown page acts as browser chrome in the host (shortcuts, the zoom + // capsule), and a native menu of a page that goes away has to close with it. useLayoutEffect(() => { if (!canCommand()) return - let cancelled = false - let settled = false - let timeout: ReturnType | undefined - const present = (visible: boolean) => { - nativePresentedRef.current = visible - void workspaceBrowserHost.setVisible(browserTabId, visible).catch(reportHostError) - } - if (snapshotRequested && nativePresentedRef.current) { - // Capture while the native page is still attached. Its WebContentsView - // would otherwise cover a DOM menu, but detaching first can capture an - // empty frame. This image is presentation-only and never becomes a chat - // attachment. Ordinary modal overlays still hide immediately below. - const finish = (dataUrl: string | null) => { - if (cancelled || settled) return - settled = true - clearTimeout(timeout) - // Commit the already-decoded image before the IPC detaches the view. - flushSync(() => setPresentationSnapshot(dataUrl)) - present(false) - } - timeout = setTimeout(() => finish(null), PRESENTATION_SNAPSHOT_TIMEOUT_MS) - void workspaceBrowserHost.snapshot(browserTabId).then(async (dataUrl) => { - if (cancelled || settled) return - if (!dataUrl?.startsWith('data:image/png;base64,')) { finish(null); return } - const image = new Image() - image.src = dataUrl - if (image.decode) await image.decode() - finish(dataUrl) - }).catch(() => finish(null)) - } else { - setPresentationSnapshot(null) - present(pageVisible) - } - return () => { - cancelled = true - clearTimeout(timeout) - } - // Navigation invalidates an image even when the page identity is reused. - }, [browserTabId, canCommand, navigationId, pageVisible, ready, reportHostError, snapshotRequested]) + void workspaceBrowserHost.setVisible(browserTabId, pageVisible).catch(reportHostError) + }, [browserTabId, canCommand, pageVisible, ready, reportHostError]) - useEffect(() => { - if (!active || !ready) return - const element = stageRef.current - if (!element) return - let cancelled = false - const update = () => { if (!cancelled) reportBounds() } - const observer = new ResizeObserver(update) - observer.observe(element) - window.addEventListener('resize', update) - return () => { - cancelled = true - observer.disconnect() - window.removeEventListener('resize', update) - } - }, [active, ready, reportBounds]) - - useLayoutEffect(() => { - if (active) reportBounds() - }, [active, appZoom, ready, reportBounds]) - - // The capsule must be drawn inside the native page, not behind it in React. - // The host retains this configuration and replays it after each navigation. + // The capsule is drawn inside the page itself, so it scrolls and zooms with + // it. The host retains this configuration and replays it after each navigation. useEffect(() => { if (!ready || !available || !active || !canCommand()) return const styles = getComputedStyle(document.documentElement) @@ -635,17 +565,7 @@ export function WorkspaceBrowserTab({ sessionId, tab, active }: WorkspaceBrowser ) : null}
-
- {presentationSnapshot ? ( - - ) : null} +
{!tab.url && !tab.loadError ? (