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 ? (