Skip to content

Commit 6117140

Browse files
committed
fix resizing issues + cookies source
1 parent bd59c5c commit 6117140

23 files changed

Lines changed: 1882 additions & 278 deletions

File tree

apps/desktop/src/main/browser-agent/context-menu.test.ts

Lines changed: 50 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ vi.mock('electron', () => import('@/test/electron-mock'))
66
import { Menu, WebContentsView } from 'electron'
77
import {
88
attachAgentContextMenu,
9+
BASE_ZOOM_FACTOR,
910
buildAgentContextMenuTemplate,
1011
steppedZoomFactor,
12+
zoomPercentOf,
1113
} from '@/main/browser-agent/context-menu'
1214

1315
const EDIT_FLAGS: ContextMenuParams['editFlags'] = {
@@ -30,7 +32,8 @@ function params(overrides: Partial<Params> = {}): Params {
3032
}
3133

3234
function page(overrides: Partial<Page> = {}): Page {
33-
return { canGoBack: true, canGoForward: true, zoomFactor: 1, ...overrides }
35+
// A fresh tab sits at the panel's baseline, which the menu reports as 100%.
36+
return { canGoBack: true, canGoForward: true, zoomFactor: BASE_ZOOM_FACTOR, ...overrides }
3437
}
3538

3639
function handlers(): Handlers {
@@ -130,7 +133,10 @@ describe('buildAgentContextMenuTemplate', () => {
130133
})
131134

132135
it('reports the current zoom and disables the ends of the ladder', () => {
133-
const stepped = buildAgentContextMenuTemplate(params(), page({ zoomFactor: 1.21 }), handlers())
136+
// Two rungs up from the baseline, reported against the baseline rather than
137+
// against Chromium's native scale (where this factor would read 110%).
138+
const twoUp = steppedZoomFactor(steppedZoomFactor(BASE_ZOOM_FACTOR, 1), 1)
139+
const stepped = buildAgentContextMenuTemplate(params(), page({ zoomFactor: twoUp }), handlers())
134140
expect(item(stepped, 'Actual Size (121%)')?.enabled).toBe(true)
135141

136142
const atMax = buildAgentContextMenuTemplate(params(), page({ zoomFactor: 3 }), handlers())
@@ -147,17 +153,16 @@ describe('buildAgentContextMenuTemplate', () => {
147153
).toBe(false)
148154
})
149155

150-
it('resets to exactly 100%, undoing accumulated drift', () => {
156+
it('resets to exactly the baseline, undoing accumulated drift', () => {
151157
const handled = handlers()
152-
const template = buildAgentContextMenuTemplate(
153-
params(),
154-
page({ zoomFactor: 1.3310000000000004 }),
155-
handled
156-
)
158+
// Three rungs of float multiplication up, so the factor no longer sits on a
159+
// clean value — reset has to restore the baseline exactly, not step back.
160+
const drifted = [1, 1, 1].reduce((factor) => steppedZoomFactor(factor, 1), BASE_ZOOM_FACTOR)
161+
const template = buildAgentContextMenuTemplate(params(), page({ zoomFactor: drifted }), handled)
157162

158163
item(template, 'Actual Size (133%)')?.click?.({} as never, undefined as never, {} as never)
159164

160-
expect(handled.setZoomFactor).toHaveBeenCalledWith(1)
165+
expect(handled.setZoomFactor).toHaveBeenCalledWith(BASE_ZOOM_FACTOR)
161166
})
162167

163168
it('never leaves a separator with nothing above it', () => {
@@ -215,8 +220,41 @@ describe('steppedZoomFactor', () => {
215220
expect(steppedZoomFactor(0.5, -1)).toBe(0.5)
216221
})
217222

218-
it('treats a nonsense factor as 100%', () => {
219-
expect(steppedZoomFactor(Number.NaN, 1)).toBe(1.1)
220-
expect(steppedZoomFactor(0, 1)).toBe(1.1)
223+
it('treats a nonsense factor as the baseline', () => {
224+
// One rung up from the baseline is Chromium's native 1.0.
225+
expect(steppedZoomFactor(Number.NaN, 1)).toBe(1)
226+
expect(steppedZoomFactor(0, 1)).toBe(1)
227+
})
228+
})
229+
230+
describe('BASE_ZOOM_FACTOR', () => {
231+
it('renders a rung below native but reads as 100%', () => {
232+
expect(BASE_ZOOM_FACTOR).toBeCloseTo(0.909, 3)
233+
expect(zoomPercentOf(BASE_ZOOM_FACTOR)).toBe(100)
234+
})
235+
236+
it('keeps the ladder landing exactly on Chromium native one step up', () => {
237+
expect(steppedZoomFactor(BASE_ZOOM_FACTOR, 1)).toBe(1)
238+
expect(zoomPercentOf(1)).toBe(110)
239+
})
240+
241+
it('stays inside the ladder, so the page menu can still step both ways', () => {
242+
expect(steppedZoomFactor(BASE_ZOOM_FACTOR, 1)).not.toBe(BASE_ZOOM_FACTOR)
243+
expect(steppedZoomFactor(BASE_ZOOM_FACTOR, -1)).not.toBe(BASE_ZOOM_FACTOR)
244+
})
245+
})
246+
247+
describe('zoomPercentOf', () => {
248+
it('reports every rung relative to the panel baseline, not to native', () => {
249+
expect(zoomPercentOf(steppedZoomFactor(BASE_ZOOM_FACTOR, -1))).toBe(91)
250+
expect(zoomPercentOf(BASE_ZOOM_FACTOR)).toBe(100)
251+
expect(zoomPercentOf(steppedZoomFactor(BASE_ZOOM_FACTOR, 1))).toBe(110)
252+
})
253+
254+
it('still reads 100% after a round trip up and back down', () => {
255+
// The ladder is float arithmetic, so the reset item's `!== 100` guard has to
256+
// survive a step that does not return bit-identically to the baseline.
257+
const roundTripped = steppedZoomFactor(steppedZoomFactor(BASE_ZOOM_FACTOR, 1), -1)
258+
expect(zoomPercentOf(roundTripped)).toBe(100)
221259
})
222260
})

apps/desktop/src/main/browser-agent/context-menu.ts

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,50 @@ import type { ContextMenuParams, MenuItemConstructorOptions, WebContents } from
1717
import { clipboard, Menu } from 'electron'
1818

1919
/**
20-
* Page-zoom ladder for the embedded browser, in factors (1 = 100%) because that
21-
* is what the menu displays.
20+
* Page-zoom ladder for the embedded browser, in Chromium's absolute zoom
21+
* factors. The ends are the platform's own limits, not a product choice —
22+
* Chromium refuses to scale past them, and a rung outside the range would come
23+
* back clamped and leave the menu offering a step that never lands.
2224
*/
2325
const ZOOM_STEP_RATIO = 1.1
2426
const MIN_ZOOM_FACTOR = 0.5
2527
const MAX_ZOOM_FACTOR = 3
2628

29+
/**
30+
* What the panel calls 100%.
31+
*
32+
* The browser lives in a panel that is only ever a fraction of the window, so
33+
* it renders a rung below Chromium's native scale and treats THAT as its
34+
* baseline: the menu reads 100% there, and every other rung is reported
35+
* relative to it. Users get a zoom control that behaves the way one should —
36+
* starts at 100%, resets to 100% — over a page that is genuinely rendering at
37+
* ~91% of native.
38+
*
39+
* Defined as one rung below native rather than as a round number so the ladder
40+
* still lands exactly on Chromium's 1.0 (the crispest rasterization, one step
41+
* up from the baseline) instead of straddling it.
42+
*/
43+
export const BASE_ZOOM_FACTOR = 1 / ZOOM_STEP_RATIO
44+
45+
/**
46+
* A Chromium zoom factor as a percentage of {@link BASE_ZOOM_FACTOR} — what the
47+
* menu shows and what "100%" means everywhere in the browser panel's UI.
48+
*
49+
* Only display and the reset target convert; the ladder itself stays in
50+
* absolute factors, so stepping never round-trips through this and cannot
51+
* accumulate float drift away from the rungs.
52+
*/
53+
export function zoomPercentOf(factor: number): number {
54+
return Math.round((factor / BASE_ZOOM_FACTOR) * 100)
55+
}
56+
2757
/**
2858
* One step along the zoom ladder, clamped to its ends. Returning the current
2959
* factor unchanged is how the menu knows an end is reached, so it can disable
3060
* the item rather than offer a step that does nothing.
3161
*/
3262
export function steppedZoomFactor(current: number, direction: 1 | -1): number {
33-
const base = Number.isFinite(current) && current > 0 ? current : 1
63+
const base = Number.isFinite(current) && current > 0 ? current : BASE_ZOOM_FACTOR
3464
const next = direction === 1 ? base * ZOOM_STEP_RATIO : base / ZOOM_STEP_RATIO
3565
return Math.min(MAX_ZOOM_FACTOR, Math.max(MIN_ZOOM_FACTOR, next))
3666
}
@@ -107,7 +137,7 @@ export function buildAgentContextMenuTemplate(
107137

108138
const zoomIn = steppedZoomFactor(page.zoomFactor, 1)
109139
const zoomOut = steppedZoomFactor(page.zoomFactor, -1)
110-
const zoomPercent = Math.round(page.zoomFactor * 100)
140+
const zoomPercent = zoomPercentOf(page.zoomFactor)
111141
template.push(
112142
{
113143
label: 'Zoom In',
@@ -122,7 +152,7 @@ export function buildAgentContextMenuTemplate(
122152
{
123153
label: `Actual Size (${zoomPercent}%)`,
124154
enabled: zoomPercent !== 100,
125-
click: () => handlers.setZoomFactor(1),
155+
click: () => handlers.setZoomFactor(BASE_ZOOM_FACTOR),
126156
}
127157
)
128158

apps/desktop/src/main/browser-agent/session.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { createLogger } from '@sim/logger'
1111
import { getErrorMessage } from '@sim/utils/errors'
1212
import type { BrowserWindow, CookiesSetDetails, Input, Session, WebContents } from 'electron'
1313
import { session as electronSession, nativeTheme, WebContentsView } from 'electron'
14-
import { attachAgentContextMenu } from '@/main/browser-agent/context-menu'
14+
import { attachAgentContextMenu, BASE_ZOOM_FACTOR } from '@/main/browser-agent/context-menu'
1515
import type { BrowserCookieSignal } from '@/main/browser-agent/known-sessions'
1616
import {
1717
detachAttachedView,
@@ -340,6 +340,9 @@ function createTabView(): WebContentsView {
340340
// applyActiveTabThrottling — never blanket across every tab.
341341
backgroundThrottling: true,
342342
spellcheck: false,
343+
// The default every origin this tab visits starts at; a per-origin zoom
344+
// the user sets from the page menu still wins and still persists.
345+
zoomFactor: BASE_ZOOM_FACTOR,
343346
},
344347
})
345348
view.setBackgroundColor(browserBackgroundColor())

apps/desktop/src/main/browser-import/chromium-cookies.test.ts

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { createCipheriv, createHash } from 'node:crypto'
22
import { mkdtemp, readdir, rm, stat, writeFile } from 'node:fs/promises'
33
import { tmpdir } from 'node:os'
44
import { join } from 'node:path'
5+
import { sleep } from '@sim/utils/helpers'
56
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
67
import { readBrowserCookies } from '@/main/browser-import/chromium-cookies'
78
import { deriveEncryptionKey } from '@/main/browser-import/chromium-crypto'
@@ -19,6 +20,29 @@ const sqliteAvailable = await import('node:sqlite').then(
1920
const KEY = deriveEncryptionKey('test-safe-storage-password')
2021
const NOW_SECONDS = 1_800_000_000
2122

23+
async function stagingDirectories(): Promise<Set<string>> {
24+
const entries = await readdir(tmpdir())
25+
return new Set(entries.filter((entry) => entry.startsWith('sim-chrome-import-')))
26+
}
27+
28+
/**
29+
* Staging copies still present that were not there at `before`.
30+
*
31+
* Every reader in this folder stages through the shared `os.tmpdir()` under one
32+
* prefix, and Vitest runs their suites in parallel workers, so a single snapshot
33+
* can catch a sibling's copy mid-read and read as a leak. A leak of our own
34+
* never clears, so this settles instead of sampling once.
35+
*/
36+
async function stagingLeftBehindSince(before: ReadonlySet<string>): Promise<string[]> {
37+
let leftover: string[] = []
38+
for (let attempt = 0; attempt < 20; attempt++) {
39+
leftover = [...(await stagingDirectories())].filter((entry) => !before.has(entry))
40+
if (leftover.length === 0) return leftover
41+
await sleep(25)
42+
}
43+
return leftover
44+
}
45+
2246
function chromeTime(unixSeconds: number): bigint {
2347
return BigInt(unixSeconds + 11_644_473_600) * 1_000_000n
2448
}
@@ -204,32 +228,22 @@ describe.skipIf(!sqliteAvailable)('readBrowserCookies', () => {
204228
})
205229

206230
it('deletes its decrypted working copy', async () => {
207-
const stagingBefore = (await readdir(tmpdir())).filter((entry) =>
208-
entry.startsWith('sim-chrome-import-')
209-
).length
231+
const before = await stagingDirectories()
210232
const path = await writeCookieDatabase([
211233
{ hostKey: 'example.com', name: 'a', encryptedValue: encryptV10(Buffer.from('v')) },
212234
])
213235

214236
await readBrowserCookies(path, KEY, NOW_SECONDS)
215237

216-
const stagingAfter = (await readdir(tmpdir())).filter((entry) =>
217-
entry.startsWith('sim-chrome-import-')
218-
).length
219-
expect(stagingAfter).toBe(stagingBefore)
238+
expect(await stagingLeftBehindSince(before)).toEqual([])
220239
})
221240

222241
it('cleans up even when the read fails', async () => {
223-
const stagingBefore = (await readdir(tmpdir())).filter((entry) =>
224-
entry.startsWith('sim-chrome-import-')
225-
).length
242+
const before = await stagingDirectories()
226243
await writeFile(join(directory, 'Cookies'), 'not a database')
227244

228245
await expect(readBrowserCookies(join(directory, 'Cookies'), KEY, NOW_SECONDS)).rejects.toThrow()
229246

230-
const stagingAfter = (await readdir(tmpdir())).filter((entry) =>
231-
entry.startsWith('sim-chrome-import-')
232-
).length
233-
expect(stagingAfter).toBe(stagingBefore)
247+
expect(await stagingLeftBehindSince(before)).toEqual([])
234248
})
235249
})

0 commit comments

Comments
 (0)