fix(desktop): fix browser bar review findings (#53627)

This commit is contained in:
Luke Parker
2026-10-07 09:50:13 +10:00
committed by GitHub
Unverified
parent d1749770c7
commit e41256eb5a
16 changed files with 427 additions and 137 deletions
@@ -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()
})
@@ -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"),
])
})
+17 -9
View File
@@ -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}`)
}
/**
@@ -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;
+37 -12
View File
@@ -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,
@@ -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
@@ -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",
+24 -8
View File
@@ -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 <img> runs no script and loads nothing, so it needs no redrawing.
// An SVG in an <img> 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 (!/<svg[\s>]/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
}
+23 -11
View File
@@ -96,8 +96,22 @@ export function createModel(ctx: SetupContext<typeof definition>) {
const recorded = new Map<string, string>()
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<Record<string, string>> }) =>
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<typeof definition>) {
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<typeof definition>) {
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<typeof definition>) {
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
@@ -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<string>()
return (
<Show
when={props.icon !== broken() ? props.icon : undefined}
fallback={<Icon name="outline-globe" class={`pointer-events-none size-4 shrink-0 ${props.class ?? ""}`} />}
>
{(icon) => (
<img
src={icon()}
alt=""
class="pointer-events-none size-4 shrink-0 object-contain"
onError={() => setBroken(icon())}
/>
)}
</Show>
)
}
+9 -3
View File
@@ -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: () => {
@@ -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) {
<button onClick={() => setStore("delayNavigation", true)}>Delay navigation</button>
<button
onClick={() => {
setStore({ error: "ERR_BLOCKED_BY_CLIENT", pendingURL: undefined })
setStore({ error: "browser.address.web", pendingURL: undefined })
report()
}}
>
+102 -73
View File
@@ -20,13 +20,16 @@ import { commentNote, pageNote } from "./comment"
import { bare, completes, highlight, suggest, type Visit } from "./history"
import type { Model } from "./model"
import type { PaneElement } from "./ipc"
import { PageIcon } from "./page-icon"
type PaneState = {
/** The address field's text while the user edits it, and the submitted address it keeps afterwards. */
address: string
editing: boolean
submitted: boolean
/** The report counts when a submitted address was kept; the next URL change or rejection shows the page's URL. */
/**
* The report counts when an address was submitted; the next URL change, finished load or rejection shows the
* page's URL.
*/
kept: { followed: number; rejected: number } | undefined
/** The movement count at a submit; the next reported movement ends the submitted navigation. */
navigating: number | undefined
@@ -89,7 +92,6 @@ export default function SessionBrowserPane(props: {
const [store, setStore] = createStore<PaneState>({
address: "",
editing: false,
submitted: false,
kept: undefined,
navigating: undefined,
typed: false,
@@ -102,8 +104,20 @@ export default function SessionBrowserPane(props: {
editorHeight: 0,
})
// Page reports, counted: a count moves on every report, even one that repeats a value. Tab switches and URL changes:
const followed = createMemo(on([() => state()?.id, address], (_input, _previous, count: number = 0) => count + 1))
// Finished loads, counted. A submitted address that lands on the URL the page already had (say "example.com" on
// https://example.com/) changes no URL, only this.
const settled = createMemo(
on(
() => !!state()?.loading,
(loading, previous, count: number = 0) => (previous && !loading ? count + 1 : count),
),
)
// Page reports, counted: a count moves on every report, even one that repeats a value. Tab switches, URL changes and
// finished loads:
const followed = createMemo(
on([() => state()?.id, address, settled], (_input, _previous, count: number = 0) => count + 1),
)
// Rejections: a blocked or rejected request leaves the page where it was.
const rejected = createMemo(
@@ -113,10 +127,10 @@ export default function SessionBrowserPane(props: {
),
)
// Any reported movement, including a rejected or blocked request.
// Any reported movement, including a rejected or blocked request. A command clearing the last error is not one.
const moved = createMemo(
on(
[() => state()?.id, () => state()?.generation, () => state()?.loading, () => props.model.error(props.session)],
[() => state()?.id, () => state()?.generation, () => state()?.loading, rejected],
(_input, _previous, count: number = 0) => count + 1,
),
)
@@ -221,12 +235,20 @@ export default function SessionBrowserPane(props: {
}
// Navigates the tab and keeps the submitted address in the field until the page reports where it went; a blank page
// keeps the field empty.
// keeps the field empty. Ends editing here rather than on blur: a click elsewhere, such as a recent page, may have
// blurred the field already.
const go = (url: string) => {
const tab = state()
if (!tab) return
setStore({ submitted: true, address: url === "about:blank" ? "" : url, navigating: moved() })
setStore({
editing: false,
typed: false,
active: -1,
address: url === "about:blank" ? "" : url,
navigating: moved(),
kept: { followed: followed(), rejected: rejected() },
})
command({ type: "navigate", tabID: tab.id, url })
input?.blur()
}
@@ -276,6 +298,9 @@ export default function SessionBrowserPane(props: {
if (!tab || !address()) return
// The editor is already open: the button keeps what the user wrote.
if (noting()) return
if (picking() && store.picking) setPicking(store.picking, false)
closeComment()
setStore("note", { sessionKey: props.session.key, tabID: tab.id, url: tab.url, title: tab.title, draft: "" })
@@ -298,6 +323,14 @@ export default function SessionBrowserPane(props: {
closeNote()
}
const zoomed = () => !!address() && (page()?.zoom ?? 1) !== 1
const zoomPage = (direction: "in" | "out" | "reset") => {
const tab = state()
if (tab) props.model.zoom(props.session, tab.id, direction)
}
const copyLink = () => {
const url = address()
@@ -359,30 +392,30 @@ export default function SessionBrowserPane(props: {
// The pane stays mounted when another session is routed; it listens to the routed session's picker.
createKeyed(
() => props.session,
(session) =>
() => props.session.key,
(sessionKey) =>
onCleanup(
props.model.onInspect(session, (event) => {
props.model.onInspect({ key: sessionKey }, (event) => {
if (event.active) {
setStore("picking", { sessionKey: session.key, tabID: event.tabID })
setStore("picking", { sessionKey, tabID: event.tabID })
return
}
if (store.picking?.sessionKey === session.key && store.picking.tabID === event.tabID)
if (store.picking?.sessionKey === sessionKey && store.picking.tabID === event.tabID)
setStore("picking", undefined)
if (!event.element) return
const tab = state()
if (tab?.id !== event.tabID || !visible()) {
props.model.highlight(session, event.tabID)
props.model.highlight({ key: sessionKey }, event.tabID)
return
}
setStore("comment", {
sessionKey: session.key,
sessionKey,
tabID: tab.id,
url: tab.url,
generation: tab.generation,
@@ -395,10 +428,10 @@ export default function SessionBrowserPane(props: {
// The page's address shortcut moves focus to this field when the page is the one on screen.
createKeyed(
() => props.session,
(session) =>
() => props.session.key,
(sessionKey) =>
onCleanup(
props.model.onAddress(session, (tabID) => {
props.model.onAddress({ key: sessionKey }, (tabID) => {
if (tabID === state()?.id && visible()) focusAddress()
}),
),
@@ -574,23 +607,29 @@ export default function SessionBrowserPane(props: {
<Show
when={site()}
fallback={
<PageIcon icon={store.editing && field() ? (store.typed ? chosen()?.icon : page()?.icon) : undefined} />
<PageIcon
icon={store.editing && field() ? (store.typed ? chosen()?.icon : page()?.icon) : undefined}
class="text-v2-icon-icon-muted"
/>
}
>
<SiteInformation
url={address()}
server={props.session.server.local ? undefined : props.session.server.name}
site={() => {
const tab = state()
{/* Site information describes one page: another tab or URL closes it, and opening it reads anew. */}
<Show when={`${props.session.key}\n${state()?.id}\n${address()}`} keyed>
<SiteInformation
url={address()}
server={props.session.server.local ? undefined : props.session.server.name}
site={() => {
const tab = state()
return tab ? props.model.site(props.session, tab.id) : undefined
}}
clear={() => {
const tab = state()
return tab ? props.model.site(props.session, tab.id) : undefined
}}
clear={() => {
const tab = state()
return tab ? props.model.clearSite(props.session, tab.id) : undefined
}}
/>
return tab ? props.model.clearSite(props.session, tab.id) : undefined
}}
/>
</Show>
</Show>
<div data-slot="browser-address-text">
<input
@@ -611,15 +650,10 @@ export default function SessionBrowserPane(props: {
setStore({ editing: true, address: field(), typed: false, active: -1, dismissed: false })
event.currentTarget.select()
}}
onBlur={() =>
setStore({
editing: false,
submitted: false,
typed: false,
active: -1,
kept: store.submitted ? { followed: followed(), rejected: rejected() } : undefined,
})
}
// Leaving the field without submitting cancels the edit; a submit already ended it.
onBlur={() => {
if (store.editing) setStore({ editing: false, typed: false, active: -1, kept: undefined })
}}
onInput={(event) => {
const value = event.currentTarget.value
const first = suggest(props.model.visits(), value, 1)[0]
@@ -631,6 +665,10 @@ export default function SessionBrowserPane(props: {
setStore({ address: value, typed: true, dismissed: false, active: complete ? 0 : -1 })
}}
onKeyDown={(event) => {
// Keys during IME composition pick and cancel candidates, not suggestions. Safari can report the
// composition-confirming keydown with isComposing false but keyCode 229.
if (event.isComposing || event.keyCode === 229) return
if (event.key === "ArrowDown" || event.key === "ArrowUp") {
if (!suggestions().length) return
event.preventDefault()
@@ -662,12 +700,25 @@ export default function SessionBrowserPane(props: {
<Show when={store.editing && store.address.length <= 48 && chosen()}>
{(visit) => (
<div aria-hidden="true" data-slot="browser-address-hint">
<span class="invisible">{store.address}</span>
<span class="ms-1.5">{`- ${bare(visit().url)}`}</span>
<span class="invisible shrink-0">{store.address}</span>
<span class="ms-1.5 min-w-0 truncate">{`- ${bare(visit().url)}`}</span>
</div>
)}
</Show>
</div>
{/* Any zoom but 100% stays in sight, as the system browser shows it; a click resets it. */}
<Show when={zoomed()}>
<Tooltip placement="top" value={extension.t("zoom.reset")}>
<button
type="button"
data-slot="browser-address-zoom"
onPointerDown={(event) => event.preventDefault()}
onClick={() => zoomPage("reset")}
>
{extension.t("zoom.percent", { percent: Math.round((page()?.zoom ?? 1) * 100) })}
</button>
</Tooltip>
</Show>
<Show when={address()}>
<div data-slot="browser-address-actions">
<Tooltip placement="top" value={extension.t("page.add")}>
@@ -756,39 +807,26 @@ export default function SessionBrowserPane(props: {
</Tooltip>
<Menu.Portal>
<Menu.Content class="min-w-56">
{/* 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. */}
<Menu.Group>
<Menu.Item
closeOnSelect={false}
disabled={!address()}
onSelect={() => {
const tab = state()
if (tab) props.model.zoom(props.session, tab.id, "in")
}}
onSelect={() => zoomPage("in")}
shortcut={<Keybind keys={[...keybinds.keys("mod+plus")]} variant="neutral" />}
>
{extension.t("zoom.in")}
</Menu.Item>
<Menu.Item
closeOnSelect={false}
disabled={!address()}
onSelect={() => {
const tab = state()
if (tab) props.model.zoom(props.session, tab.id, "out")
}}
onSelect={() => zoomPage("out")}
shortcut={<Keybind keys={[...keybinds.keys("mod+-")]} variant="neutral" />}
>
{extension.t("zoom.out")}
</Menu.Item>
<Menu.Item
closeOnSelect={false}
disabled={!address() || (page()?.zoom ?? 1) === 1}
onSelect={() => {
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: {
</div>
<Show when={error() && !failed()}>
<div
class="shrink-0 px-3 py-1.5 text-12-regular text-text-danger-base border-b border-v2-border-border-muted"
class="shrink-0 px-3 py-1.5 text-12-regular text-v2-state-fg-danger border-b border-v2-border-border-muted"
role="alert"
aria-live="assertive"
>
@@ -987,15 +1025,6 @@ export default function SessionBrowserPane(props: {
)
}
/** A page's icon, or the globe while it has none. */
function PageIcon(props: { icon: string | undefined }) {
return (
<Show when={props.icon} fallback={<Icon name="outline-globe" class="size-4 shrink-0 text-v2-icon-icon-muted" />}>
{(icon) => <img src={icon()} alt="" class="size-4 shrink-0 object-contain" />}
</Show>
)
}
/** A page in the suggestions or the new tab page: its icon, title and address, with the query's matches drawn. */
function PageRow(props: {
visit: Visit
@@ -1006,7 +1035,7 @@ function PageRow(props: {
}) {
const body = () => (
<>
<PageIcon icon={props.visit.icon} />
<PageIcon icon={props.visit.icon} class="text-v2-icon-icon-muted" />
<span data-slot="browser-page-row-text">
<Show when={props.visit.title}>
<span data-slot="browser-page-row-title">
@@ -1,5 +1,5 @@
import { expect, test } from "bun:test"
import { allowedDestination, destinationOrigin, fileURLWithin, localFileURL, normalizeURL } from "./policy"
import { allowedDestination, destinationOrigin, fileURLWithin, localFileURL, normalizeURL, refusal } from "./policy"
test("allows cross-origin HTTP navigation but rejects unsafe destinations and embedded credentials", () => {
expect(destinationOrigin("https://other.example/path")).toBe("https://other.example")
@@ -39,6 +39,37 @@ test("file documents load only from allowed workspace roots and never from a hos
expect(() => normalizeURL("file:///home/me/repo/x.html")).toThrow()
expect(() => normalizeURL("file:///etc/passwd", { fileRoots: roots })).toThrow()
expect(normalizeURL("localhost:3000", { fileRoots: roots })).toBe("http://localhost:3000")
expect(normalizeURL("app.localhost:3000/")).toBe("http://app.localhost:3000/")
expect(normalizeURL(" ABOUT:BLANK ")).toBe("about:blank")
})
test("the pane names why it refuses an address, so the window can explain it", () => {
const roots = ["/home/me/repo"]
expect(
[
"example.com",
"about:blank",
"file:///home/me/repo/x.html",
"https://user:pass@example.com",
"file:///etc/passwd",
"file://server/home/me/repo/x.html",
"ftp://example.com",
"about:config",
].map((url) => refusal(url, { fileRoots: roots })),
).toEqual([
undefined,
undefined,
undefined,
"browser.address.credentials",
"browser.address.workspace",
"browser.address.workspace",
"browser.address.workspace",
"browser.address.workspace",
])
// Where no files open, only web pages do.
expect(refusal("file:///home/me/repo/x.html")).toBe("browser.address.web")
expect(() => normalizeURL("https://user:pass@example.com")).toThrow("user name or password")
})
test("windows workspace roots match drive-letter file URLs", () => {
+34 -5
View File
@@ -49,13 +49,13 @@ export function allowedDestination(input: string, policy?: Policy) {
}
export function normalizeURL(input: string, policy?: Policy) {
const value = input.trim() || "about:blank"
const local = /^(?:localhost|127(?:\.\d{1,3}){3}|\[::1\])(?::\d+)?(?:[/?#]|$)/i.test(value)
const url = withScheme(input)
const reason = refusal(url, policy)
const url =
value === "about:blank" || /^[a-z][a-z\d+.-]*:\/\//i.test(value) ? value : `${local ? "http" : "https"}://${value}`
if (reason === "browser.address.credentials")
throw new Error("URLs with a user name or password are not supported. Remove them from the URL.")
if (url !== "about:blank" && !allowedDestination(url, policy))
if (reason)
throw new Error(
policy?.fileRoots?.length
? "Only HTTP, HTTPS, about:blank, and file URLs inside the workspace are supported."
@@ -64,3 +64,32 @@ export function normalizeURL(input: string, policy?: Policy) {
return url
}
/**
* Why the pane does not open an address, as the code the window explains it with; undefined when it opens.
* - `browser.address.credentials`: a web page address with a user name or password.
* - `browser.address.workspace`: anything but a web page or a file in the workspace.
* - `browser.address.web`: anything but a web page, where no files open.
*/
export function refusal(input: string, policy?: Policy) {
const url = withScheme(input)
if (url === "about:blank" || allowedDestination(url, policy)) return
// A web page is refused only for its credentials.
if (URL.canParse(url) && /^https?:$/.test(new URL(url).protocol)) return "browser.address.credentials"
return policy?.fileRoots?.length ? "browser.address.workspace" : "browser.address.web"
}
// The URL an address opens: a blank page for none, else the address with a scheme, `http` for this machine's names.
function withScheme(input: string) {
const value = input.trim()
if (!value || /^about:blank$/i.test(value)) return "about:blank"
if (/^[a-z][a-z\d+.-]*:\/\//i.test(value)) return value
const local = /^(?:(?:[a-z\d-]+\.)*localhost|127(?:\.\d{1,3}){3}|\[::1\])(?::\d+)?(?:[/?#]|$)/i.test(value)
return `${local ? "http" : "https"}://${value}`
}
@@ -1,9 +1,9 @@
import { createMemo, createSignal, getOwner, lazy, onCleanup, runWithOwner, Show, Suspense } from "solid-js"
import { Icon } from "@opencode/ui/icon"
import { Command, createKeyed, LinkHandler, MenuItem, onIdle, Panel, Style, type PanelTab, type Setup } from "../sdk"
import { Browser } from "./contract"
import type definition from "./index"
import type { Model } from "./model"
import { PageIcon } from "./page-icon"
import barStyles from "./bar.css?inline"
import commentStyles from "./comment.css?inline"
import tabStyles from "./tabs.css?inline"
@@ -153,9 +153,7 @@ const setup: Setup<typeof definition> = (ctx) => {
},
label: () => (
<div class="flex items-center gap-1.5">
<Show when={icon()} fallback={<Icon name="outline-globe" class="shrink-0" />}>
{(source) => <img src={source()} alt="" class="size-4 shrink-0 object-contain" />}
</Show>
<PageIcon icon={icon()} />
<span class="max-w-40 truncate">{text()}</span>
</div>
),