From c1986770634320652ef6abea264bb79fe7ebb000 Mon Sep 17 00:00:00 2001 From: dietrich ayala Date: Sat, 15 Aug 2026 20:14:42 +0200 Subject: [PATCH] fix(window-backend): a stacking re-assert is not evidence about focus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setStacking asks for relative z order and nothing else. Off darwin the primitives that realize it — moveTop() for a stacked window, setAlwaysOnTop(true, 'floating') for a pinned one — activate the window as a side effect, and that focus was being reported as OS_FOCUS_GAINED. A focus report is a genuine key diff, a key diff emits RaiseWindow, and RaiseWindow is realized as an activating show that reorders again, so the reorder re-ran the whole stacking band replay and the cycle sustained itself at roughly 2ms a lap with no external input. This is the principle raiseWindow already states and honors by echoing WINDOW_SHOWN rather than OS_FOCUS_GAINED: a machine-initiated operation must not feed its own side effect back in as fresh OS evidence. setStacking now arms pendingStackingFocus before its native call, and the window 'focus' listener consumes that arming and declines to emit. A machine-armed focus still reports, because a focus-family command genuinely asked for that one; the arming is dropped on blur, on close, and whenever armSelfCausedFocus supersedes it, so a never-consumed arming cannot swallow a later real focus. Scoped to platforms whose z-raise primitive activates. On macOS moveTop() reorders without taking key, so STACKING_TAKES_FOCUS is false and the suppression is inert there — unverified, since only the active branch has been exercised. Measured on Linux. A six-round activation reproduction went from rounds [true, true, false, false, false, false] to all six true; native focus reports over that run fell from 227 to 15 and emitted effects from 1230 to 142. The seven Hybrid Chrome Overlay side-panel and notes/widgets specs went from 5 passed / 2 failed to 7 passed / 0 failed on two consecutive runs, and the six window-operation patterns from 41-42 passed / 20-21 failed to 53 passed / 9 failed, wall clock 92.5s against roughly eight minutes. check-r2-window-ops holds at 15 entries / 23 calls, check-platform-leak at 0 leaks. --- apps/desktop/main/electron-window-backend.ts | 67 +++++++++++++++++++- 1 file changed, 65 insertions(+), 2 deletions(-) diff --git a/apps/desktop/main/electron-window-backend.ts b/apps/desktop/main/electron-window-backend.ts index 68601c50..b561f4f7 100644 --- a/apps/desktop/main/electron-window-backend.ts +++ b/apps/desktop/main/electron-window-backend.ts @@ -142,6 +142,22 @@ function requestGuestDestroy(guest: TeardownGuest): void { */ const APP_RESIGN_SETTLE_MS = 500; +/** + * Whether this platform's z-raise primitive also moves OS focus. + * + * A stacking call declares NO intention about focus — `setStacking` asks for + * relative z order and nothing else. But off darwin the primitives that realize + * it (`moveTop()`, `setAlwaysOnTop(true, 'floating')`) activate the window as a + * side effect, and that focus is THIS BACKEND'S OWN doing, not evidence about + * the world — so it must not be fed back to the machine as an OS focus event + * (see `raiseWindow`'s docblock for the principle, and the `'focus'` listener + * for the suppression). + * + * On macOS `moveTop()` genuinely reorders without activating or taking key, so + * the suppression is inert there and this stays `false`. + */ +const STACKING_TAKES_FOCUS = process.platform !== 'darwin'; + /** * The structural minimum of a native window this backend drives. Both * `BaseWindow` (page-host) and `BrowserWindow` (browser) satisfy it — a @@ -361,6 +377,30 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi */ private readonly pendingSelfCausedFocus = new Map(); + /** + * Windows whose next native `'focus'` is the side effect of a stacking call + * this backend just made, on a platform where the z-raise primitive takes + * focus (`STACKING_TAKES_FOCUS`). Such a focus is not evidence about the + * world and is DROPPED rather than translated into an event. + * + * A Set, not a Map: a stacking call carries no op token to label the focus + * with, because it never asked for focus at all. + * + * Lifecycle, all four edges deliberate: + * - ARMED by `setStacking`, immediately before the native z-raise, and only + * where that primitive actually takes focus. + * - CONSUMED (one shot) in the window's `'focus'` listener, which suppresses + * the event unless `pendingSelfCausedFocus` also armed it — a focus-family + * command genuinely asked for that one, so it still reports. + * - DROPPED on the window's own `blur` and `closed`, for the same reason + * `pendingSelfCausedFocus` is: focus went elsewhere, so whatever arrives + * next is a new event and not our side effect. + * - DROPPED by `armSelfCausedFocus`: a real focus-family command supersedes + * an outstanding stacking arming, so a never-consumed one cannot swallow + * the focus that command is about to provoke. + */ + private readonly pendingStackingFocus = new Set(); + /** Per-window bounded fullscreen settle timers (§2.3). */ private readonly fullscreenSettleTimers = new Map>(); @@ -656,12 +696,26 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi rendererWC: NativeWebContents | null, ): void { win.on('focus', () => { + // Unconditional: focus landed in-app, so the app did not resign — that is + // true however the focus got here. this.cancelBlurSettle(); + const stackingProvoked = this.pendingStackingFocus.delete(id); + const cause = this.takeFocusCause(id); + const suppressed = stackingProvoked && cause.kind !== 'machine'; + // A focus provoked by `setStacking` is this backend's own side effect on + // platforms whose z-raise primitive activates: the machine asked for z + // order, never for focus. Reporting it would feed a machine-initiated + // operation back in as fresh OS evidence — the principle `raiseWindow` + // states and honors by echoing `WINDOW_SHOWN` instead — and here it + // closes a cycle (stacking → focus → raise → stacking) that sustains + // itself. A machine-armed focus still reports: a focus-family command + // genuinely asked for that one. + if (suppressed) return; this.emit({ type: 'OS_FOCUS_GAINED', id, lastHttpUrl: this.readHttpUrl(id), - cause: this.takeFocusCause(id), + cause, }); // Feeds the kernel (window-kernel-shadow.ts), which drives the real // window operations post-cutover — see that module's header. @@ -674,8 +728,9 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi win.on('blur', () => { // Focus left this window, so any self-caused-focus arming for it is void — // whatever focuses it next is a fresh event, not our echo (see - // `pendingSelfCausedFocus`). + // `pendingSelfCausedFocus`, `pendingStackingFocus`). this.pendingSelfCausedFocus.delete(id); + this.pendingStackingFocus.delete(id); this.armBlurSettle(); }); win.on('show', () => { @@ -700,6 +755,7 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi win.on('closed', () => { this.clearFullscreenSettle(id); this.pendingSelfCausedFocus.delete(id); + this.pendingStackingFocus.delete(id); this.windows.delete(id); // `osWindowId` was captured at registration — reading `win.id` here would // throw on the already-destroyed native window. @@ -792,9 +848,15 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi * call really can take focus: an activating `performOsShow`, `raiseWindow`'s * content focus, `focusContent`. A `showInactive()` takes no focus and arms * nothing — arming it would leave a token nothing consumes. + * + * A command from this family also VOIDS any outstanding stacking arming for + * the window: that arming exists to swallow a focus nobody asked for, and a + * stale one (the z-raise took no focus after all) would otherwise swallow the + * focus this command is about to provoke. */ private armSelfCausedFocus(id: WindowId, opSeq: number): void { this.pendingSelfCausedFocus.set(id, opSeq); + this.pendingStackingFocus.delete(id); } /** @@ -1311,6 +1373,7 @@ export class ElectronWindowBackend implements WindowBackend, BackendMigrationShi if (!win) return; try { if (win.isDestroyed()) return; + if (STACKING_TAKES_FOCUS) this.pendingStackingFocus.add(id); if (level === 'pinned') win.setAlwaysOnTop(true, 'floating'); else win.moveTop(); } catch { -- 2.51.2