Skip to content

Commit d8a6dfd

Browse files
fix(desktop): app zoom no longer zooms the preview browser (#6649)
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent e9ae134 commit d8a6dfd

4 files changed

Lines changed: 219 additions & 38 deletions

File tree

apps/desktop/src/preview/Manager.test.ts

Lines changed: 119 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -979,7 +979,10 @@ describe("PreviewManager", () => {
979979
),
980980
);
981981

982-
effectIt.effect("mirrors Electron's effective zoom across registration and navigation", () =>
982+
// The guest reports whatever zoom level Chromium handed it from the app
983+
// window, so the tab's own zoom is the source of truth in both directions:
984+
// asserted onto every guest, never read back off one.
985+
effectIt.effect("keeps the tab's own zoom instead of the guest's reported zoom", () =>
983986
withManager((manager) =>
984987
Effect.gen(function* () {
985988
let effectiveZoom = 0.9;
@@ -1025,18 +1028,13 @@ describe("PreviewManager", () => {
10251028
yield* manager.createTab("tab_zoom");
10261029
yield* manager.registerWebview("tab_zoom", 42);
10271030

1028-
expect(states.at(-1)?.zoomFactor).toBe(0.9);
1029-
expect(setZoomFactor).not.toHaveBeenCalled();
1031+
expect(states.at(-1)?.zoomFactor).toBe(1);
1032+
expect(setZoomFactor).toHaveBeenCalledWith(1);
10301033

1031-
effectiveZoom = 1.25;
1032-
listeners.get("did-navigate")?.();
1033-
yield* Effect.yieldNow;
1034-
1035-
expect(states.at(-1)?.zoomFactor).toBe(1.25);
1036-
expect(setZoomFactor).not.toHaveBeenCalled();
1037-
1038-
zoomReadable = false;
1039-
url = "https://example.com/after-zoom-read-failed";
1034+
// An app zoom leaves the guest reporting the inherited level. Navigating
1035+
// must not adopt it as the preview's zoom.
1036+
effectiveZoom = 0.8;
1037+
url = "https://example.com/after-app-zoom";
10401038
listeners.get("did-navigate")?.();
10411039
yield* Effect.yieldNow;
10421040

@@ -1045,7 +1043,18 @@ describe("PreviewManager", () => {
10451043
url,
10461044
title: "Example",
10471045
});
1048-
expect(states.at(-1)?.zoomFactor).toBe(1.25);
1046+
expect(states.at(-1)?.zoomFactor).toBe(1);
1047+
1048+
// Only the preview's own zoom controls move it.
1049+
yield* manager.zoomIn("tab_zoom");
1050+
expect(setZoomFactor).toHaveBeenCalledWith(1.1);
1051+
expect(states.at(-1)?.zoomFactor).toBe(1.1);
1052+
1053+
zoomReadable = false;
1054+
listeners.get("did-navigate")?.();
1055+
yield* Effect.yieldNow;
1056+
1057+
expect(states.at(-1)?.zoomFactor).toBe(1.1);
10491058

10501059
const replacementSetZoomFactor = vi.fn();
10511060
fromId.mockReturnValue({
@@ -1074,8 +1083,103 @@ describe("PreviewManager", () => {
10741083

10751084
yield* manager.registerWebview("tab_zoom", 43);
10761085

1077-
expect(replacementSetZoomFactor).toHaveBeenCalledWith(1.25);
1078-
expect(states.at(-1)?.zoomFactor).toBe(1.25);
1086+
expect(replacementSetZoomFactor).toHaveBeenCalledWith(1.1);
1087+
expect(states.at(-1)?.zoomFactor).toBe(1.1);
1088+
}),
1089+
),
1090+
);
1091+
1092+
// Zooming the app UI pushes the window's zoom level onto every guest, so the
1093+
// preview has to be put back at the zoom the user gave it.
1094+
effectIt.effect("re-applies each tab's own zoom when the app window zooms", () =>
1095+
withManager((manager) =>
1096+
Effect.gen(function* () {
1097+
const setZoomFactor = vi.fn();
1098+
fromId.mockReturnValue({
1099+
id: 42,
1100+
isDestroyed: () => false,
1101+
getType: () => "webview",
1102+
getURL: () => "https://example.com",
1103+
getTitle: () => "Example",
1104+
isLoading: () => false,
1105+
getZoomFactor: () => 1,
1106+
setZoomFactor,
1107+
on: vi.fn(),
1108+
off: vi.fn(),
1109+
ipc: { on: vi.fn(), off: vi.fn() },
1110+
send: webviewSend,
1111+
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1112+
setWindowOpenHandler: vi.fn(),
1113+
debugger: {
1114+
isAttached: () => false,
1115+
attach: vi.fn(),
1116+
sendCommand: vi.fn(async () => undefined),
1117+
on: vi.fn(),
1118+
off: vi.fn(),
1119+
},
1120+
} as never);
1121+
1122+
yield* manager.createTab("tab_reapply");
1123+
yield* manager.registerWebview("tab_reapply", 42);
1124+
yield* manager.zoomIn("tab_reapply");
1125+
setZoomFactor.mockClear();
1126+
1127+
yield* manager.reapplyZoom();
1128+
1129+
expect(setZoomFactor).toHaveBeenCalledTimes(1);
1130+
expect(setZoomFactor).toHaveBeenCalledWith(1.1);
1131+
}),
1132+
),
1133+
);
1134+
1135+
// did-attach and dom-ready both re-register the guest that is already
1136+
// attached, and a guest that just inherited the app window's zoom needs its
1137+
// own back — without that round trip republishing tab state.
1138+
effectIt.effect("re-asserts the tab's zoom when the active guest registers again", () =>
1139+
withManager((manager) =>
1140+
Effect.gen(function* () {
1141+
const setZoomFactor = vi.fn();
1142+
fromId.mockReturnValue({
1143+
id: 42,
1144+
isDestroyed: () => false,
1145+
getType: () => "webview",
1146+
getURL: () => "https://example.com",
1147+
getTitle: () => "Example",
1148+
isLoading: () => false,
1149+
getZoomFactor: () => 1,
1150+
setZoomFactor,
1151+
on: vi.fn(),
1152+
off: vi.fn(),
1153+
ipc: { on: vi.fn(), off: vi.fn() },
1154+
send: webviewSend,
1155+
navigationHistory: { canGoBack: () => false, canGoForward: () => false },
1156+
setWindowOpenHandler: vi.fn(),
1157+
debugger: {
1158+
isAttached: () => false,
1159+
attach: vi.fn(),
1160+
sendCommand: vi.fn(async () => undefined),
1161+
on: vi.fn(),
1162+
off: vi.fn(),
1163+
},
1164+
} as never);
1165+
const states: PreviewManager.PreviewTabState[] = [];
1166+
yield* manager.subscribeStateChanges((_tabId, state) =>
1167+
Effect.sync(() => {
1168+
states.push(state);
1169+
}),
1170+
);
1171+
1172+
yield* manager.createTab("tab_reregister_zoom");
1173+
yield* manager.registerWebview("tab_reregister_zoom", 42);
1174+
yield* manager.zoomIn("tab_reregister_zoom");
1175+
setZoomFactor.mockClear();
1176+
const publishedBefore = states.length;
1177+
1178+
yield* manager.registerWebview("tab_reregister_zoom", 42);
1179+
1180+
expect(setZoomFactor).toHaveBeenCalledWith(1.1);
1181+
expect(states.length).toBe(publishedBefore);
1182+
expect(states.at(-1)?.zoomFactor).toBe(1.1);
10791183
}),
10801184
),
10811185
);

apps/desktop/src/preview/Manager.ts

Lines changed: 49 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,22 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
647647
if (Option.isSome(next)) yield* emit(tabId, next.value);
648648
});
649649

650+
/**
651+
* Pushes a tab's zoom factor onto whichever guest it currently owns, reading
652+
* both at call time. Anything that applies zoom after an await goes through
653+
* here: a snapshot taken before the await can be older than a zoom action that
654+
* landed in between, and re-applying it would roll that action back.
655+
*/
656+
const assertTabZoom = Effect.fn("PreviewManager.assertTabZoom")(function* (tabId: string) {
657+
const tab = (yield* SynchronizedRef.get(tabsRef)).get(tabId);
658+
if (!tab || tab.webContentsId == null) return;
659+
const wc = webContents.fromId(tab.webContentsId);
660+
if (!wc || wc.isDestroyed()) return;
661+
yield* attempt({ operation: "assertTabZoom", tabId, webContentsId: wc.id }, () =>
662+
wc.setZoomFactor(tab.zoomFactor),
663+
).pipe(Effect.ignore);
664+
});
665+
650666
const requireWebContents = Effect.fn("PreviewManager.requireWebContents")(function* (
651667
tabId: string,
652668
) {
@@ -1305,10 +1321,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
13051321
confirmedNavigation = false,
13061322
) {
13071323
if (wc.isDestroyed()) return;
1308-
const zoomFactor = yield* attempt(
1309-
{ operation: "syncWebContentsState.getZoomFactor", tabId, webContentsId: wc.id },
1310-
() => wc.getZoomFactor(),
1311-
).pipe(Effect.option);
13121324
const computedNavStatus = computeNavStatus(wc);
13131325
const canGoBack = wc.navigationHistory.canGoBack();
13141326
const canGoForward = wc.navigationHistory.canGoForward();
@@ -1338,7 +1350,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
13381350
navStatus,
13391351
canGoBack,
13401352
canGoForward,
1341-
...(Option.isSome(zoomFactor) ? { zoomFactor: zoomFactor.value } : {}),
1353+
// zoomFactor is deliberately not read back from the guest: Chromium
1354+
// reports the level it inherited from the app window, so mirroring it
1355+
// would turn an app zoom into the preview's own zoom.
13421356
updatedAt,
13431357
};
13441358
return [
@@ -1716,11 +1730,10 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
17161730
const annotationTheme = yield* Ref.get(annotationThemeRef);
17171731
const currentAttachment = attached.get(webContentsId);
17181732
if (tab.webContentsId === webContentsId && currentAttachment?.webContents === wc) {
1719-
const zoomFactor = yield* attempt(
1720-
{ operation: "registerWebview.getZoomFactor", tabId, webContentsId },
1721-
() => wc.getZoomFactor(),
1722-
);
1723-
yield* update(tabId, { zoomFactor });
1733+
// The guest we already own re-announced itself, so nothing about the tab
1734+
// changed. Only push its zoom back down — Chromium may have just handed
1735+
// this guest the app window's zoom level.
1736+
yield* assertTabZoom(tabId);
17241737
yield* attempt({ operation: "registerWebview.sendTheme", tabId, webContentsId }, () =>
17251738
wc.send(ANNOTATION_THEME_CHANNEL, annotationTheme),
17261739
);
@@ -1749,18 +1762,13 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
17491762
) {
17501763
return yield* new PreviewTabNotFoundError({ tabId });
17511764
}
1752-
const zoomFactor =
1753-
replacedWebContentsId !== null
1754-
? yield* attempt(
1755-
{ operation: "registerWebview.restoreZoomFactor", tabId, webContentsId },
1756-
() => {
1757-
wc.setZoomFactor(currentTab.zoomFactor);
1758-
return currentTab.zoomFactor;
1759-
},
1760-
)
1761-
: yield* attempt({ operation: "registerWebview.getZoomFactor", tabId, webContentsId }, () =>
1762-
wc.getZoomFactor(),
1763-
);
1765+
// Always assert the tab's own zoom rather than reading the guest's: a guest
1766+
// attaching while the app UI is zoomed starts at the embedder's inherited
1767+
// zoom level, which is not the preview's zoom. Done before the guest is
1768+
// published so it never paints a frame at the inherited zoom.
1769+
yield* attempt({ operation: "registerWebview.restoreZoomFactor", tabId, webContentsId }, () =>
1770+
wc.setZoomFactor(currentTab.zoomFactor),
1771+
);
17641772
yield* attachListeners(tabId, wc);
17651773
const registeredAt = yield* currentIso;
17661774
const registration = yield* SynchronizedRef.modifyEffect(tabsRef, (tabs) =>
@@ -1784,7 +1792,6 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
17841792
navStatus: pendingUrl === null ? computeNavStatus(wc) : current.navStatus,
17851793
canGoBack: wc.navigationHistory.canGoBack(),
17861794
canGoForward: wc.navigationHistory.canGoForward(),
1787-
zoomFactor,
17881795
updatedAt: registeredAt,
17891796
};
17901797
return [
@@ -1806,6 +1813,9 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
18061813
return yield* new PreviewTabNotFoundError({ tabId });
18071814
}
18081815
const { state: registered, pendingUrl } = registration.value;
1816+
// A zoom action that landed while this attach was in flight addressed the
1817+
// guest this one replaced, so settle the new guest on the committed factor.
1818+
yield* assertTabZoom(tabId);
18091819
runFork(restoreControlSession(tabId, wc));
18101820
yield* emit(tabId, registered);
18111821
yield* attempt({ operation: "registerWebview.sendTheme", tabId, webContentsId }, () =>
@@ -2099,6 +2109,17 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
20992109
);
21002110
});
21012111

2112+
/**
2113+
* Chromium hands every guest `<webview>` the embedder's zoom level, so zooming
2114+
* the app UI drags the previewed page along with it. The preview browser owns
2115+
* its own zoom factor, so re-assert it on each attached guest whenever the main
2116+
* window's zoom changes (see DesktopWindow.zoomMain).
2117+
*/
2118+
const reapplyZoom = Effect.fn("PreviewManager.reapplyZoom")(function* () {
2119+
const tabIds = Array.from((yield* SynchronizedRef.get(tabsRef)).keys());
2120+
yield* Effect.forEach(tabIds, assertTabZoom, { discard: true });
2121+
});
2122+
21022123
const applyZoom = Effect.fn("PreviewManager.applyZoom")(function* (
21032124
tabId: string,
21042125
transform: (current: number) => number,
@@ -3476,6 +3497,7 @@ const makeNativeOperations = Effect.fn("PreviewManager.makeOperations")(function
34763497
openPictureInPicture,
34773498
openDevTools,
34783499
pickElement,
3500+
reapplyZoom,
34793501
refresh,
34803502
registerWebview,
34813503
resetZoom: (tabId: string) => applyZoom(tabId, () => DEFAULT_ZOOM_FACTOR),
@@ -3774,6 +3796,9 @@ export class PreviewManager extends Context.Service<
37743796
readonly zoomIn: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
37753797
readonly zoomOut: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
37763798
readonly resetZoom: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
3799+
// Re-applies every attached guest's own zoom factor, undoing the zoom level
3800+
// Chromium inherits from the embedder when the app UI zooms.
3801+
readonly reapplyZoom: () => Effect.Effect<void>;
37773802
readonly hardReload: (tabId: string) => Effect.Effect<void, PreviewManagerError>;
37783803
readonly setColorScheme: (
37793804
tabId: string,
@@ -3874,6 +3899,7 @@ export const make = Effect.gen(function* PreviewManagerMake() {
38743899
zoomIn: operations.zoomIn,
38753900
zoomOut: operations.zoomOut,
38763901
resetZoom: operations.resetZoom,
3902+
reapplyZoom: operations.reapplyZoom,
38773903
hardReload: operations.hardReload,
38783904
setColorScheme: operations.setColorScheme,
38793905
openDevTools: operations.openDevTools,

apps/desktop/src/window/DesktopWindow.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,14 @@ const environmentInput = {
6161
function makeFakeBrowserWindow() {
6262
const windowListeners = new Map<string, (...args: readonly unknown[]) => void>();
6363
const webContentsListeners = new Map<string, (...args: readonly unknown[]) => void>();
64+
let zoomLevel = 0;
6465
const webContents = {
6566
copyImageAt: vi.fn(),
6667
getURL: vi.fn(() => "t3code-dev://app/"),
68+
getZoomLevel: vi.fn(() => zoomLevel),
69+
setZoomLevel: vi.fn((level: number) => {
70+
zoomLevel = level;
71+
}),
6772
isLoadingMainFrame: vi.fn(() => false),
6873
on: vi.fn((eventName: string, listener: (...args: readonly unknown[]) => void) => {
6974
webContentsListeners.set(eventName, listener);
@@ -116,6 +121,7 @@ function makeFakeBrowserWindow() {
116121
openDevTools: webContents.openDevTools,
117122
reload: webContents.reload,
118123
send: webContents.send,
124+
setZoomLevel: webContents.setZoomLevel,
119125
setAutoHideCursor: window.setAutoHideCursor,
120126
webContentsListeners,
121127
windowListeners,
@@ -186,6 +192,7 @@ function makeTestLayer(input: {
186192
bounds: DesktopAppSettings.DesktopWindowBounds,
187193
) => Effect.Effect<void>;
188194
readonly openedExternalUrls?: unknown[];
195+
readonly previewZoomReapplies?: number[];
189196
}) {
190197
let desktopSettings = input.desktopSettings ?? DesktopAppSettings.DEFAULT_DESKTOP_SETTINGS;
191198
const desktopAppSettingsLayer = Layer.succeed(DesktopAppSettings.DesktopAppSettings, {
@@ -264,6 +271,10 @@ function makeTestLayer(input: {
264271
setMainWindow: () => Effect.void,
265272
isBrowserPartition: (partition) => partition.startsWith("persist:t3code-preview-"),
266273
getBrowserPartition: () => Effect.succeed("persist:t3code-preview-test"),
274+
reapplyZoom: () =>
275+
Effect.sync(() => {
276+
input.previewZoomReapplies?.push(input.window.webContents.getZoomLevel());
277+
}),
267278
}),
268279
),
269280
),
@@ -483,6 +494,42 @@ describe("DesktopWindow", () => {
483494
}),
484495
);
485496

497+
// Chromium hands the main window's zoom level down to embedded preview
498+
// guests, so every app zoom has to put the preview browser back at its own
499+
// zoom or zooming the UI drags the previewed page with it.
500+
it.effect("restores the preview browser's own zoom after zooming the app", () =>
501+
Effect.gen(function* () {
502+
const fakeWindow = makeFakeBrowserWindow();
503+
const createCount = yield* Ref.make(0);
504+
const mainWindow = yield* Ref.make<Option.Option<Electron.BrowserWindow>>(Option.none());
505+
const previewZoomReapplies: number[] = [];
506+
const layer = makeTestLayer({
507+
window: fakeWindow.window,
508+
createCount,
509+
mainWindow,
510+
previewZoomReapplies,
511+
});
512+
513+
yield* Effect.gen(function* () {
514+
const desktopWindow = yield* DesktopWindow.DesktopWindow;
515+
yield* desktopWindow.handleBackendReady(new URL("http://127.0.0.1:3773"));
516+
517+
yield* desktopWindow.zoomMain("out");
518+
yield* desktopWindow.zoomMain("out");
519+
yield* desktopWindow.zoomMain("in");
520+
yield* desktopWindow.zoomMain("reset");
521+
522+
assert.deepEqual(
523+
fakeWindow.setZoomLevel.mock.calls.map(([level]) => level),
524+
[-0.5, -1, -0.5, 0],
525+
);
526+
// Recorded after the window level moved, so the preview is put back at
527+
// its own zoom on every step rather than left on the inherited one.
528+
assert.deepEqual(previewZoomReapplies, [-0.5, -1, -0.5, 0]);
529+
}).pipe(Effect.provide(layer));
530+
}),
531+
);
532+
486533
it.effect("uses the persisted main window bounds when opening the window", () =>
487534
Effect.gen(function* () {
488535
const fakeWindow = makeFakeBrowserWindow();

0 commit comments

Comments
 (0)