fix(desktop): reveal every window after a missed ready event, not just the main one
Session, instance, HUD, quick-entry and pet-overlay windows all open with show: false and are revealed only by ready-to-show, so the Electron 40 bug strands them exactly the way it stranded the primary window — and none of them have the second-launch workaround that made the main-window case recoverable. Generalize the controller to any window and wire all six through one wireWindowReveal helper. Callers pass their own reveal action (showInactive for the pet overlay, show + focus for the HUD and quick entry) and their own post-visible work, so whichever path wins runs them exactly once. Quick entry now reveals the window the call created rather than whatever `quickEntryWindow` points at when the event lands.
This commit is contained in:
parent
041cbff5e3
commit
41ae3db426
|
|
@ -654,10 +654,10 @@ export async function waitForAppReady(fixture: MockBackendFixture | NoProviderFi
|
|||
|
||||
// On Electron 40.x, ready-to-show may never fire (electron/electron#51972)
|
||||
// and the window stays hidden even though the DOM is rendered. The main
|
||||
// process has a TEST_WORKER_INDEX-gated fallback that force-shows the
|
||||
// window, but the DOM can be ready before that fires. Poll until the
|
||||
// window is actually visible so interactions (click, screenshot) don't
|
||||
// hit a hidden surface.
|
||||
// process reveals it anyway — immediately under TEST_WORKER_INDEX, and via
|
||||
// wireWindowReveal's post-load fallback in production — but the DOM can be
|
||||
// ready before that lands. Poll until the window is actually visible so
|
||||
// interactions (click, screenshot) don't hit a hidden surface.
|
||||
if (app) {
|
||||
const deadline = Date.now() + timeoutMs
|
||||
|
||||
|
|
|
|||
|
|
@ -136,7 +136,6 @@ import {
|
|||
} from './hardening'
|
||||
import { createLinkTitleWindow, guardLinkTitleSession, readLinkTitleWindowTitle } from './link-title-window'
|
||||
import { ensureMainWindow } from './main-window-lifecycle'
|
||||
import { createMainWindowRevealController } from './main-window-reveal'
|
||||
import {
|
||||
oauthGuardMayHardFail,
|
||||
oauthSessionIsLive,
|
||||
|
|
@ -211,6 +210,7 @@ import { formatBlockerMessage, formatProbeFailedMessage, scanVenvBlockers } from
|
|||
import { fetchMarketplaceThemes, searchMarketplaceThemes } from './vscode-marketplace'
|
||||
import { createWakeIndicatorWindowController } from './wake-indicator-window'
|
||||
import { readWindowBelow } from './window-below'
|
||||
import { createWindowRevealController } from './window-reveal'
|
||||
import {
|
||||
computeWindowOptions,
|
||||
debounce,
|
||||
|
|
@ -8726,6 +8726,31 @@ function wireCommonWindowHandlers(win, { zoom = true }: { zoom?: boolean } = {})
|
|||
})
|
||||
}
|
||||
|
||||
// Every window we open starts with `show: false` so the renderer's first themed
|
||||
// paint lands before it appears, and `ready-to-show` is what reveals it.
|
||||
// Electron 40 can drop that event entirely (electron/electron#51972) on
|
||||
// Linux/Wayland, remote displays and VMs, leaving the window hidden forever even
|
||||
// though the renderer finished loading. Keep the themed path as the preferred
|
||||
// reveal, then fall back a few seconds after the renderer loads. `show` and
|
||||
// `onRevealed` carry the caller's reveal action and post-visible work; whichever
|
||||
// path wins runs them exactly once.
|
||||
function wireWindowReveal(win, { show, onRevealed }: { show?: () => void; onRevealed?: () => void } = {}) {
|
||||
const controller = createWindowRevealController(
|
||||
{
|
||||
isDestroyed: () => win.isDestroyed(),
|
||||
isVisible: () => win.isVisible(),
|
||||
show: show ?? (() => win.show())
|
||||
},
|
||||
{ onRevealed }
|
||||
)
|
||||
|
||||
win.once('ready-to-show', controller.reveal)
|
||||
win.webContents.once('did-finish-load', controller.scheduleFallback)
|
||||
win.on('closed', controller.dispose)
|
||||
|
||||
return controller
|
||||
}
|
||||
|
||||
// Secondary "session windows" — one extra OS window per chat so a user can
|
||||
// work with multiple chats side by side. The registry guarantees one window
|
||||
// per sessionId (re-opening focuses the existing window) and self-cleans on
|
||||
|
|
@ -8779,11 +8804,7 @@ function spawnSecondaryWindow({ sessionId, watch }: { sessionId?: string; watch?
|
|||
win.setWindowButtonPosition?.(WINDOW_BUTTON_POSITION)
|
||||
}
|
||||
|
||||
win.once('ready-to-show', () => {
|
||||
if (!win.isDestroyed()) {
|
||||
win.show()
|
||||
}
|
||||
})
|
||||
wireWindowReveal(win)
|
||||
|
||||
win.on('enter-full-screen', () => sendWindowStateChanged(true))
|
||||
win.on('leave-full-screen', () => sendWindowStateChanged(false))
|
||||
|
|
@ -8861,11 +8882,7 @@ function createInstanceWindow() {
|
|||
win.setWindowButtonPosition?.(WINDOW_BUTTON_POSITION)
|
||||
}
|
||||
|
||||
win.once('ready-to-show', () => {
|
||||
if (!win.isDestroyed()) {
|
||||
win.show()
|
||||
}
|
||||
})
|
||||
wireWindowReveal(win)
|
||||
|
||||
// Per-window fullscreen chrome: send this window its own titlebar inset so its
|
||||
// traffic lights hide/show independently of the primary.
|
||||
|
|
@ -8983,11 +9000,7 @@ function spawnPetOverlayWindow(bounds) {
|
|||
// owns its window-fit + scale, and inheriting zoom would crop the sprite.
|
||||
wireCommonWindowHandlers(win, zoomWiringForWindowKind('petOverlay'))
|
||||
|
||||
win.once('ready-to-show', () => {
|
||||
if (!win.isDestroyed()) {
|
||||
win.showInactive()
|
||||
}
|
||||
})
|
||||
wireWindowReveal(win, { show: () => win.showInactive() })
|
||||
|
||||
win.on('closed', () => {
|
||||
if (petOverlayWindow === win) {
|
||||
|
|
@ -9232,17 +9245,16 @@ function spawnHudWindow(sessionId) {
|
|||
win.on('moved', schedulePersistHudState)
|
||||
win.on('resized', schedulePersistHudState)
|
||||
|
||||
win.once('ready-to-show', () => {
|
||||
if (win.isDestroyed()) {
|
||||
return
|
||||
}
|
||||
|
||||
win.show()
|
||||
win.focus()
|
||||
|
||||
// Step the app aside: the HUD IS the surface now.
|
||||
if (hudRestoreMainWindow && mainWindow && !mainWindow.isDestroyed()) {
|
||||
mainWindow.hide()
|
||||
wireWindowReveal(win, {
|
||||
show: () => {
|
||||
win.show()
|
||||
win.focus()
|
||||
},
|
||||
onRevealed: () => {
|
||||
// Step the app aside: the HUD IS the surface now.
|
||||
if (hudRestoreMainWindow && mainWindow && !mainWindow.isDestroyed()) {
|
||||
mainWindow.hide()
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
|
|
@ -9455,11 +9467,15 @@ function repositionQuickEntryWindow(win) {
|
|||
|
||||
function showQuickEntryWindow() {
|
||||
if (!quickEntryWindow || quickEntryWindow.isDestroyed()) {
|
||||
quickEntryWindow = spawnQuickEntryWindow()
|
||||
quickEntryWindow.once('ready-to-show', () => {
|
||||
if (!quickEntryWindow?.isDestroyed()) {
|
||||
quickEntryWindow.show()
|
||||
quickEntryWindow.focus()
|
||||
// Reveal the window this call created, not whatever `quickEntryWindow`
|
||||
// points at by the time the event lands.
|
||||
const win = spawnQuickEntryWindow()
|
||||
quickEntryWindow = win
|
||||
|
||||
wireWindowReveal(win, {
|
||||
show: () => {
|
||||
win.show()
|
||||
win.focus()
|
||||
}
|
||||
})
|
||||
|
||||
|
|
@ -9582,7 +9598,7 @@ function createWindow() {
|
|||
mainWindow.maximize()
|
||||
}
|
||||
|
||||
const revealController = createMainWindowRevealController(createdMainWindow, {
|
||||
const revealController = wireWindowReveal(createdMainWindow, {
|
||||
onRevealed: () => {
|
||||
// Persist geometry as soon as the window is visible so a crash before the
|
||||
// first clean resize/move/close still captures the restored bounds (#56726).
|
||||
|
|
@ -9609,8 +9625,6 @@ function createWindow() {
|
|||
}
|
||||
})
|
||||
|
||||
mainWindow.once('ready-to-show', revealController.reveal)
|
||||
|
||||
// Under Playwright testing, instantly show the window: `ready-to-show`
|
||||
// doesn't fire in some testing envs, and the suite can't wait out the
|
||||
// production fallback.
|
||||
|
|
@ -9637,7 +9651,6 @@ function createWindow() {
|
|||
|
||||
// the closed wrapper remains truthy, so clear only the window this callback owns.
|
||||
mainWindow.on('closed', () => {
|
||||
revealController.dispose()
|
||||
closePetOverlay()
|
||||
wakeIndicatorController.close()
|
||||
|
||||
|
|
@ -9747,11 +9760,6 @@ function createWindow() {
|
|||
startHermes().catch(error => rememberLog(error.stack || error.message))
|
||||
|
||||
mainWindow.webContents.once('did-finish-load', () => {
|
||||
// Electron 40 can omit ready-to-show even after the renderer has loaded,
|
||||
// leaving the primary window hidden indefinitely. Give the normal themed
|
||||
// paint path four seconds to win, then reveal the same window as a fallback.
|
||||
revealController.scheduleFallback()
|
||||
|
||||
// Zoom restore is handled by wireCommonWindowHandlers (shared with session
|
||||
// windows); no need to reapply it here.
|
||||
broadcastBootProgress()
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import assert from 'node:assert/strict'
|
|||
|
||||
import { test } from 'vitest'
|
||||
|
||||
import { createMainWindowRevealController } from './main-window-reveal'
|
||||
import { createWindowRevealController } from './window-reveal'
|
||||
|
||||
function createHarness({ visible = false }: { visible?: boolean } = {}) {
|
||||
let destroyed = false
|
||||
|
|
@ -13,7 +13,7 @@ function createHarness({ visible = false }: { visible?: boolean } = {}) {
|
|||
let scheduledDelay: number | null = null
|
||||
let clearCalls = 0
|
||||
|
||||
const controller = createMainWindowRevealController(
|
||||
const controller = createWindowRevealController(
|
||||
{
|
||||
isDestroyed: () => destroyed,
|
||||
isVisible: () => isVisible,
|
||||
|
|
@ -128,3 +128,61 @@ test('does not reveal a destroyed window', () => {
|
|||
harness.controller.scheduleFallback()
|
||||
assert.equal(harness.scheduledCallback, null)
|
||||
})
|
||||
|
||||
// The pet overlay reveals with showInactive() and the HUD with show() + focus(),
|
||||
// so the fallback has to run the caller's action rather than a plain show().
|
||||
test('the fallback runs the caller reveal action, not a plain show', () => {
|
||||
const actions: string[] = []
|
||||
let scheduled: (() => void) | null = null
|
||||
|
||||
const controller = createWindowRevealController(
|
||||
{
|
||||
isDestroyed: () => false,
|
||||
isVisible: () => false,
|
||||
show: () => actions.push('showInactive')
|
||||
},
|
||||
{
|
||||
onRevealed: () => actions.push('revealed'),
|
||||
setTimer: callback => {
|
||||
scheduled = callback
|
||||
|
||||
return 1 as unknown as ReturnType<typeof setTimeout>
|
||||
},
|
||||
clearTimer: () => {}
|
||||
}
|
||||
)
|
||||
|
||||
controller.scheduleFallback()
|
||||
scheduled?.()
|
||||
|
||||
assert.deepEqual(actions, ['showInactive', 'revealed'])
|
||||
})
|
||||
|
||||
// The HUD hides the main window from onRevealed — whichever path wins, that
|
||||
// side effect has to happen exactly once.
|
||||
test('onRevealed side effects run once when both paths fire', () => {
|
||||
const harness = createHarness()
|
||||
|
||||
harness.controller.scheduleFallback()
|
||||
harness.controller.reveal()
|
||||
harness.scheduledCallback?.()
|
||||
harness.controller.reveal()
|
||||
|
||||
assert.equal(harness.revealCalls, 1)
|
||||
})
|
||||
|
||||
// Session and instance windows have no post-visible work to do.
|
||||
test('reveals without an onRevealed callback', () => {
|
||||
let shown = false
|
||||
|
||||
const controller = createWindowRevealController({
|
||||
isDestroyed: () => false,
|
||||
isVisible: () => shown,
|
||||
show: () => {
|
||||
shown = true
|
||||
}
|
||||
})
|
||||
|
||||
assert.equal(controller.reveal(), true)
|
||||
assert.equal(shown, true)
|
||||
})
|
||||
|
|
@ -1,4 +1,4 @@
|
|||
type MainWindowRevealTarget = {
|
||||
type WindowRevealTarget = {
|
||||
isDestroyed: () => boolean
|
||||
isVisible: () => boolean
|
||||
show: () => void
|
||||
|
|
@ -6,23 +6,23 @@ type MainWindowRevealTarget = {
|
|||
|
||||
type TimerHandle = ReturnType<typeof setTimeout>
|
||||
|
||||
type MainWindowRevealOptions = {
|
||||
onRevealed: () => void
|
||||
type WindowRevealOptions = {
|
||||
onRevealed?: () => void
|
||||
delayMs?: number
|
||||
setTimer?: (callback: () => void, delayMs: number) => TimerHandle
|
||||
clearTimer?: (timer: TimerHandle) => void
|
||||
}
|
||||
|
||||
export const MAIN_WINDOW_REVEAL_FALLBACK_MS = 4_000
|
||||
export const WINDOW_REVEAL_FALLBACK_MS = 4_000
|
||||
|
||||
export function createMainWindowRevealController(
|
||||
window: MainWindowRevealTarget,
|
||||
export function createWindowRevealController(
|
||||
window: WindowRevealTarget,
|
||||
{
|
||||
onRevealed,
|
||||
delayMs = MAIN_WINDOW_REVEAL_FALLBACK_MS,
|
||||
onRevealed = () => {},
|
||||
delayMs = WINDOW_REVEAL_FALLBACK_MS,
|
||||
setTimer = (callback, delay) => setTimeout(callback, delay),
|
||||
clearTimer = timer => clearTimeout(timer)
|
||||
}: MainWindowRevealOptions
|
||||
}: WindowRevealOptions = {}
|
||||
) {
|
||||
let disposed = false
|
||||
let revealed = false
|
||||
Loading…
Reference in New Issue