From e41256eb5a926764d6fcbdaa0d762d767c88668a Mon Sep 17 00:00:00 2001 From: Luke Parker <10430890+Hona@users.noreply.github.com> Date: Wed, 7 Oct 2026 09:50:13 +1000 Subject: [PATCH] fix(desktop): fix browser bar review findings (#53627) --- .../app/component-tests/browser-pane.spec.ts | 67 ++++++- .../src/browser/address.test.ts | 24 +++ .../gui-extensions/src/browser/address.ts | 26 ++- packages/gui-extensions/src/browser/bar.css | 24 +++ .../gui-extensions/src/browser/chromium.ts | 49 +++-- .../gui-extensions/src/browser/connection.ts | 7 +- .../gui-extensions/src/browser/i18n/en.ts | 3 + packages/gui-extensions/src/browser/icon.ts | 32 +++- packages/gui-extensions/src/browser/model.ts | 34 ++-- .../gui-extensions/src/browser/page-icon.tsx | 27 +++ packages/gui-extensions/src/browser/pane.ts | 12 +- .../src/browser/panel.fixture.tsx | 6 +- packages/gui-extensions/src/browser/panel.tsx | 175 ++++++++++-------- .../gui-extensions/src/browser/policy.test.ts | 33 +++- packages/gui-extensions/src/browser/policy.ts | 39 +++- .../gui-extensions/src/browser/renderer.tsx | 6 +- 16 files changed, 427 insertions(+), 137 deletions(-) create mode 100644 packages/gui-extensions/src/browser/page-icon.tsx diff --git a/packages/app/component-tests/browser-pane.spec.ts b/packages/app/component-tests/browser-pane.spec.ts index cbe2db7a753..ab62484147c 100644 --- a/packages/app/component-tests/browser-pane.spec.ts +++ b/packages/app/component-tests/browser-pane.spec.ts @@ -282,13 +282,28 @@ story("restores the current URL each time the same submitted navigation is block await address.press("Enter") await expect(address).toHaveValue("https://blocked.example/") await root.getByRole("button", { name: "Block navigation", exact: true }).click() - await expect(root.getByRole("alert")).toHaveText("Request failed") + await expect(root.getByRole("alert")).toHaveText("Only web pages can open here.") await expect(address).toHaveValue("https://alpha.example/") await expect(root.getByTestId("native-Alpha")).toHaveAttribute("data-visible", "true") }) } }) +story("explains why a typed address cannot open instead of searching for it", async ({ page }) => { + const root = page.getByTestId("browser-pane-fixture") + const address = root.getByRole("combobox", { name: "Browser address", exact: true }) + + for (const [typed, reason] of [ + ["ftp://example.com", "Only web pages can open here."], + ["https://user:pass@example.com", "Addresses with a user name or password can't open here."], + ]) { + await address.fill(typed) + await address.press("Enter") + await expect(root.getByRole("alert")).toHaveText(reason) + await expect(address).toHaveValue("https://alpha.example/") + } +}) + story("shows a themed failure state for only the failed tab and allows retry", async ({ page }) => { const root = page.getByTestId("browser-pane-fixture") await root.getByRole("button", { name: "Failed page", exact: true }).click() @@ -435,6 +450,9 @@ story("suggests visited pages and opens the first one that completes the typed a const options = page.getByRole("option") await expect(options).toHaveCount(1) await expect(options.first()).toHaveAttribute("aria-selected", "true") + // Arrow keys that pick IME candidates leave the suggestions alone. + await address.dispatchEvent("keydown", { key: "ArrowDown", isComposing: true }) + await expect(options.first()).toHaveAttribute("aria-selected", "true") await address.press("Enter") await expect(address).toHaveValue("http://localhost:5173/settings") @@ -451,14 +469,39 @@ story("suggests visited pages and opens the first one that completes the typed a story("lists recent pages on a blank tab and opens one", async ({ page }) => { const root = page.getByTestId("browser-pane-fixture") + const address = root.getByRole("combobox", { name: "Browser address", exact: true }) await root.getByRole("button", { name: "Seed history", exact: true }).click() await root.getByRole("button", { name: "Blank page", exact: true }).click() await expect(root.getByText("Recent", { exact: true })).toBeVisible() + // The globe at the field's start belongs to the field: a click there focuses it. + await expect(address).not.toBeFocused() + await root.locator('[data-component="browser-address"]').click({ position: { x: 16, y: 14 } }) + await expect(address).toBeFocused() + await address.blur() + await root.getByRole("button", { name: /^Settings - Preview/ }).click() - await expect(root.getByRole("combobox", { name: "Browser address", exact: true })).toHaveValue( - "http://localhost:5173/settings", - ) + await expect(address).toHaveValue("http://localhost:5173/settings") await expect(root.getByTestId("native-Alpha")).toHaveAttribute("data-visible", "true") + // The opened page owns the field afterwards: an edit that is never submitted shows its URL again. + await address.click() + await address.fill("unsubmitted") + await address.blur() + await expect(address).toHaveValue("http://localhost:5173/settings") +}) + +story("adds the whole page to the session and keeps the draft when the button is pressed again", async ({ page }) => { + const root = page.getByTestId("browser-pane-fixture") + const field = root.locator('[data-component="browser-address"]') + const add = root.getByRole("button", { name: "Add page to session", exact: true }) + await field.hover() + await add.click() + const editor = root.locator('[data-slot="browser-comment-editor"] textarea') + await editor.fill("Check the header layout") + await field.hover() + await add.click() + await expect(editor).toHaveValue("Check the header layout") + await editor.press("Enter") + await expect(root.getByTestId("fixture-comments").getByRole("listitem")).toHaveText([/: Check the header layout$/]) }) story("focuses the address field when the page uses the address shortcut", async ({ page }) => { @@ -478,12 +521,22 @@ story("shows the page's cookies in the site information and clears them", async await expect(site).toContainText("0 cookies in use") }) -story("zooms the page from the browser options and shows its zoom", async ({ page }) => { +story("zooms the page from the browser options in view and keeps its zoom in the address field", async ({ page }) => { const root = page.getByTestId("browser-pane-fixture") - await root.getByRole("button", { name: "Browser options", exact: true }).click() + const options = root.getByRole("button", { name: "Browser options", exact: true }) + await options.click() await page.getByRole("menuitem", { name: /^Zoom in/ }).click() await expect(root.getByText("Zoom: 110", { exact: true })).toBeVisible() + // The menu covered the page with a still; it closes so the zoomed page shows at once. + await expect(page.getByRole("menu")).toBeHidden() + await expect(root.getByTestId("native-Alpha")).toHaveAttribute("data-visible", "true") + + const zoom = root.getByRole("button", { name: "110%", exact: true }) + await expect(zoom).toBeVisible() + await options.click() await expect(page.getByRole("menuitem", { name: /^Reset zoom/ })).toContainText("110%") - await page.getByRole("menuitem", { name: /^Reset zoom/ }).click() + await page.keyboard.press("Escape") + await zoom.click() await expect(root.getByText("Zoom: 100", { exact: true })).toBeVisible() + await expect(zoom).toBeHidden() }) diff --git a/packages/gui-extensions/src/browser/address.test.ts b/packages/gui-extensions/src/browser/address.test.ts index b606a1f1828..777e1c24040 100644 --- a/packages/gui-extensions/src/browser/address.test.ts +++ b/packages/gui-extensions/src/browser/address.test.ts @@ -19,6 +19,19 @@ test("the address field navigates to what reads as an address and searches for a "mdn", "css grid gap", "localhost 3000", + "ABOUT:BLANK", + "file:///C:/repo/index.html", + // A typed scheme is an address even where main refuses it, which then says why. + "file://server/share/x.html", + "ftp://example.com", + "about:config", + // Text that is no URL searches: no host, a space in it, or a host main would not open as a web page. + "https://", + "http:// foo", + "about: the movie", + "user@example.com", + ":8080", + "error: cannot find module", ].map(resolveAddress), ).toEqual([ "about:blank", @@ -33,6 +46,17 @@ test("the address field navigates to what reads as an address and searches for a search("mdn"), search("css grid gap"), search("localhost 3000"), + "about:blank", + "file:///C:/repo/index.html", + "file://server/share/x.html", + "ftp://example.com", + "about:config", + search("https://"), + search("http:// foo"), + search("about: the movie"), + search("user@example.com"), + search(":8080"), + search("error: cannot find module"), ]) }) diff --git a/packages/gui-extensions/src/browser/address.ts b/packages/gui-extensions/src/browser/address.ts index 0ff0070988e..0a8ac351a5d 100644 --- a/packages/gui-extensions/src/browser/address.ts +++ b/packages/gui-extensions/src/browser/address.ts @@ -1,3 +1,5 @@ +import { destinationOrigin } from "./policy" + /** Where text that is not an address goes. Shared with the system browser's default so results look familiar. */ const SEARCH = "https://www.google.com/search?q=" @@ -8,25 +10,31 @@ const SEARCH = "https://www.google.com/search?q=" export function resolveAddress(input: string) { const value = input.trim() - if (!value) return "about:blank" + if (!value || value.toLowerCase() === "about:blank") return "about:blank" return searches(value) ? `${SEARCH}${encodeURIComponent(value)}` : value } -/** Whether the address field would search for the text rather than open it. */ +/** + * Whether the address field would search for the text rather than open it. A typed scheme means an address, which + * main opens or explains why not; it searches only when the text is no URL at all, such as a bare `https://`. Text + * without one is an address when it names a host main opens as a web page. + */ export function searches(input: string) { const value = input.trim() - if (/^[a-z][a-z\d+.-]*:\/\//i.test(value) || /^about:/i.test(value)) return false + if (!value || /^about:\S*$/i.test(value)) return false - if (!value || /\s/.test(value)) return !!value + if (/^[a-z][a-z\d+.-]*:\/\//i.test(value)) return !URL.canParse(value) + + if (/\s/.test(value)) return true const host = value.split(/[/?#]/, 1)[0] ?? "" - return !( - /^(?:localhost|\[[\da-f:.]+\])(?::\d+)?$/i.test(host) || - /:\d+$/.test(host) || - /^[^.]+(?:\.[^.]+)+$/.test(host) - ) + const named = + /^(?:localhost|\[[\da-f:.]+\])(?::\d+)?$/i.test(host) || /:\d+$/.test(host) || /^[^.]+(?:\.[^.]+)+$/.test(host) + + // Main adds the scheme the same way, so the text opens only if the whole URL it becomes does. + return !named || !destinationOrigin(`https://${value}`) } /** diff --git a/packages/gui-extensions/src/browser/bar.css b/packages/gui-extensions/src/browser/bar.css index e5fc6830e65..0d889ee2391 100644 --- a/packages/gui-extensions/src/browser/bar.css +++ b/packages/gui-extensions/src/browser/bar.css @@ -108,6 +108,30 @@ color: var(--v2-text-text-base); } + /* The page's zoom when it is not 100%, drawn like the zoom badge in the browser options. */ + [data-slot="browser-address-zoom"] { + flex: none; + display: inline-flex; + align-items: center; + height: 16px; + padding: 0 4px; + border: 0.5px solid var(--v2-border-border-base); + border-radius: 2px; + background: var(--v2-background-bg-layer-02); + color: var(--v2-text-text-muted); + font-size: 11px; + font-weight: 530; + line-height: var(--line-height-tight); + letter-spacing: 0.05px; + font-variant-numeric: tabular-nums; + white-space: nowrap; + cursor: default; + + &:hover { + color: var(--v2-text-text-base); + } + } + /* Page actions show on hover and while the field has focus. */ [data-slot="browser-address-actions"] { display: none; diff --git a/packages/gui-extensions/src/browser/chromium.ts b/packages/gui-extensions/src/browser/chromium.ts index 9aaff748cf7..4e665fa8c87 100644 --- a/packages/gui-extensions/src/browser/chromium.ts +++ b/packages/gui-extensions/src/browser/chromium.ts @@ -9,7 +9,15 @@ import { createDiagnostics } from "./diagnostics" import { loadIcon } from "./icon" import { createProfiling, type Recording } from "./profiling" import type { BrowserNetwork } from "./network" -import { allowedDestination, destinationOrigin, fileURLWithin, localFileURL, normalizeURL, type Policy } from "./policy" +import { + allowedDestination, + destinationOrigin, + fileURLWithin, + localFileURL, + normalizeURL, + refusal, + type Policy, +} from "./policy" import type { PaneElement } from "./ipc" type Element = { backendID: number; frameID: string; sessionID?: string } @@ -91,9 +99,15 @@ function zoomKey(input: Electron.Input) { if (input.key === "0") return "reset" } -/** The address a cookie is removed by: its domain without the leading dot, its path, and its scheme. */ +/** + * The address a cookie is removed by: its domain without the leading dot, its path, and its scheme. An IPv6 host + * needs its brackets in a URL. + */ function cookieURL(cookie: Electron.Cookie) { - return `${cookie.secure ? "https" : "http"}://${(cookie.domain ?? "").replace(/^\./, "")}${cookie.path ?? "/"}` + const domain = (cookie.domain ?? "").replace(/^\./, "") + const host = domain.includes(":") && !domain.startsWith("[") ? `[${domain}]` : domain + + return `${cookie.secure ? "https" : "http"}://${host}${cookie.path ?? "/"}` } /** The next preset in the direction, or 100%; the current factor at either end. */ @@ -218,7 +232,8 @@ export function createBrowserPage( const detail = () => { if (closed) return - const next: PageDetail = { zoom: contents.getZoomFactor() } + // Chromium's factor for the outermost presets can land a rounding error outside them. + const next: PageDetail = { zoom: Math.min(5, Math.max(0.25, contents.getZoomFactor())) } if (icon) next.icon = icon const key = `${next.zoom}:${next.icon ?? ""}` @@ -322,19 +337,22 @@ export function createBrowserPage( // error statuses are real documents from the user's server and stay visible. if (status === 502) failure = { url, message: `${status} ${statusText}`.trim().slice(0, 2_048) } - // Another site's icon arrives with its document; the same site keeps its icon meanwhile, as tabs do. - if (!URL.canParse(url) || new URL(url).origin !== origin) { + // Another site's icon arrives with its document; the same site keeps its icon meanwhile, as tabs do. Files and blank + // pages have no shared origin, so each keeps none. + const next = destinationOrigin(url) ?? url + + if (next !== origin) { icon = undefined iconRequest++ } - origin = URL.canParse(url) ? new URL(url).origin : "" - // Chromium keeps a zoom per origin, so a new site may have its own. - detail() + origin = next // A blank or failed document paints at commit; a real one waits for dom-ready. if (url === "about:blank" || failure) settle() publish() + // After the state that names the new URL. Chromium keeps a zoom per origin, so a new site may have its own. + detail() }) contents.on("did-fail-load", (_event, code, description, url, isMainFrame) => { // Cancelled navigation and failed subframes do not replace the current page. @@ -371,7 +389,8 @@ export function createBrowserPage( if (!event.isMainFrame && fileURLWithin(event.url, policy.fileRoots ?? [])) return event.preventDefault() - options.publish("ERR_BLOCKED_BY_CLIENT") + // The same reason a typed address gets, so a blocked link and a blocked address read alike. + options.publish(refusal(event.url, policy) ?? "ERR_BLOCKED_BY_CLIENT") } contents.on("will-frame-navigate", guard) @@ -553,7 +572,10 @@ export function createBrowserPage( return { cookies: (await contents.session.cookies.get({ url })).length } }, - /** Deletes what the page's site stored: the cookies its address can read, and its origin's storage. */ + /** + * Deletes what the page's site stored: the cookies its address can read, and its origin's storage. Resolves once + * the page reloaded without them, so a count read next includes what the reload set again. + */ async clearSite() { const url = contents.getURL() const origin = destinationOrigin(url) @@ -566,7 +588,10 @@ export function createBrowserPage( storages: ["localstorage", "indexdb", "serviceworkers", "cachestorage", "filesystem", "shadercache"], }) - if (!closed) contents.reload() + if (closed) return + contents.reload() + // A slow page still cleared; the count then reads what it set so far. + await waitFor(() => closed || !contents.isLoading(), new AbortController().signal, 30_000).catch(() => undefined) }, /** Reports the page's icon and zoom when either changed since the last report. */ detail, diff --git a/packages/gui-extensions/src/browser/connection.ts b/packages/gui-extensions/src/browser/connection.ts index ade15877109..2118537483f 100644 --- a/packages/gui-extensions/src/browser/connection.ts +++ b/packages/gui-extensions/src/browser/connection.ts @@ -41,9 +41,10 @@ export function createConnection(input: { target: () => { server: string; session: string } /** * `mirror` applies the current tabs to the strip; call it right after storing the state, in the same batch, or - * later once the strip can be written. A mirror that never runs leaves the strip's tabs as they are. + * later once the strip can be written. A mirror that never runs leaves the strip's tabs as they are. `native` is true + * for an inventory the desktop reported, false for a change made here, such as a new embed or a suspension. */ - change: (state: ConnectionState, mirror: () => void) => void + change: (state: ConnectionState, mirror: () => void, native: boolean) => void /** The session's strip: the browser tab IDs it stores, and quietly adding or removing one. */ strip: { stored: () => readonly string[] @@ -98,7 +99,7 @@ export function createConnection(input: { const publish = (native = false) => { pruning ||= native - input.change({ ...state }, mirror) + input.change({ ...state }, mirror, native) } // The pane itself is unreachable while its main extension restarts or is disabled, and main drops every diff --git a/packages/gui-extensions/src/browser/i18n/en.ts b/packages/gui-extensions/src/browser/i18n/en.ts index ac57016a16f..e4b7dc8e13a 100644 --- a/packages/gui-extensions/src/browser/i18n/en.ts +++ b/packages/gui-extensions/src/browser/i18n/en.ts @@ -8,6 +8,9 @@ export default { "address.label": "Browser address", "address.search": "Search or enter a URL", "address.suggestions": "Suggestions", + "refused.web": "Only web pages can open here.", + "refused.workspace": "Only web pages and files inside this session's workspace can open here.", + "refused.credentials": "Addresses with a user name or password can't open here.", inspect: "Select an element to comment on", "inspect.active": "Click an element in the page to comment on it. Press Escape to cancel.", "inspect.pageShortcut": "While the page has focus", diff --git a/packages/gui-extensions/src/browser/icon.ts b/packages/gui-extensions/src/browser/icon.ts index 8383d7da6e7..fbc0de2d42b 100644 --- a/packages/gui-extensions/src/browser/icon.ts +++ b/packages/gui-extensions/src/browser/icon.ts @@ -6,7 +6,8 @@ type Fetched = { mime: string; data: Buffer } /** * The first of a page's icon candidates that loads, as a data URL small enough to report and store. An SVG stays as - * it is; any other format is redrawn as a PNG of at most 32px, which also drops anything that is not an image. + * it is; any other format is redrawn as a PNG of at most 32px, which also drops anything that is not an image. Never + * rejects: a candidate that fails falls through to the next. */ export function loadIcon(candidates: readonly string[], network: BrowserNetwork | null) { return candidates @@ -16,7 +17,7 @@ export function loadIcon(candidates: readonly string[], network: BrowserNetwork (found, url) => found.then(async (icon) => { if (icon) return icon - const fetched = url.startsWith("data:") ? inline(url) : await network?.icon(url) + const fetched = /^data:/i.test(url) ? inline(url) : await network?.icon(url).catch(() => undefined) return fetched ? encode(fetched) : undefined }), @@ -30,15 +31,22 @@ function inline(url: string): Fetched | undefined { if (!match) return const body = match[3] ?? "" - return { - mime: (match[1] ?? "").toLowerCase(), - data: match[2] ? Buffer.from(body, "base64") : Buffer.from(decodeURIComponent(body)), - } + return { mime: (match[1] ?? "").toLowerCase(), data: match[2] ? Buffer.from(body, "base64") : unescape(body) } +} + +// Percent-decodes to bytes. A stray `%`, as in `width="100%"`, stays as it is rather than failing the whole icon. +function unescape(text: string) { + return Buffer.concat( + text + .split(/(%[\da-f]{2})/i) + .map((part) => (/^%[\da-f]{2}$/i.test(part) ? Buffer.from([parseInt(part.slice(1), 16)]) : Buffer.from(part))), + ) } function encode(fetched: Fetched) { - // An SVG in an runs no script and loads nothing, so it needs no redrawing. + // An SVG in an runs no script and loads nothing, so it needs no redrawing; one with no drawing is no icon. if (fetched.mime === "image/svg+xml") { + if (!/]/i.test(fetched.data.toString("utf8"))) return const url = `data:image/svg+xml;base64,${fetched.data.toString("base64")}` return url.length <= MAX_ICON_URL ? url : undefined @@ -47,7 +55,15 @@ function encode(fetched: Fetched) { const image = nativeImage.createFromBuffer(fetched.data) if (image.isEmpty()) return - const url = (image.getSize().width > 32 ? image.resize({ width: 32, quality: "best" }) : image).toDataURL() + const size = image.getSize() + + // The longer side fits 32px; resizing one side keeps the shape. + const fitted = + Math.max(size.width, size.height) <= 32 + ? image + : image.resize(size.width >= size.height ? { width: 32, quality: "best" } : { height: 32, quality: "best" }) + + const url = fitted.toDataURL() return url.length <= MAX_ICON_URL ? url : undefined } diff --git a/packages/gui-extensions/src/browser/model.ts b/packages/gui-extensions/src/browser/model.ts index e28fe3a3967..ffde316eb60 100644 --- a/packages/gui-extensions/src/browser/model.ts +++ b/packages/gui-extensions/src/browser/model.ts @@ -96,8 +96,22 @@ export function createModel(ctx: SetupContext) { const recorded = new Map() const key = (tabID: string) => `${ctx.id}:${tabID}` + // Main names why it moved control elsewhere or refused an address; any other failure is a failed request. + const describe = (error: string) => { + if (error === "browser.pane.replaced") return ctx.t("replaced") + + if (error === "browser.address.credentials") return ctx.t("refused.credentials") + + if (error === "browser.address.workspace") return ctx.t("refused.workspace") + + if (error === "browser.address.web") return ctx.t("refused.web") + + return ctx.t("common.requestFailed") + } + // Pages the user or the agent opened, once loaded: only a tab whose page exists has a real URL and title, as a - // restored tab's saved URL carries no title until it loads again. + // restored tab's saved URL carries no title until it loads again. Only the desktop's own reports count: a restored + // tab's embed arrives before its page loads, with the saved URL as if it had. const record = (session: string, next: { browser: Browser.State | null; embeds: Readonly> }) => next.browser?.tabs.forEach((tab) => { if (tab.loading || tab.loadError || !next.embeds[tab.id] || !recordable(tab.url)) return @@ -194,7 +208,7 @@ export function createModel(ctx: SetupContext) { if (visits) history.set({ visits }) }, address: (tabID) => addressed.get(id)?.forEach((listener) => listener(tabID)), - change: (next, mirror) => { + change: (next, mirror, native) => { if (next.error === "browser.pane.unsupported") { setState("unsupported", ref.server.id, true) @@ -216,15 +230,11 @@ export function createModel(ctx: SetupContext) { browser: next.browser, embeds: next.embeds, suspended: next.suspended, - error: - next.error === "browser.pane.replaced" - ? ctx.t("replaced") - : next.error - ? ctx.t("common.requestFailed") - : undefined, + error: next.error ? describe(next.error) : undefined, }), ) - record(id, next) + + if (native) record(id, next) // After the store: closing a strip tab asks this model whether the desktop still has it. if (ref.location) return mirror() @@ -361,9 +371,11 @@ export function createModel(ctx: SetupContext) { if (state.attachments[id]) setState("attachments", id, "error", undefined) }) - // An unreachable pane is suspended, not a failed request. + // An unreachable pane is suspended, not a failed request. A tab that never opened has no address field to focus. const failed = (cause: unknown) => { - if (!unavailable(cause)) setState("errors", id, ctx.t("common.requestFailed")) + if (action.type === "tabs.open") opening.delete(id) + + if (!unavailable(cause)) setState("errors", id, describe(cause instanceof Error ? cause.message : "")) } const connection = live.get(id)?.connection diff --git a/packages/gui-extensions/src/browser/page-icon.tsx b/packages/gui-extensions/src/browser/page-icon.tsx new file mode 100644 index 00000000000..82655755be7 --- /dev/null +++ b/packages/gui-extensions/src/browser/page-icon.tsx @@ -0,0 +1,27 @@ +import { createSignal, Show } from "solid-js" +import { Icon } from "@opencode/ui/icon" + +/** + * A page's icon, or the globe while it has none or its image fails to decode. Clicks pass through to what holds it, so + * the address field focuses from its icon too. + */ +export function PageIcon(props: { icon: string | undefined; class?: string }) { + // The icon that failed; a new icon for the page is tried again. + const [broken, setBroken] = createSignal() + + return ( + } + > + {(icon) => ( + setBroken(icon())} + /> + )} + + ) +} diff --git a/packages/gui-extensions/src/browser/pane.ts b/packages/gui-extensions/src/browser/pane.ts index 5b5fa7f9d06..8b1ddb18be5 100644 --- a/packages/gui-extensions/src/browser/pane.ts +++ b/packages/gui-extensions/src/browser/pane.ts @@ -9,7 +9,7 @@ import type { Embeds, Persisted, ServerEndpoints, Storage, Windows } from "../sd import { createBrowserPage, type BrowserPage, type Shared } from "./chromium" import { browserFailure } from "./errors" import { createBrowserNetwork, type BrowserNetwork } from "./network" -import { destinationOrigin, fileURLWithin } from "./policy" +import { destinationOrigin, fileURLWithin, refusal } from "./policy" import { createRefs } from "./refs" import type { PaneEvent } from "./ipc" import { createBrowserRestoreStore } from "./restore" @@ -336,6 +336,11 @@ export function createBrowserPane(input: { }, async command(window: number, binding: string, command: Browser.Action) { const entry = owned(window, binding) + const url = command.type === "navigate" || command.type === "tabs.open" ? command.url : undefined + // The window explains a refused address by its code; the agent's commands get main's own message instead. + const reason = url === undefined ? undefined : refusal(url, { fileRoots: entry.fileRoots }) + + if (reason) throw new Error(reason) await execute(entry, { action: command, files: [] }, new AbortController().signal) }, async close(window: number, binding: string) { @@ -489,9 +494,10 @@ export function createBrowserPane(input: { inspect: (event) => { if (entry.pages.has(id)) report(entry, { type: "inspect", tabID: id, ...event }) }, - // Straight to the window: icons and zoom are local to this desktop, like the embed. + // Icons and zoom are local to this desktop, but wait behind the states reported before them, so the window files + // a new page's icon under that page's URL. detail: (value) => { - if (entry.pages.has(id)) publish(entry, { type: "page", tabID: id, ...value }) + if (entry.pages.has(id)) report(entry, { type: "page", tabID: id, ...value }) }, zoomed: () => entry.pages.forEach((page) => page.detail()), address: () => { diff --git a/packages/gui-extensions/src/browser/panel.fixture.tsx b/packages/gui-extensions/src/browser/panel.fixture.tsx index dfed61a7060..8e28f8afe29 100644 --- a/packages/gui-extensions/src/browser/panel.fixture.tsx +++ b/packages/gui-extensions/src/browser/panel.fixture.tsx @@ -48,6 +48,7 @@ import type definition from "./index" import { createModel } from "./model" import SessionBrowserPane from "./panel" import { BrowserPane, type PaneEvent } from "./ipc" +import { refusal } from "./policy" /** The renderer host pieces the pane runs on, passed in by `packages/app/component-tests/browser-pane.spec.ts`. */ type PaneHost = { @@ -287,7 +288,10 @@ export function mountBrowserPane(input: PaneHost) { load: async () => undefined, command: async (value) => { const command = value.command + // Main refuses what it does not open before anything happens, naming why. + const reason = command.type === "navigate" ? refusal(command.url) : undefined + if (reason) throw new Error(reason) setStore("error", undefined) if (command.type === "navigate" || command.type === "reload") setStore("loadErrors", store.session, undefined) @@ -440,7 +444,7 @@ export function mountBrowserPane(input: PaneHost) { + +
@@ -756,39 +807,26 @@ export default function SessionBrowserPane(props: { + {/* Zooming closes the menu: the page stays a still while the menu covers it, so a zoom would show + only once it closed. The address field shows any zoom but 100% afterwards. */} { - const tab = state() - - if (tab) props.model.zoom(props.session, tab.id, "in") - }} + onSelect={() => zoomPage("in")} shortcut={} > {extension.t("zoom.in")} { - const tab = state() - - if (tab) props.model.zoom(props.session, tab.id, "out") - }} + onSelect={() => zoomPage("out")} shortcut={} > {extension.t("zoom.out")} { - const tab = state() - - if (tab) props.model.zoom(props.session, tab.id, "reset") - }} + disabled={!zoomed()} + onSelect={() => zoomPage("reset")} badge={extension.t("zoom.percent", { percent: Math.round((page()?.zoom ?? 1) * 100) })} > {extension.t("zoom.reset")} @@ -812,7 +850,7 @@ export default function SessionBrowserPane(props: {