diff --git a/apps/desktop/e2e/browser-focus.spec.ts b/apps/desktop/e2e/browser-focus.spec.ts new file mode 100644 index 00000000000..1f740a6b7fe --- /dev/null +++ b/apps/desktop/e2e/browser-focus.spec.ts @@ -0,0 +1,319 @@ +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { createServer, type Server } from 'node:http' +import { tmpdir } from 'node:os' +import { dirname, join } from 'node:path' +import { fileURLToPath } from 'node:url' +import { + type ElectronApplication, + _electron as electron, + expect, + type Page, + test, +} from '@playwright/test' +import type { BrowserToolName } from '@sim/browser-protocol' +import type { SimDesktopApi } from '@sim/desktop-bridge' +import { getErrorMessage } from '@sim/utils/errors' + +const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url)) +const SCOPE = 'browser-focus-e2e' +const PRIMARY = process.platform === 'darwin' ? 'meta' : 'control' +const SHELL_FIXTURE = + 'Sim fixture

Browser focus fixture

' +const page = (name: string) => + `${name}Open popup` + +type Bridge = typeof globalThis & { simDesktop: SimDesktopApi; shellMarker?: string } + +/** + * The browser shares one window with Sim, so focus and shortcuts decide whether + * a keystroke lands in chat, in the page, or reloads all of Sim. These checks + * drive the real shell and native tab views. + */ +test('browser focus and shortcuts stay with the surface the user is using', async () => { + const reportPath = + process.env.DESKTOP_BROWSER_FOCUS_REPORT_PATH ?? test.info().outputPath('browser-focus.json') + const checks: { + name: string + status: 'passed' | 'failed' + durationMs: number + error?: string + }[] = [] + const check = async (name: string, run: () => Promise) => { + const started = Date.now() + try { + await test.step(name, run) + checks.push({ name, status: 'passed', durationMs: Date.now() - started }) + } catch (error) { + checks.push({ + name, + status: 'failed', + durationMs: Date.now() - started, + error: getErrorMessage(error), + }) + throw error + } + } + const calls = new Map() + const server: Server = createServer(async (request, response) => { + const path = new URL(request.url ?? '/', 'http://localhost').pathname + if (path === '/api/desktop/tool/authorize') { + let body = '' + for await (const chunk of request) body += chunk.toString() + const authorization = calls.get(JSON.parse(body).toolCallId) + response.writeHead(authorization ? 200 : 403, { 'Content-Type': 'application/json' }) + response.end(JSON.stringify(authorization ?? {})) + return + } + // The app origin serves the shell; the same server on localhost is the web. + const isSite = request.headers.host?.startsWith('localhost') === true + response.writeHead(200, { + 'Content-Type': 'text/html', + ...(isSite ? {} : { 'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; Path=/' }), + }) + response.end(isSite ? page(path.slice(1)) : SHELL_FIXTURE) + }) + const userData = mkdtempSync(join(tmpdir(), 'sim-browser-focus-e2e-')) + let app: ElectronApplication | undefined + let passed = false + try { + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() + if (!address || typeof address === 'string') throw new Error('Missing fixture address') + const origin = `http://127.0.0.1:${address.port}` + /** Pages outside the app origin browse in the agent partition, like any third-party site. */ + const site = origin.replace('127.0.0.1', 'localhost') + const shellApp = await electron.launch({ + args: [process.env.SIM_DESKTOP_E2E_MAIN ?? '.'], + cwd: DESKTOP_DIR, + env: { ...process.env, SIM_DESKTOP_ORIGIN: origin, SIM_DESKTOP_USER_DATA: userData }, + }) + app = shellApp + const shell: Page = await shellApp.firstWindow() + await shellApp.evaluate(({ app, BrowserWindow }) => { + const host = BrowserWindow.getAllWindows()[0] + host.webContents.setBackgroundThrottling(false) + app.focus({ steal: true }) + host.focus() + }) + await expect(shell.getByRole('heading')).toHaveText('Browser focus fixture') + await shell.evaluate(async (scope) => { + const api = (globalThis as Bridge).simDesktop.browserAgent + await api.activateScope(scope) + const updateBounds = () => + api.setPanelBounds( + { x: 0, y: 120, width: innerWidth, height: innerHeight - 120 }, + null, + scope + ) + updateBounds() + ;(globalThis as Bridge & { boundsTimer?: number }).boundsTimer = window.setInterval( + updateBounds, + 200 + ) + ;(globalThis as Bridge).shellMarker = 'alive' + }, SCOPE) + + let callCount = 0 + const execute = async (tool: BrowserToolName, args: Record) => { + const callId = `browser-focus-${++callCount}` + calls.set(callId, { chatId: SCOPE, toolName: tool, args }) + const result = await shell.evaluate( + ({ callId, tool, args, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.executeTool(callId, tool, args, scope), + { callId, tool, args, scope: SCOPE } + ) + expect(result.ok).toBe(true) + return result.ok ? result.result : undefined + } + const panelAction = (action: Record) => + shell.evaluate( + ({ action, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.panelAction( + action as unknown as Parameters[0], + scope + ), + { action, scope: SCOPE } + ) + const setPanelFocused = (focused: boolean) => + shell.evaluate( + ({ focused, scope }) => + (globalThis as Bridge).simDesktop.browserAgent.setPanelFocused(focused, scope), + { focused, scope: SCOPE } + ) + const tabCount = () => + shell.evaluate( + async (scope) => + (await (globalThis as Bridge).simDesktop.browserAgent.activateScope(scope)).tabs.length, + SCOPE + ) + const shellAlive = () => + shell.evaluate(() => (globalThis as Bridge).shellMarker === 'alive').catch(() => false) + const composerFocused = () => + shell.evaluate(() => document.hasFocus() && document.activeElement?.id === 'composer') + const focusedPageUrls = () => + shellApp.evaluate(({ BrowserWindow, WebContentsView }) => + BrowserWindow.getAllWindows()[0] + .contentView.children.filter( + (view) => view instanceof WebContentsView && view.webContents.isFocused() + ) + .map((view) => (view as Electron.WebContentsView).webContents.getURL()) + ) + const visiblePageUrl = () => + shellApp.evaluate(({ BrowserWindow, WebContentsView }) => { + const view = BrowserWindow.getAllWindows()[0].contentView.children.find( + (child) => child instanceof WebContentsView && child.getVisible() + ) as Electron.WebContentsView | undefined + return view?.webContents.getURL() ?? null + }) + /** Counts main-frame loads of the visible page from now on. */ + const countVisiblePageLoads = () => + shellApp.evaluate(({ BrowserWindow, WebContentsView }) => { + const view = BrowserWindow.getAllWindows()[0].contentView.children.find( + (child) => child instanceof WebContentsView && child.getVisible() + ) as Electron.WebContentsView + const counter = globalThis as typeof globalThis & { pageLoads?: number; counted?: boolean } + counter.pageLoads = 0 + if (!counter.counted) { + counter.counted = true + view.webContents.on('did-finish-load', () => { + counter.pageLoads = (counter.pageLoads ?? 0) + 1 + }) + } + }) + const pageLoads = () => + shellApp.evaluate(() => (globalThis as typeof globalThis & { pageLoads?: number }).pageLoads) + const focusVisiblePage = () => + shellApp.evaluate(({ BrowserWindow, WebContentsView }) => { + const view = BrowserWindow.getAllWindows()[0].contentView.children.find( + (child) => child instanceof WebContentsView && child.getVisible() + ) as Electron.WebContentsView + view.webContents.focus() + }) + const pressInPage = (keyCode: string, modifiers: string[]) => + shellApp.evaluate( + ({ BrowserWindow, WebContentsView }, { keyCode, modifiers }) => { + const view = BrowserWindow.getAllWindows()[0].contentView.children.find( + (child) => child instanceof WebContentsView && child.getVisible() + ) as Electron.WebContentsView + const input = { keyCode, modifiers } as Electron.KeyboardInputEvent + view.webContents.sendInputEvent({ ...input, type: 'keyDown' }) + view.webContents.sendInputEvent({ ...input, type: 'keyUp' }) + }, + { keyCode, modifiers } + ) + /** Clicks an application-menu item the way its accelerator would. */ + const clickMenu = (label: string) => + shellApp.evaluate(({ BrowserWindow, Menu }, label) => { + const win = BrowserWindow.getAllWindows()[0] + const find = (items: Electron.MenuItem[]): Electron.MenuItem | null => { + for (const item of items) { + if (item.label === label) return item + const found = item.submenu ? find(item.submenu.items) : null + if (found) return found + } + return null + } + const item = find(Menu.getApplicationMenu()?.items ?? []) + if (!item) throw new Error(`No menu item ${label}`) + item.click(undefined, win, win.webContents) + }, label) + + await check('the agent working never takes the caret from the composer', async () => { + await shell.locator('#composer').click() + await shell.keyboard.type('draft') + for (const [tool, args] of [ + ['browser_open_url', { url: `${site}/one` }], + ['browser_navigate', { url: `${site}/two` }], + ['browser_open_tab', { url: `${site}/three` }], + ] as const) { + await execute(tool, args) + await expect.poll(visiblePageUrl).not.toBeNull() + expect(await composerFocused()).toBe(true) + expect(await focusedPageUrls()).toEqual([]) + } + // A link without an opener is focused by Chromium itself as it is created. + const snapshot = await execute('browser_snapshot', {}) + const ref = /"Open popup" \[ref=(\d+)\]/.exec( + String((snapshot as { outline?: string }).outline) + )?.[1] + expect(ref, 'snapshot lists the popup link').toBeTruthy() + await execute('browser_click', { elementId: Number(ref) }) + await expect.poll(tabCount).toBe(3) + await expect.poll(focusedPageUrls).toEqual([]) + expect(await composerFocused()).toBe(true) + await shell.keyboard.type(' continues') + await expect(shell.locator('#composer')).toHaveValue('draft continues') + }) + + await check('an omnibox navigation hands focus to the page it loads', async () => { + await panelAction({ action: 'switch-tab', tabId: '1' }) + await panelAction({ action: 'navigate', url: `${site}/four` }) + await expect.poll(focusedPageUrls).toEqual([`${site}/four`]) + }) + + await check('a popup the agent opens hands focus back to the page the user is in', async () => { + const before = await tabCount() + const snapshot = await execute('browser_snapshot', {}) + const ref = /"Open popup" \[ref=(\d+)\]/.exec( + String((snapshot as { outline?: string }).outline) + )?.[1] + expect(ref, 'snapshot lists the popup link').toBeTruthy() + await execute('browser_click', { elementId: Number(ref) }) + await expect.poll(tabCount).toBe(before + 1) + await expect.poll(focusedPageUrls).toEqual([`${site}/four`]) + }) + + await check('reload keys typed in the page reload only that page', async () => { + for (const [keyCode, modifiers] of [ + ['R', [PRIMARY]], + ['R', [PRIMARY, 'shift']], + ['F5', []], + ] as const) { + await countVisiblePageLoads() + await focusVisiblePage() + await pressInPage(keyCode, [...modifiers]) + await expect.poll(pageLoads).toBe(1) + } + }) + + await check('the page keeps its shortcut claim after the chrome reports blur', async () => { + await setPanelFocused(true) + await focusVisiblePage() + await setPanelFocused(false) + await countVisiblePageLoads() + await clickMenu('Reload') + await expect.poll(pageLoads).toBe(1) + expect(await shellAlive()).toBe(true) + }) + + await check('Back and Forward move through the focused page history', async () => { + await panelAction({ action: 'navigate', url: `${site}/five` }) + await expect.poll(visiblePageUrl).toBe(`${site}/five`) + await focusVisiblePage() + await clickMenu('Back') + await expect.poll(visiblePageUrl).toBe(`${site}/four`) + await focusVisiblePage() + await clickMenu('Forward') + await expect.poll(visiblePageUrl).toBe(`${site}/five`) + }) + + await check('browser shortcuts work while renderer chrome hides the page', async () => { + const before = await tabCount() + await shell.evaluate((scope) => { + const bridge = globalThis as Bridge & { boundsTimer?: number } + window.clearInterval(bridge.boundsTimer) + bridge.simDesktop.browserAgent.setPanelBounds(null, null, scope) + }, SCOPE) + await setPanelFocused(true) + await clickMenu('New Tab') + await expect.poll(tabCount).toBe(before + 1) + }) + passed = true + } finally { + mkdirSync(dirname(reportPath), { recursive: true }) + writeFileSync(reportPath, JSON.stringify({ passed, checks }, null, 2)) + await app?.close() + await new Promise((resolve) => server.close(() => resolve())) + rmSync(userData, { recursive: true, force: true }) + } +}) diff --git a/apps/desktop/src/main/browser-agent/driver.ts b/apps/desktop/src/main/browser-agent/driver.ts index 4e2dbe9c126..ba968d28051 100644 --- a/apps/desktop/src/main/browser-agent/driver.ts +++ b/apps/desktop/src/main/browser-agent/driver.ts @@ -5399,6 +5399,7 @@ export async function handlePanelAction( ) session.prepareExplicitNavigation(contents) void contents.loadURL(action.url).catch(() => {}) + session.focusPageForUser(contents) } return } diff --git a/apps/desktop/src/main/browser-agent/panel.ts b/apps/desktop/src/main/browser-agent/panel.ts index b72290245da..304e7e22ade 100644 --- a/apps/desktop/src/main/browser-agent/panel.ts +++ b/apps/desktop/src/main/browser-agent/panel.ts @@ -47,6 +47,8 @@ export interface PanelHost { onViewDetached: (view: WebContentsView | null) => void /** Invalidates field-anchored UI when the page moves, hides, or detaches. */ onGeometryChanged?: () => void + /** Runs after each layout that leaves the active view attached and visible. */ + onViewShown?: (view: WebContentsView) => void } let host: PanelHost = { @@ -549,6 +551,7 @@ export function layout(): void { }, 1_000) } } + if (visible) host.onViewShown?.(active.view) } /** Converts the applied native DIP rectangle back into Sim viewport CSS pixels. */ diff --git a/apps/desktop/src/main/browser-agent/session.ts b/apps/desktop/src/main/browser-agent/session.ts index 1b7de802598..6f494b979f8 100644 --- a/apps/desktop/src/main/browser-agent/session.ts +++ b/apps/desktop/src/main/browser-agent/session.ts @@ -119,6 +119,8 @@ export interface AgentTab { lastRealUserGestureAt?: number /** The tab whose page opened this one; agent work returns there when this tab closes. */ openerTabId?: string + /** A user action asked for this page to take focus once it is on screen. */ + pendingUserFocus?: boolean } interface PendingMediaPermission { @@ -211,7 +213,13 @@ const FOREGROUND_TAB_RESTORE_TIMEOUT_MS = 20_000 const MEDIA_PERMISSION_GESTURE_WINDOW_MS = 10_000 const MEDIA_PERMISSION_PROMPT_TIMEOUT_MS = 30_000 -export type BrowserShortcut = 'focus-omnibox' | 'new-tab' | 'close-tab' | 'find' +export type BrowserShortcut = + | 'focus-omnibox' + | 'new-tab' + | 'close-tab' + | 'find' + | 'reload' + | 'hard-reload' type BrowserShortcutInput = Pick< Input, @@ -220,25 +228,32 @@ type BrowserShortcutInput = Pick< /** * Resolves browser-level shortcuts using Command on macOS and Control - * elsewhere. Modified/composing/repeated keystrokes stay with the page. + * elsewhere. Alt, composing, and repeated keystrokes stay with the page. + * + * Reload resolves here, from the page's own keystroke, rather than through the + * application menu: the menu can only guess which surface owns focus, and a + * wrong guess reloads all of Sim instead of the page. */ export function browserShortcutForInput( input: BrowserShortcutInput, platform: NodeJS.Platform = process.platform ): BrowserShortcut | null { - if ( - input.type !== 'keyDown' || - input.isAutoRepeat || - input.isComposing || - input.shift || - input.alt - ) { + if (input.type !== 'keyDown' || input.isAutoRepeat || input.isComposing || input.alt) { return null } const primaryModifier = platform === 'darwin' ? input.meta : input.control - if (!primaryModifier) return null + const otherModifier = platform === 'darwin' ? input.control : input.meta - switch (input.key.toLowerCase()) { + if (input.key === 'F5' && !otherModifier) { + if (input.shift || input.control) return 'hard-reload' + return input.meta ? null : 'reload' + } + if (!primaryModifier || otherModifier) return null + + const key = input.key.toLowerCase() + if (key === 'r') return input.shift ? 'hard-reload' : 'reload' + if (input.shift) return null + switch (key) { case 'l': return 'focus-omnibox' case 't': @@ -266,6 +281,12 @@ interface BrowserScopeState { lastPersistedSnapshot: string | null focusedBrowserTabId: string | null focusedBrowserClearTimer: ReturnType | null + /** + * Renderer-drawn browser chrome (omnibox, toolbar, a New Tab or error page) + * owns the user's interaction. Those surfaces hide the native page, so this + * is the only evidence the Browser is the shortcut target while they show. + */ + browserChromeFocused: boolean automationActive: boolean automationNeedsAttention: boolean /** @@ -293,6 +314,7 @@ function createBrowserScopeState(): BrowserScopeState { lastPersistedSnapshot: null, focusedBrowserTabId: null, focusedBrowserClearTimer: null, + browserChromeFocused: false, automationActive: false, automationNeedsAttention: false, findingTabId: null, @@ -1039,6 +1061,10 @@ export function initSession( if (!scopeId) return withBrowserScope(scopeId, restoreBrowserSession) }, + onViewShown: (view) => { + const scopeId = browserScopeIdForView(view) + if (scopeId) withBrowserScope(scopeId, () => applyPendingUserFocus(view)) + }, onViewDetached: (view) => { if (!view) return const scopeId = browserScopeIdForView(view) @@ -1815,6 +1841,29 @@ function configureBrowserDownloads(ses: Session): void { }) } +/** + * Hands keyboard focus to a page the user just navigated or opened, the way + * Chrome focuses the content after an omnibox Enter. Tab views never focus + * themselves on navigation, so this is the only path that moves focus into a + * page. A view that is not on screen yet takes focus when it is first shown. + */ +export function focusPageForUser(contents: WebContents): void { + const tab = tabForContents(contents) + if (!tab) return + tab.pendingUserFocus = true + applyPendingUserFocus(tab.view) +} + +function applyPendingUserFocus(view: WebContentsView): void { + const tab = activeTab() + if (!tab?.pendingUserFocus || tab.view !== view || view.webContents.isDestroyed()) return + // A renderer modal can hide the view while the panel keeps its bounds. + if (!view.getVisible() || !isPanelVisible()) return + if (getBrowserScopeId() !== getActiveBrowserScopeId()) return + tab.pendingUserFocus = false + view.webContents.focus() +} + function focusRendererOmnibox(mode: BrowserOmniboxFocusMode): void { if (getBrowserScopeId() !== getActiveBrowserScopeId()) return const win = panelWindow() @@ -1830,7 +1879,18 @@ function tabForContents(contents: WebContents): AgentTab | null { function publishPageIssue(tab: AgentTab, focusRecovery = false): void { events?.onTabsChanged() if (tab.id !== currentScope.activeTabId) return - if (focusRecovery && getBrowserScopeId() === getActiveBrowserScopeId() && isPanelVisible()) { + // Recovery moves focus to the renderer's issue page only when the failed page + // held it; the user typing in chat keeps their caret. + const pageHadFocus = + !currentScope.browserChromeFocused && + (currentScope.focusedBrowserTabId === tab.id || + (!tab.view.webContents.isDestroyed() && tab.view.webContents.isFocused())) + if ( + focusRecovery && + pageHadFocus && + getBrowserScopeId() === getActiveBrowserScopeId() && + isPanelVisible() + ) { const win = panelWindow() if (win && !win.isDestroyed()) win.webContents.focus() } @@ -2087,10 +2147,10 @@ function adoptPopupTab( opener: WebContents, url: string, popup: PopupWindowOptions, - agentOwned: boolean + { agentOwned, background }: { agentOwned: boolean; background: boolean } ): WebContents { const openerTabId = tabForContents(opener)?.id - const tab = agentOwned ? addAutomationTab(url, popup) : addTab(url, popup) + const tab = openLinkTab(url, { agentOwned, background }, popup) tab.openerTabId = openerTabId const contents = tab.view.webContents // A background-tab disposition defers creation, so Chromium supplies no contents to adopt. @@ -2098,16 +2158,38 @@ function adoptPopupTab( return contents } +/** + * Creates the tab a link opens. The user's Cmd/middle-click keeps their page + * in front, as in Chrome; a foreground tab the user opened takes focus. + */ +function openLinkTab( + url: string, + { agentOwned, background }: { agentOwned: boolean; background: boolean }, + popup?: PopupWindowOptions +): AgentTab { + if (agentOwned) return addAutomationTab(url, popup) + if (background) { + restoreBrowserSession() + return addTabInternal({ activate: false, url, popup }) + } + const tab = addTab(url, popup) + focusPageForUser(tab.view.webContents) + return tab +} + /** * Opens a link from a page in another tab of this browser. Shared by the * window.open interception and the page's right-click menu — both have to stay * inside the browser resource rather than spawn a native window, and both are * reached from an untrusted page, so the scheme is checked here once. */ -function openTabWithUrl(url: string, { agentOwned }: { agentOwned: boolean }): void { +function openTabWithUrl( + url: string, + { agentOwned, background = false }: { agentOwned: boolean; background?: boolean } +): void { if (!/^https?:\/\//i.test(url)) return try { - const tab = agentOwned ? addAutomationTab(url) : addTab(url) + const tab = openLinkTab(url, { agentOwned, background }) void tab.view.webContents.loadURL(url).catch(() => {}) } catch (error) { logger.warn('Could not open a link in a new browser tab', { @@ -2165,6 +2247,10 @@ function createFreshTabView(appSession: BrowserAppSession | undefined): WebConte // the active tab while a tool waits on it, applied explicitly by // applyAutomationTabPolicy — never blanket across every tab. backgroundThrottling: true, + // Electron focuses a page on every main-frame commit, even a hidden + // agent tab, which steals the caret from chat. Focus follows explicit + // user actions instead (see focusPageForUser). + focusOnNavigation: false, spellcheck: false, // The default every origin this tab visits starts at; a per-origin zoom // the user sets from the page menu still wins and still persists. @@ -2209,7 +2295,8 @@ function initializeTabView( registerAgentNavigation(contents, routeNavigation) attachAgentContextMenu(contents, { addToChat: (text) => withBrowserScope(scopeId, () => addPageSelectionToChat(contents, text)), - openTab: (url) => withBrowserScope(scopeId, () => openTabWithUrl(url, { agentOwned: false })), + openTab: (url) => + withBrowserScope(scopeId, () => openTabWithUrl(url, { agentOwned: false, background: true })), defaultZoomFactor: getBrowserDefaultZoomFactor, }) @@ -2221,7 +2308,38 @@ function initializeTabView( currentScope.focusedBrowserClearTimer = null } const tab = tabs.find((entry) => entry.view.webContents === contents) - currentScope.focusedBrowserTabId = tab?.id ?? currentScope.activeTabId + // A tab the user cannot see never keeps keyboard focus. Chromium focuses + // a page opened without an opener (a target=_blank link) while creating + // it, before the tab is even listed. Hand focus back to the visible page + // the user was in, or else to Sim, once that focus call has returned, or + // Chromium finishes it over the top. + if (!tab || tab.id !== currentScope.activeTabId) { + const active = activeTab() + const returnTo = + active && + !currentScope.browserChromeFocused && + currentScope.focusedBrowserTabId === active.id + ? active + : null + setImmediate( + bindToBrowserScope(scopeId, () => { + const current = tabs.find((entry) => entry.view.webContents === contents) + const win = panelWindow() + if (current?.id === currentScope.activeTabId || !win || win.isDestroyed()) return + if (contents.isDestroyed() || !contents.isFocused()) return + if ( + returnTo?.id === currentScope.activeTabId && + !returnTo.view.webContents.isDestroyed() + ) { + returnTo.view.webContents.focus() + } else { + win.webContents.focus() + } + }) + ) + return + } + currentScope.focusedBrowserTabId = tab.id }) ) contents.on( @@ -2270,16 +2388,20 @@ function initializeTabView( contents.setWindowOpenHandler((details) => withBrowserScope(scopeId, () => { const agentOwned = agentOwnsPopupFrom(contents) + const background = details.disposition === 'background-tab' if (!canAdoptPopup(contents, details.url)) { - openTabWithUrl(details.url, { agentOwned }) + openTabWithUrl(details.url, { agentOwned, background }) return { action: 'deny' } } return { action: 'allow', outlivesOpener: true, + // A popup's contents are created by Chromium from these preferences, not + // from createFreshTabView, so they must opt out of navigation focus here. + overrideBrowserWindowOptions: { webPreferences: { focusOnNavigation: false } }, createWindow: (options) => withBrowserScope(scopeId, () => - adoptPopupTab(contents, details.url, options, agentOwned) + adoptPopupTab(contents, details.url, options, { agentOwned, background }) ), } }) @@ -2367,6 +2489,15 @@ function initializeTabView( focusRendererOmnibox('clear') return } + if (shortcut === 'reload') { + reloadPage(contents) + return + } + if (shortcut === 'hard-reload') { + prepareExplicitNavigation(contents) + contents.reloadIgnoringCache() + return + } if (tab) closeTabFromUser(tab.id) }) @@ -3177,6 +3308,7 @@ export function switchTab(tabId: string, { claim = true }: { claim?: boolean } = const previousActiveTab = activeTab() if (previousActiveTab && previousActiveTab.id !== tab.id) { revokeTabMediaPermissions(previousActiveTab, false) + previousActiveTab.pendingUserFocus = false } currentScope.activeTabId = tab.id if (claim) currentScope.visibleTabUserSelected = true @@ -3302,9 +3434,9 @@ export function closeAutomationTab(tabId: string): void { /** The live page whose browser surface owns a menu accelerator. */ function focusedTabForShortcut(ownerWindow?: BrowserWindow | null): AgentTab | null { - if (!isPanelVisible() || !panelUpdateAllowed(ownerWindow ?? undefined, getBrowserScopeId())) { - return null - } + if (!panelUpdateAllowed(ownerWindow ?? undefined, getBrowserScopeId())) return null + if (currentScope.browserChromeFocused) return activeTab() + if (!isPanelVisible()) return null return ( tabs.find( (tab) => @@ -3383,6 +3515,12 @@ export function handleFocusedShortcut( prepareExplicitNavigation(shortcutTab.view.webContents) shortcutTab.view.webContents.reloadIgnoringCache() return true + case 'back': + goBack(shortcutTab.view.webContents) + return true + case 'forward': + goForward(shortcutTab.view.webContents) + return true } const zoomAction = zoomActionForShortcut(shortcut) @@ -3404,9 +3542,22 @@ export function setPanelFocused( withBrowserScope(scopeId, () => { if (!panelUpdateAllowed(ownerWindow, getBrowserScopeId())) return if (!focused) { + currentScope.browserChromeFocused = false + for (const tab of tabs) tab.pendingUserFocus = false + // The renderer reports losing focus when the user clicks into the native + // page too; that page's own focus claim must survive the report. + const claimed = tabs.find((tab) => tab.id === currentScope.focusedBrowserTabId) + if ( + claimed && + !claimed.view.webContents.isDestroyed() && + claimed.view.webContents.isFocused() + ) { + return + } clearFocusedBrowserTab() return } + currentScope.browserChromeFocused = true if (currentScope.focusedBrowserClearTimer !== null) { clearTimeout(currentScope.focusedBrowserClearTimer) currentScope.focusedBrowserClearTimer = null diff --git a/apps/desktop/src/main/ipc.ts b/apps/desktop/src/main/ipc.ts index 840334ce29c..4b2fb02f087 100644 --- a/apps/desktop/src/main/ipc.ts +++ b/apps/desktop/src/main/ipc.ts @@ -51,6 +51,7 @@ import { isAgentWebContents } from '@/main/browser-agent/registry' import { addTab, findInActiveTab, + focusPageForUser, getBrowserDownloadsState, peekTabsState, reorderTab, @@ -959,6 +960,7 @@ export function registerIpcHandlers(deps: IpcDeps): void { return peekTabsState() } void tab.view.webContents.loadURL(destination).catch(() => {}) + focusPageForUser(tab.view.webContents) return peekTabsState() }) }, diff --git a/apps/desktop/src/main/menu.ts b/apps/desktop/src/main/menu.ts index c7d61ec0ba6..b2234f74d5b 100644 --- a/apps/desktop/src/main/menu.ts +++ b/apps/desktop/src/main/menu.ts @@ -95,6 +95,29 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] } } + /** The focused Browser tab moves through its own history; otherwise the Sim window does. */ + const traverseHistory = ( + direction: 'back' | 'forward' + ): NonNullable => { + return (_item, focusedWindow) => { + const win = focusedMainOrFallback(focusedWindow) + if (!win) return + if (deps.handleFocusedResourceShortcut(win, direction)) return + const history = win.webContents.navigationHistory + if (direction === 'back' ? history.canGoBack() : history.canGoForward()) { + if (direction === 'back') history.goBack() + else history.goForward() + } + } + } + + const reload: NonNullable = (_item, focusedWindow) => { + const win = focusedMainOrFallback(focusedWindow) + if (!win) return + if (deps.handleFocusedResourceShortcut(win, 'reload-or-clear')) return + win.webContents.reload() + } + const viewSubmenu: MenuItemConstructorOptions[] = [ /** * The command palette is the web app's own `Mod+K` command; claiming the @@ -120,25 +143,20 @@ export function buildMenuTemplate(deps: MenuDeps): MenuItemConstructorOptions[] { label: 'Back', accelerator: 'CmdOrCtrl+[', - click: (_item, focusedWindow) => { - const win = focusedMainOrFallback(focusedWindow) - if (!win) return - const history = win.webContents.navigationHistory - if (history.canGoBack()) { - history.goBack() - } - }, + click: traverseHistory('back'), + }, + { + label: 'Forward', + accelerator: 'CmdOrCtrl+]', + click: traverseHistory('forward'), }, { label: 'Reload', accelerator: 'CmdOrCtrl+R', - click: (_item, focusedWindow) => { - const win = focusedMainOrFallback(focusedWindow) - if (!win) return - if (deps.handleFocusedResourceShortcut(win, 'reload-or-clear')) return - win.webContents.reload() - }, + click: reload, }, + /** F5 is the Windows and Linux reload key; hidden so the menu keeps one Reload row. */ + { label: 'Reload', accelerator: 'F5', visible: false, click: reload }, /** * Hard refresh, cache ignored. A focused Browser tab claims it first * (same boundary as Reload/Close Tab); otherwise it reloads the Sim diff --git a/apps/desktop/src/main/resource-shortcuts.ts b/apps/desktop/src/main/resource-shortcuts.ts index 24aa02d7350..cfcff836a47 100644 --- a/apps/desktop/src/main/resource-shortcuts.ts +++ b/apps/desktop/src/main/resource-shortcuts.ts @@ -13,6 +13,8 @@ export type FocusedResourceShortcut = | ResourceTabSelectionShortcut | 'reload-or-clear' | 'hard-reload' + | 'back' + | 'forward' | `zoom-${DesktopZoomAction}` export type ResourceTabSelectionShortcut = diff --git a/apps/desktop/src/main/terminal/index.ts b/apps/desktop/src/main/terminal/index.ts index 6256a0bcd93..d20e68a57ec 100644 --- a/apps/desktop/src/main/terminal/index.ts +++ b/apps/desktop/src/main/terminal/index.ts @@ -495,8 +495,15 @@ export class TerminalService { emitRendererCommand: (command: TerminalShortcutCommand, terminalId: string) => void, confirmCloseRunning?: (running: string) => boolean | Promise ): boolean { - // Hard reload has no terminal meaning — leave it to the Browser or shell. - if (shortcut === 'focus-omnibox' || shortcut === 'hard-reload') return false + // Hard reload and history have no terminal meaning — leave them to the Browser or shell. + if ( + shortcut === 'focus-omnibox' || + shortcut === 'hard-reload' || + shortcut === 'back' || + shortcut === 'forward' + ) { + return false + } const visibleTabShortcut = shortcut === 'new-tab' || shortcut === 'reopen-closed-tab' || shortcut === 'close-tab' const ownsVisibleTabs = diff --git a/apps/desktop/src/test/electron-mock.ts b/apps/desktop/src/test/electron-mock.ts index 822eab9e3ad..84155b510d7 100644 --- a/apps/desktop/src/test/electron-mock.ts +++ b/apps/desktop/src/test/electron-mock.ts @@ -250,7 +250,11 @@ function createWebContentsMock() { export class WebContentsView { webContents = createWebContentsMock() setBackgroundColor = vi.fn() - setVisible = vi.fn() + private visible = true + setVisible = vi.fn((visible: boolean) => { + this.visible = visible + }) + getVisible = vi.fn(() => this.visible) private bounds = { x: 0, y: 0, width: 0, height: 0 } setBounds = vi.fn((bounds: { x: number; y: number; width: number; height: number }) => { this.bounds = { ...bounds } diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-find-bar.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-find-bar.tsx index a3f9e23198d..5827b5d2013 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-find-bar.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-find-bar.tsx @@ -141,6 +141,8 @@ export function BrowserFindBar({ inputRef, onClose, scopeId }: BrowserFindBarPro onKeyDown={(event) => { // The panel and the global command layer both listen for these. event.stopPropagation() + // Keys during an IME composition edit the composed text. + if (event.nativeEvent.isComposing) return if (event.key === 'Escape') { event.preventDefault() dismiss() diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-issue.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-issue.tsx index 199aa2b8be9..e1a5bc9d603 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-issue.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-page-issue.tsx @@ -125,13 +125,28 @@ export function browserPageIssueCopy(issue: BrowserPageIssue): BrowserPageIssueC } } +function isEditingOutside(heading: HTMLElement | null): boolean { + const active = document.activeElement + // activeElement survives a window blur, so a caret left in chat still counts. + if (!(active instanceof HTMLElement)) return false + const section = heading?.closest('section') + if (section?.contains(active)) return false + return ( + active.isContentEditable || + active instanceof HTMLInputElement || + active instanceof HTMLTextAreaElement + ) +} + /** Replaces a hidden native page and optionally claims renderer focus for keyboard recovery. */ export function BrowserPageIssueView({ issue, onReload, focusRecovery }: BrowserPageIssueProps) { const headingRef = useRef(null) const copy = browserPageIssueCopy(issue) useEffect(() => { - if (focusRecovery) headingRef.current?.focus() + // Keyboard recovery for someone who was in the page; a caret in chat or + // any other Sim field stays where it is. + if (focusRecovery && !isEditingOutside(headingRef.current)) headingRef.current?.focus() }, [focusRecovery, issue]) const Icon = issue.kind === 'load-error' ? Globe : CircleAlert diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session.tsx index e8ecc6223b6..94ce6a6d3a3 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-content/components/browser-session/browser-session.tsx @@ -34,6 +34,10 @@ import { useTheme } from 'next-themes' import { createPortal } from 'react-dom' import { BrowserImportDialog } from '@/components/browser-import/browser-import-dialog' import { EmptyState } from '@/components/empty-state/empty-state' +import { + onBrowserOmniboxFocusRequest, + takeBrowserOmniboxFocusRequest, +} from '@/lib/browser-agent/omnibox-focus' import { onFocusVisibleBrowserOmnibox } from '@/lib/browser-agent/renderer-shortcuts' import { loadBrowserSearchSuggestions, @@ -643,16 +647,17 @@ export function BrowserSession({ useEffect(() => onBrowserOmniboxFocus(focusOmnibox, scopeId), [focusOmnibox, scopeId]) - // A fresh blank tab coming on screen — opened from the resource strip or by - // Cmd+T — gets the omnibox, the way Chrome's new-tab page does. A tab with a - // page keeps its content. - const focusedBlankTabIdRef = useRef(null) + // A blank tab the user opened from the resource strip gets the omnibox once + // it is on screen, the way Chrome's new-tab page does. Cmd+T arrives from the + // shell above. A blank tab the agent opened must never take the caret. useEffect(() => { - if (!visible || !activeTabId || !showEmptyState) return - if (focusedBlankTabIdRef.current === activeTabId) return - focusedBlankTabIdRef.current = activeTabId - focusOmnibox('clear') - }, [activeTabId, focusOmnibox, showEmptyState, visible]) + if (!visible || !activeTabId) return + const claimFocusRequest = () => { + if (takeBrowserOmniboxFocusRequest(activeTabId, scopeId)) focusOmnibox('clear') + } + claimFocusRequest() + return onBrowserOmniboxFocusRequest(claimFocusRequest) + }, [activeTabId, focusOmnibox, scopeId, visible]) // Sim owns keyboard events while its renderer has focus. Claim Cmd+L here // before the workspace's global "Go to Logs" command can navigate away. @@ -1151,6 +1156,8 @@ export function BrowserSession({ }} onKeyDown={(event) => { event.stopPropagation() + // Keys during an IME composition edit the composed text, not the URL. + if (event.nativeEvent.isComposing) return if (event.key === 'ArrowDown' || event.key === 'ArrowUp') { // Never move a highlight through a list that is not on screen. if (!suggestionsOpen) return @@ -1167,10 +1174,17 @@ export function BrowserSession({ } if (event.key === 'Enter') submitUrl() if (event.key === 'Escape') { - // Dismiss the list first; leave the omnibox only once - // there is no highlight left to back out of. - if (activeSuggestion !== null) setActiveSuggestion(null) - else urlInputRef.current?.blur() + // Back out one step at a time, as Chrome does: the + // highlight, then the edited text, then the omnibox. + if (activeSuggestion !== null) { + setActiveSuggestion(null) + } else if ((urlDraft ?? '') !== (pageState?.url ?? '')) { + setUrlDraft(pageState?.url ?? '') + setSuggestionsVisible(false) + selectFocusedOmniboxOnNextFrame(event.currentTarget) + } else { + urlInputRef.current?.blur() + } } }} /> diff --git a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx index c5d8a5e8095..2071fb84190 100644 --- a/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx +++ b/apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx @@ -23,6 +23,7 @@ import { import { Columns3, Eye, Pencil } from '@sim/emcn/icons' import type { TerminalTabState } from '@sim/terminal-protocol' import { useQueries } from '@tanstack/react-query' +import { requestBrowserOmniboxFocus } from '@/lib/browser-agent/omnibox-focus' import { browserTabTitle } from '@/lib/browser-agent/tab-label' import { openBrowserTab, @@ -342,7 +343,9 @@ export function ResourceTabs({ if (resource.type === 'browser') { void openBrowserTab(desktopScopeId) .then((state) => { - if (state?.activeTabId) selectResource(state.activeTabId) + if (!state?.activeTabId) return + requestBrowserOmniboxFocus(state.activeTabId, state.scopeId) + selectResource(state.activeTabId) }) .catch(() => toast.error('Could not open a new browser tab. Please try again.')) return diff --git a/apps/sim/lib/browser-agent/omnibox-focus.ts b/apps/sim/lib/browser-agent/omnibox-focus.ts new file mode 100644 index 00000000000..be0d980cfd2 --- /dev/null +++ b/apps/sim/lib/browser-agent/omnibox-focus.ts @@ -0,0 +1,34 @@ +/** + * "Put the caret in this new tab's omnibox" — sent by the resource strip when + * the user opens a blank browser tab. The tab only reaches the screen after the + * desktop confirms it and the strip selects it, and the browser panel may not + * be mounted yet, so the request is held here until the panel shows that exact + * tab. Tabs the agent opens never send one, so its work cannot move the caret. + */ +const BROWSER_OMNIBOX_FOCUS_EVENT = 'sim:focus-browser-omnibox' + +interface BrowserOmniboxFocusRequest { + scopeId: string + tabId: string +} + +let pendingRequest: BrowserOmniboxFocusRequest | null = null + +/** Asks the browser panel to focus the omnibox once it shows this tab. */ +export function requestBrowserOmniboxFocus(tabId: string, scopeId: string): void { + pendingRequest = { scopeId, tabId } + window.dispatchEvent(new Event(BROWSER_OMNIBOX_FOCUS_EVENT)) +} + +/** Claims the pending request when it names the tab now on screen. */ +export function takeBrowserOmniboxFocusRequest(tabId: string, scopeId: string): boolean { + if (pendingRequest?.tabId !== tabId || pendingRequest.scopeId !== scopeId) return false + pendingRequest = null + return true +} + +/** Subscribes the browser panel to new requests; returns an unsubscribe. */ +export function onBrowserOmniboxFocusRequest(callback: () => void): () => void { + window.addEventListener(BROWSER_OMNIBOX_FOCUS_EVENT, callback) + return () => window.removeEventListener(BROWSER_OMNIBOX_FOCUS_EVENT, callback) +}