Per-tab zoom - #485
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 92d08174a4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!tab) return { ok: false }; | ||
| const zoom = logic.nextZoomFactor(tab.zoom ?? 1, direction); | ||
| patch(id, { zoom }); | ||
| views.get(id)?.webContents.setZoomFactor(zoom); |
There was a problem hiding this comment.
Keep Electron zoom state from leaking across tabs
When two embedded tabs are on the same origin, calling Electron's setZoomFactor here does not stay scoped to the one tab whose model was patched; Electron documents zoom as same-origin policy (webContents docs), so zooming https://example.com/a also changes other example.com tab views while their zoom field remains 1 and no badge/persisted state is updated. This makes the new per-tab zoom model diverge from the actual rendered zoom for same-origin tabs.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed — the manager re-asserts the tab's model zoom on activation and full navigation, so same-origin tabs can't leak Electron's per-origin zoom into each other's views (3b89a32).
Chrome's zoom ladder owned by main so renderer and main can't drift: ⌘= / ⌘+ / ⌘- steps through it (ties snap to the lower step), ⌘0 resets to 100%. The factor lives on the tab model and persists in tabs.json, applies to live views on set and to new views on creation, and a small pill inside the omnibox shows the current zoom (click resets). Pure nextZoomFactor is unit-tested; manager test covers set + apply.
Codex on #485: Electron's setZoomFactor is per-origin for the session, so two tabs on one origin share the rendered zoom while the model says per-tab. The manager now re-asserts the tab's model zoom whenever the view could be showing another origin's value — on tab activation and on full navigation — so rendered zoom tracks the active tab again. Test covers the same-origin-neighbour case; FakeWebContents gains the setZoomFactor spy the activation path now always calls.
bda3509 to
ec16bd7
Compare
What
Per-tab zoom, Chrome-style:
browser-logic.ts) so renderer and main can't drift, and there are tests covering set / apply / persist.Stacked on
pr/downloads-shelf.For QA