fix(browser): restore configured zoom after page reload (#10800)
Reviewed with an independent reproduction. Rewrote the reload-zoom reassert to be per-pane instead of sharing the value zoom in/out writes, fixing Cmd/Ctrl+0 reset and cross-tab zoom leakage, with E2E coverage proven to fail on revert.
This commit is contained in:
Luiz Eduardo de Oliveira Fonseca
GitHub
parent
872a9c3930
commit
60c7faf930
|
|
@ -2827,8 +2827,10 @@ function BrowserPagePane({
|
|||
const setBrowserDefaultZoomLevel = useAppStore((state) => state.setBrowserDefaultZoomLevel)
|
||||
const normalizedBrowserDefaultZoomLevel = normalizeBrowserPageZoomLevel(browserDefaultZoomLevel)
|
||||
const browserDefaultZoomPercent = browserPageZoomLevelToPercent(normalizedBrowserDefaultZoomLevel)
|
||||
const browserDefaultZoomLevelRef = useRef(normalizedBrowserDefaultZoomLevel)
|
||||
browserDefaultZoomLevelRef.current = normalizedBrowserDefaultZoomLevel
|
||||
// Why: the level THIS pane should hold. Seeded once from the configured default ("applied to newly
|
||||
// opened browser tabs") and moved only by zooming this pane, so a reload can't broadcast another
|
||||
// tab's zoom through the shared setting.
|
||||
const paneZoomLevelRef = useRef(normalizedBrowserDefaultZoomLevel)
|
||||
const grabElementShortcut = useShortcutLabel('browser.grabElement')
|
||||
const faviconUrlRef = useRef<string | null>(browserTab.faviconUrl)
|
||||
const initialBrowserUrlRef = useRef(browserTab.url)
|
||||
|
|
@ -3560,8 +3562,10 @@ function BrowserPagePane({
|
|||
if (!isActiveRef.current) {
|
||||
return
|
||||
}
|
||||
// Why: reset targets 100% like Chromium; the configured default is a new-tab seed, not a reset target.
|
||||
const nextLevel = applyBrowserPageZoom(webviewRef.current, direction)
|
||||
if (nextLevel !== null) {
|
||||
paneZoomLevelRef.current = nextLevel
|
||||
setBrowserDefaultZoomLevel(nextLevel)
|
||||
showBrowserZoomFeedback(nextLevel)
|
||||
}
|
||||
|
|
@ -3659,7 +3663,6 @@ function BrowserPagePane({
|
|||
container = ensuredWebview.container
|
||||
const webview = ensuredWebview.webview
|
||||
const needsInitialNavigation = ensuredWebview.created
|
||||
let needsInitialDefaultZoom = ensuredWebview.created
|
||||
|
||||
if (!ensuredWebview.created) {
|
||||
// pointerEvents already applied inside ensureBrowserPageWebview for the reused-webview path.
|
||||
|
|
@ -3739,12 +3742,12 @@ function BrowserPagePane({
|
|||
if (!queuedAnnotationViewportBridgeSync) {
|
||||
syncBrowserAnnotationViewportBridge()
|
||||
}
|
||||
if (needsInitialDefaultZoom) {
|
||||
const appliedLevel = setBrowserPageZoomLevel(webview, browserDefaultZoomLevelRef.current)
|
||||
if (appliedLevel !== null) {
|
||||
setBrowserZoomPercent(browserPageZoomLevelToPercent(appliedLevel))
|
||||
}
|
||||
needsInitialDefaultZoom = false
|
||||
// Why: Chromium restores per-origin zoom on reload/navigation, so reassert THIS pane's level after
|
||||
// every guest load. Uses the pane-local level, not the shared setting, so reloading one tab never
|
||||
// adopts a zoom the user applied to a different tab.
|
||||
const appliedLevel = setBrowserPageZoomLevel(webview, paneZoomLevelRef.current)
|
||||
if (appliedLevel !== null) {
|
||||
setBrowserZoomPercent(browserPageZoomLevelToPercent(appliedLevel))
|
||||
}
|
||||
// Why: CDP viewport overrides are scoped to the debugger session and don't survive cross-origin nav, so reapply (idempotently) on dom-ready.
|
||||
const presetId = viewportPresetIdRef.current
|
||||
|
|
|
|||
|
|
@ -106,6 +106,121 @@ describe('setBrowserPageZoomLevel', () => {
|
|||
expect(setBrowserPageZoomLevel(webview, 1.26)).toBe(1.5)
|
||||
expect(webview.setZoomLevel).toHaveBeenCalledWith(1.5)
|
||||
})
|
||||
|
||||
it('restores the configured level when Chromium carries zoom across reloads', () => {
|
||||
const webview = {
|
||||
getZoomLevel: vi.fn(() => 0.5),
|
||||
setZoomLevel: vi.fn()
|
||||
}
|
||||
|
||||
expect(setBrowserPageZoomLevel(webview, 0)).toBe(0)
|
||||
expect(setBrowserPageZoomLevel(webview, 0)).toBe(0)
|
||||
expect(webview.setZoomLevel).toHaveBeenNthCalledWith(1, 0)
|
||||
expect(webview.setZoomLevel).toHaveBeenNthCalledWith(2, 0)
|
||||
})
|
||||
})
|
||||
|
||||
/**
|
||||
* Models BrowserPagePane's zoom wiring: each pane keeps its own level, dom-ready reasserts
|
||||
* that level, and zooming also writes the shared `browserDefaultZoomLevel` setting.
|
||||
*/
|
||||
describe('browser pane zoom across reloads', () => {
|
||||
function createPane(sharedSetting: { level: number }) {
|
||||
// Chromium remembers zoom per origin and replays it on reload.
|
||||
const originZoom = new Map<string, number>()
|
||||
let url = 'https://a.example'
|
||||
let live = 0
|
||||
const webview = {
|
||||
getZoomLevel: () => live,
|
||||
setZoomLevel: (level: number) => {
|
||||
live = level
|
||||
originZoom.set(url, level)
|
||||
}
|
||||
}
|
||||
let paneLevel = sharedSetting.level
|
||||
webview.setZoomLevel(paneLevel)
|
||||
|
||||
return {
|
||||
get level() {
|
||||
return live
|
||||
},
|
||||
zoom(direction: 'in' | 'out' | 'reset') {
|
||||
const next = applyBrowserPageZoom(webview, direction)
|
||||
if (next !== null) {
|
||||
paneLevel = next
|
||||
sharedSetting.level = next
|
||||
}
|
||||
},
|
||||
load(nextUrl = url) {
|
||||
url = nextUrl
|
||||
live = originZoom.get(url) ?? 0
|
||||
setBrowserPageZoomLevel(webview, paneLevel)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
it('reasserts the pane level after normal and hard reloads', () => {
|
||||
const setting = { level: 0 }
|
||||
const pane = createPane(setting)
|
||||
|
||||
// Chromium hands back a stale 150% for this origin on reload.
|
||||
pane.load()
|
||||
expect(pane.level).toBe(0)
|
||||
pane.load()
|
||||
expect(pane.level).toBe(0)
|
||||
})
|
||||
|
||||
it('keeps an explicit non-default configured zoom across a reload', () => {
|
||||
const setting = { level: 1.5 }
|
||||
const pane = createPane(setting)
|
||||
expect(pane.level).toBe(1.5)
|
||||
|
||||
pane.load()
|
||||
expect(pane.level).toBe(1.5)
|
||||
})
|
||||
|
||||
it('resets to 100% on reset even after zooming moved the shared setting', () => {
|
||||
const setting = { level: 0 }
|
||||
const pane = createPane(setting)
|
||||
|
||||
pane.zoom('in')
|
||||
pane.zoom('in')
|
||||
expect(pane.level).toBe(1)
|
||||
expect(setting.level).toBe(1)
|
||||
|
||||
// Regression: resetting toward the shared setting would be a fixed point and never move.
|
||||
pane.zoom('reset')
|
||||
expect(pane.level).toBe(0)
|
||||
})
|
||||
|
||||
it('does not adopt another tab zoom when reloading an untouched tab', () => {
|
||||
const setting = { level: 0 }
|
||||
const tabA = createPane(setting)
|
||||
const tabB = createPane(setting)
|
||||
|
||||
tabB.zoom('in')
|
||||
tabB.zoom('in')
|
||||
expect(tabB.level).toBe(1)
|
||||
expect(setting.level).toBe(1)
|
||||
expect(tabA.level).toBe(0)
|
||||
|
||||
// Regression: reasserting the shared setting would silently zoom tab A to tab B's level.
|
||||
tabA.load()
|
||||
expect(tabA.level).toBe(0)
|
||||
})
|
||||
|
||||
it('keeps a zoomed tab at its own level across reload and cross-origin navigation', () => {
|
||||
const setting = { level: 0 }
|
||||
const pane = createPane(setting)
|
||||
|
||||
pane.zoom('in')
|
||||
expect(pane.level).toBe(0.5)
|
||||
|
||||
pane.load()
|
||||
expect(pane.level).toBe(0.5)
|
||||
pane.load('https://b.example')
|
||||
expect(pane.level).toBe(0.5)
|
||||
})
|
||||
})
|
||||
|
||||
describe('getBrowserPageZoomIndicatorState', () => {
|
||||
|
|
|
|||
|
|
@ -97,7 +97,7 @@ async function switchToBrowserTab(
|
|||
)
|
||||
}
|
||||
|
||||
async function startBrowserFormServer(): Promise<{
|
||||
async function startBrowserFormServer(host = '127.0.0.1'): Promise<{
|
||||
url: (label: string) => string
|
||||
close: () => Promise<void>
|
||||
}> {
|
||||
|
|
@ -113,10 +113,10 @@ async function startBrowserFormServer(): Promise<{
|
|||
</html>
|
||||
`)
|
||||
})
|
||||
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
|
||||
await new Promise<void>((resolve) => server.listen(0, host, resolve))
|
||||
const port = (server.address() as AddressInfo).port
|
||||
return {
|
||||
url: (label: string) => `http://127.0.0.1:${port}/${encodeURIComponent(label)}`,
|
||||
url: (label: string) => `http://${host}:${port}/${encodeURIComponent(label)}`,
|
||||
close: () => closeServer(server)
|
||||
}
|
||||
}
|
||||
|
|
@ -461,6 +461,160 @@ test.describe('Browser Tab', () => {
|
|||
}
|
||||
})
|
||||
|
||||
test('browser page reload restores the configured 100% zoom', async ({ orcaPage }) => {
|
||||
const formServer = await startBrowserFormServer()
|
||||
try {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
const browserTab = await createBrowserTab(
|
||||
orcaPage,
|
||||
worktreeId,
|
||||
formServer.url('Zoom reload'),
|
||||
'Zoom Reload'
|
||||
)
|
||||
expect(browserTab?.id).toBeTruthy()
|
||||
await expect
|
||||
.poll(async () => readBrowserInputValue(orcaPage, browserTab!.id), { timeout: 5_000 })
|
||||
.not.toBeNull()
|
||||
|
||||
const zoomLevels = await orcaPage.evaluate(async (browserTabId) => {
|
||||
const slot = document.querySelector(`[data-browser-overlay-tab-id="${browserTabId}"]`)
|
||||
const webview = slot?.querySelector('webview') as Electron.WebviewTag | null
|
||||
if (!webview) {
|
||||
throw new Error(`Missing webview for browser tab ${browserTabId}`)
|
||||
}
|
||||
|
||||
const levels = [webview.getZoomLevel()]
|
||||
webview.setZoomLevel(0.5)
|
||||
for (let reload = 0; reload < 3; reload += 1) {
|
||||
await new Promise<void>((resolve) => {
|
||||
webview.addEventListener('dom-ready', () => resolve(), { once: true })
|
||||
if (reload === 1) {
|
||||
webview.reloadIgnoringCache()
|
||||
} else {
|
||||
webview.reload()
|
||||
}
|
||||
})
|
||||
levels.push(webview.getZoomLevel())
|
||||
}
|
||||
return levels
|
||||
}, browserTab!.id)
|
||||
|
||||
expect(zoomLevels).toEqual([0, 0, 0, 0])
|
||||
} finally {
|
||||
await formServer.close()
|
||||
}
|
||||
})
|
||||
|
||||
test('Cmd/Ctrl+0 resets a zoomed browser page to 100%', async ({ orcaPage }) => {
|
||||
const formServer = await startBrowserFormServer()
|
||||
try {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
const browserTab = await createBrowserTab(
|
||||
orcaPage,
|
||||
worktreeId,
|
||||
formServer.url('Zoom reset'),
|
||||
'Zoom Reset'
|
||||
)
|
||||
expect(browserTab?.id).toBeTruthy()
|
||||
await expect
|
||||
.poll(async () => readBrowserInputValue(orcaPage, browserTab!.id), { timeout: 5_000 })
|
||||
.not.toBeNull()
|
||||
|
||||
await orcaPage.evaluate(
|
||||
async ({ browserTabId, browserPageId, modifier }) => {
|
||||
const slot = document.querySelector(`[data-browser-overlay-tab-id="${browserTabId}"]`)
|
||||
const webview = slot?.querySelector('webview') as Electron.WebviewTag | null
|
||||
if (!webview) {
|
||||
throw new Error(`Missing webview for browser tab ${browserTabId}`)
|
||||
}
|
||||
window.dispatchEvent(
|
||||
new CustomEvent('orca:browser-page-zoom', {
|
||||
detail: { browserPageId, direction: 'in' }
|
||||
})
|
||||
)
|
||||
await webview.sendInputEvent({ type: 'keyDown', keyCode: '0', modifiers: [modifier] })
|
||||
await webview.sendInputEvent({ type: 'keyUp', keyCode: '0', modifiers: [modifier] })
|
||||
},
|
||||
{
|
||||
browserTabId: browserTab!.id,
|
||||
browserPageId: browserTab!.pageId ?? browserTab!.id,
|
||||
modifier: process.platform === 'darwin' ? 'meta' : 'control'
|
||||
}
|
||||
)
|
||||
await expect
|
||||
.poll(() =>
|
||||
orcaPage.evaluate((browserTabId) => {
|
||||
const slot = document.querySelector(`[data-browser-overlay-tab-id="${browserTabId}"]`)
|
||||
return (slot?.querySelector('webview') as Electron.WebviewTag | null)?.getZoomLevel()
|
||||
}, browserTab!.id)
|
||||
)
|
||||
.toBe(0)
|
||||
} finally {
|
||||
await formServer.close()
|
||||
}
|
||||
})
|
||||
|
||||
test('reloading one browser tab does not adopt another tab zoom', async ({ orcaPage }) => {
|
||||
const [formServerA, formServerB] = await Promise.all([
|
||||
startBrowserFormServer(),
|
||||
startBrowserFormServer('localhost')
|
||||
])
|
||||
try {
|
||||
const worktreeId = (await getActiveWorktreeId(orcaPage))!
|
||||
const tabA = await createBrowserTab(orcaPage, worktreeId, formServerA.url('Zoom A'), 'Zoom A')
|
||||
const tabB = await createBrowserTab(orcaPage, worktreeId, formServerB.url('Zoom B'), 'Zoom B')
|
||||
expect(tabA?.id).toBeTruthy()
|
||||
expect(tabB?.id).toBeTruthy()
|
||||
for (const tab of [tabA, tabB]) {
|
||||
await expect
|
||||
.poll(async () => readBrowserInputValue(orcaPage, tab!.id), { timeout: 5_000 })
|
||||
.not.toBeNull()
|
||||
}
|
||||
|
||||
const levels = await orcaPage.evaluate(
|
||||
async ({ tabAId, tabBId, pageBId }) => {
|
||||
const webviewFor = (id: string): Electron.WebviewTag => {
|
||||
const slot = document.querySelector(`[data-browser-overlay-tab-id="${id}"]`)
|
||||
const webview = slot?.querySelector('webview') as Electron.WebviewTag | null
|
||||
if (!webview) {
|
||||
throw new Error(`Missing webview for browser tab ${id}`)
|
||||
}
|
||||
return webview
|
||||
}
|
||||
const webviewA = webviewFor(tabAId)
|
||||
const webviewB = webviewFor(tabBId)
|
||||
|
||||
// Zoom only tab B through the real renderer zoom path (also writes the shared setting).
|
||||
for (let step = 0; step < 2; step += 1) {
|
||||
window.dispatchEvent(
|
||||
new CustomEvent('orca:browser-page-zoom', {
|
||||
detail: { browserPageId: pageBId, direction: 'in' }
|
||||
})
|
||||
)
|
||||
await new Promise((resolve) => setTimeout(resolve, 100))
|
||||
}
|
||||
const zoomedB = webviewB.getZoomLevel()
|
||||
const untouchedA = webviewA.getZoomLevel()
|
||||
|
||||
await new Promise<void>((resolve) => {
|
||||
webviewA.addEventListener('dom-ready', () => resolve(), { once: true })
|
||||
webviewA.reload()
|
||||
})
|
||||
|
||||
return { zoomedB, untouchedA, reloadedA: webviewA.getZoomLevel() }
|
||||
},
|
||||
{ tabAId: tabA!.id, tabBId: tabB!.id, pageBId: tabB!.pageId ?? tabB!.id }
|
||||
)
|
||||
|
||||
expect(levels.zoomedB).toBeGreaterThan(0)
|
||||
expect(levels.untouchedA).toBe(0)
|
||||
// Regression: reasserting the shared default would drag tab A to tab B's zoom.
|
||||
expect(levels.reloadedA).toBe(0)
|
||||
} finally {
|
||||
await Promise.all([formServerA.close(), formServerB.close()])
|
||||
}
|
||||
})
|
||||
|
||||
test('plain links stay current while explicit new-tab gestures activate Orca tabs', async ({
|
||||
electronApp,
|
||||
orcaPage
|
||||
|
|
|
|||
Loading…
Reference in New Issue