Skip to content

Commit 17832ac

Browse files
frenchie4111claude
andauthored
fix(browser): screenshot and inspect tabs that aren't on screen (#264)
## Summary - `screenshot_tab` failed for any browser tab that wasn't the visible pane. Hiding a tab detached its `WebContentsView` from the window, and a parentless view has no compositor surface and reports a 0×0 viewport — so `capturePage()` threw `UnknownVizError` (surfacing as `harness HTTP 500: capture failed`), or for a never-displayed tab produced an empty image that the control server returned as **HTTP 200 with `data: ''`**, which the MCP bridge reported as a bare "screenshot failed" with nothing logged anywhere. `get_tab_clickables` shared the root cause: a 0×0 viewport makes a page look like it has no buttons. - Hidden views are now **parked** in a lazily-created, never-shown frameless window sized to fit them, so they keep laying out and painting. Capture falls back to CDP `Page.captureScreenshot({ fromSurface: false })` when the on-screen surface is missing. Empty / zero-size captures are now an error carrying a readable reason, never a 200. - Separately: `create_browser_tab` didn't normalize a bare host, so `example.com` failed with `ERR_INVALID_URL` despite the tool description promising a scheme would be prepended. `navigate()` did normalize. Both managers now share one `normalizeBrowserUrl` helper. ### Notes on the approach Three alternatives were probed and rejected with evidence: - **CDP `fromSurface: false` alone** — fails on a parentless view ("Unable to capture screenshot"), and hangs or crashes the process on a never-attached 0×0 view. It needs the view parented, which is what the park window provides. - **`setBounds()` in `create()` alone** — a parentless view stays 0×0 no matter what bounds you set. - **Attaching the view "behind" the main renderer view** — impossible: `win.contentView.children.length` is 0 for a `BrowserWindow`, so its page isn't a child view and any added view composites *above* the UI. The parked tab would visibly leak over the app. The park window is `show: false`, `skipTaskbar: true`, frameless and square-cornered (a title bar and rounded corners clip the region a parked view can paint). A quit guard destroys it once no real window is left, so it can't suppress `window-all-closed` on Linux/Windows. ## Test plan Covered by tests: - [x] `normalizeBrowserUrl` — bare host, explicit schemes (`about:`/`file:`/`data:`), `host:port` vs scheme ambiguity, loopback → `http`, whitespace, empty - [x] `viewportCaptureError` / `encodedCaptureError` — 0×0 bounds, NaN/negative, zero-size image, empty buffer - [x] `/browser/screenshot` responses — 200 with data, 500 with the verbatim reason, 500 for empty data, 500 for a vanished tab - [x] `npm run typecheck`, `npx electron-vite build`, `npx vitest run` Verified by hand in a running dev instance (real MCP HTTP path): - [x] Reproduced the original `UnknownVizError` live, then confirmed the fallback returns a real image - [x] Bare host `example.com` → `https://example.com/` - [x] Screenshot of a tab hidden by another tab taking the pane → fresh, correct content - [x] `get_tab_clickables` returns a real viewport and finds targets Verified by hand against the real `BrowserManager` in an Electron process, images inspected visually: - [x] visible 900×600, parked-after-hide, never-displayed at 1280×800, parked 1400×1000 fully painted (no black band), and a parked tab navigated while hidden returning the *new* page - [x] Park window stays `isVisible() === false`, host window keeps focus, `destroyAll()` tears it down Not verified by hand: the literal "switch the sidebar to another worktree" gesture (can't drive the native UI) — the identical `hide()` → detach path was induced by opening a second tab that displaces the first. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
1 parent 2891a7d commit 17832ac

10 files changed

Lines changed: 448 additions & 51 deletions

src/main/browser-manager-playwright.test.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,9 @@ describe('PlaywrightBrowserManager', () => {
127127
expect(url).toMatch(new RegExp(`^${baseUrl}/?$`))
128128

129129
const shot = await m.capturePage(tabId)
130-
expect(shot).not.toBeNull()
131-
expect(shot!.format).toBe('jpeg')
132-
expect(shot!.data.length).toBeGreaterThan(100)
130+
expect(shot?.error).toBeUndefined()
131+
expect(shot?.format).toBe('jpeg')
132+
expect(shot?.data?.length ?? 0).toBeGreaterThan(100)
133133

134134
const clickables = (await m.getClickables(tabId)) as
135135
| { items: Array<{ role: string; name: string; cx: number; cy: number }> }

src/main/browser-manager-playwright.ts

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,10 @@
2121

2222
import { createRequire } from 'module'
2323
import type { Browser, BrowserContext, Page } from 'playwright-core'
24-
import type { BrowserManagerLike, ConsoleLog } from './browser-manager-types'
24+
import type { BrowserManagerLike, CaptureResult, ConsoleLog } from './browser-manager-types'
2525
import type { Store } from './store'
2626
import { log } from './debug'
27+
import { normalizeBrowserUrl } from './browser-url'
2728

2829
const CONSOLE_LOG_CAP = 200
2930

@@ -311,7 +312,7 @@ export class PlaywrightBrowserManager implements BrowserManagerLike {
311312
if (this.hasTab(tabId)) return
312313
log('browser-playwright', `create tab=${tabId} wt=${worktreePath} url=${url}`)
313314
this.pendingTabIds.add(tabId)
314-
const initialUrl = url && url.trim() ? url : 'about:blank'
315+
const initialUrl = normalizeBrowserUrl(url) ?? 'about:blank'
315316
this.dispatchState(tabId, { url: initialUrl, loading: true })
316317
void this.createAsync(tabId, worktreePath, initialUrl).catch((err) => {
317318
const message = err instanceof Error ? err.message : String(err)
@@ -468,9 +469,8 @@ export class PlaywrightBrowserManager implements BrowserManagerLike {
468469
navigate(tabId: string, url: string): void {
469470
const inst = this.instances.get(tabId)
470471
if (!inst) return
471-
const target = url.trim()
472-
if (!target) return
473-
const normalized = /^[a-z][a-z0-9+\-.]*:/i.test(target) ? target : `https://${target}`
472+
const normalized = normalizeBrowserUrl(url)
473+
if (!normalized) return
474474
this.dispatchState(tabId, { loading: true })
475475
inst.page.goto(normalized).catch((err) => {
476476
log(
@@ -619,25 +619,25 @@ export class PlaywrightBrowserManager implements BrowserManagerLike {
619619
async capturePage(
620620
tabId: string,
621621
opts?: { format?: 'jpeg' | 'png'; quality?: number }
622-
): Promise<{ data: string; format: 'jpeg' | 'png' } | null> {
622+
): Promise<CaptureResult | null> {
623623
const inst = this.instances.get(tabId)
624624
if (!inst) return null
625625
try {
626626
const format = opts?.format === 'png' ? 'png' : 'jpeg'
627-
if (format === 'png') {
628-
const buf = await inst.page.screenshot({ type: 'png' })
629-
return { data: buf.toString('base64'), format: 'png' }
630-
}
631627
const q = Math.max(1, Math.min(100, Math.round(opts?.quality ?? 70)))
632-
const buf = await inst.page.screenshot({ type: 'jpeg', quality: q })
633-
return { data: buf.toString('base64'), format: 'jpeg' }
628+
const buf =
629+
format === 'png'
630+
? await inst.page.screenshot({ type: 'png' })
631+
: await inst.page.screenshot({ type: 'jpeg', quality: q })
632+
if (buf.length < 1) {
633+
log('browser-playwright', `capturePage produced no image tab=${tabId}`)
634+
return { error: 'capture encoded to 0 bytes' }
635+
}
636+
return { data: buf.toString('base64'), format }
634637
} catch (err) {
635-
log(
636-
'browser-playwright',
637-
`capturePage failed tab=${tabId}`,
638-
err instanceof Error ? err.message : err
639-
)
640-
return null
638+
const message = err instanceof Error ? err.message : String(err)
639+
log('browser-playwright', `capturePage failed tab=${tabId}`, message)
640+
return { error: `capture failed: ${message}` }
641641
}
642642
}
643643

src/main/browser-manager-types.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,12 @@ export interface ConsoleLog {
1717
message: string
1818
}
1919

20+
/** Success carries the encoded image; failure carries a reason the caller can
21+
* surface verbatim. `null` from capturePage still means "no such tab". */
22+
export type CaptureResult =
23+
| { data: string; format: 'jpeg' | 'png'; error?: undefined }
24+
| { error: string; data?: undefined; format?: undefined }
25+
2026
export interface BrowserManagerLike {
2127
setStore(store: Store): void
2228
hasTab(tabId: string): boolean
@@ -54,6 +60,6 @@ export interface BrowserManagerLike {
5460
capturePage(
5561
tabId: string,
5662
opts?: { format?: 'jpeg' | 'png'; quality?: number }
57-
): Promise<{ data: string; format: 'jpeg' | 'png' } | null>
63+
): Promise<CaptureResult | null>
5864
getDom(tabId: string): Promise<string | null>
5965
}

0 commit comments

Comments
 (0)