From ae375e0e8fbf769d7214f0eec5de814157f9ad88 Mon Sep 17 00:00:00 2001 From: 4gray <4gray@users.noreply.github.com> Date: Sun, 9 Aug 2026 18:45:48 +0200 Subject: [PATCH] fix(settings): protect unsaved edits on window close, quit, and reload (#1394) --- .changes/settings-unsaved-close-protection.md | 10 + CLAUDE.md | 2 +- .../src/electron-test-fixtures.ts | 19 + apps/electron-backend-e2e/src/epg.e2e.ts | 5 + apps/electron-backend-e2e/src/settings.e2e.ts | 63 +++ .../src/app/api/main.preload.ts | 17 + .../app/services/app-update.service.spec.ts | 85 ++++ .../src/app/services/app-update.service.ts | 37 +- .../window-close-guard.service.spec.ts | 443 ++++++++++++++++++ .../services/window-close-guard.service.ts | 249 ++++++++++ apps/electron-backend/src/main.ts | 17 +- ...app-update-notification-panel.component.ts | 14 +- .../app-update-install.service.spec.ts | 200 ++++++++ .../services/app-update-install.service.ts | 95 ++++ .../settings-app-update.facade.spec.ts | 12 +- .../settings/settings-app-update.facade.ts | 13 +- .../settings-unload-guard.service.spec.ts | 381 +++++++++++++++ .../settings/settings-unload-guard.service.ts | 252 ++++++++++ .../src/app/settings/settings.component.ts | 10 + .../test-stubs/settings-test-harness.stub.ts | 4 + .../src/lib/electron-api.interface.ts | 21 + .../shared/interfaces/src/lib/ipc-commands.ts | 7 + 22 files changed, 1942 insertions(+), 14 deletions(-) create mode 100644 .changes/settings-unsaved-close-protection.md create mode 100644 apps/electron-backend/src/app/services/window-close-guard.service.spec.ts create mode 100644 apps/electron-backend/src/app/services/window-close-guard.service.ts create mode 100644 apps/web/src/app/services/app-update-install.service.spec.ts create mode 100644 apps/web/src/app/services/app-update-install.service.ts create mode 100644 apps/web/src/app/settings/settings-unload-guard.service.spec.ts create mode 100644 apps/web/src/app/settings/settings-unload-guard.service.ts diff --git a/.changes/settings-unsaved-close-protection.md b/.changes/settings-unsaved-close-protection.md new file mode 100644 index 000000000..37f62e770 --- /dev/null +++ b/.changes/settings-unsaved-close-protection.md @@ -0,0 +1,10 @@ +--- +type: fix +area: settings +--- + +Unsaved settings edits no longer vanish silently when you close the window, +quit the app, or reload the page. The desktop app now asks whether to save, +discard, or keep editing — the same dialog in-app navigation shows — and only +closes once your choice is applied; a failed save keeps the window open. In the +browser, the native leave-page warning appears instead. diff --git a/CLAUDE.md b/CLAUDE.md index 6b81be8bb..37cfb3de6 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -527,7 +527,7 @@ See `docs/architecture/m3u-playlist-module.md` for complete documentation. `/workspace/xtreams/:id/downloads/:downloadId` and `/workspace/stalker/:id/downloads/:downloadId`. Focused download details hide the workspace context panel. -- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation +- Settings: `/workspace/settings/:section` — one page per section (`general`, `playback`, `epg`, `dashboard`, `remote-control`, `tmdb`, `backup`, `reset`, `about`); `/workspace/settings` redirects to `general`, unknown or capability-gated sections redirect there too, and `/settings` redirects into the workspace. The shared form lives on the parent `SettingsComponent`, so edits survive section switches; a floating unsaved-changes bar (Save/Discard) replaces the old always-visible footer Save button. Leaving the settings AREA with a dirty form triggers `settingsUnsavedChangesGuard` (canDeactivate) and a save/discard/stay dialog — section switches deliberately bypass it, and a failed save cancels the navigation. Non-router exits are covered too: `SettingsUnloadGuardService` (provided by `SettingsComponent`) arms a `beforeunload` handler while the form is dirty (native leave prompt in the PWA) and arms an Electron main-process close guard (`window-close-guard.service.ts`) for the whole settings mount — mount-long on purpose, since arming on the first edit would race the close it protects against. The guard intercepts window close/app quit before `beforeunload` fires and completes the original intent only after the renderer confirms through the same dialog (a pristine form auto-confirms); Electron reloads are cancelled and re-triggered the same way, a failed save always keeps the window open, and installing an app update suspends the whole guard so the updater's quit passes unchallenged — every install entry point (settings About section and the global update notification panel) must go through the root `AppUpdateInstallService`, which owns that suspend/restore choreography **Service Architecture** (Factory Pattern): diff --git a/apps/electron-backend-e2e/src/electron-test-fixtures.ts b/apps/electron-backend-e2e/src/electron-test-fixtures.ts index 2af39ac3e..87e41df35 100644 --- a/apps/electron-backend-e2e/src/electron-test-fixtures.ts +++ b/apps/electron-backend-e2e/src/electron-test-fixtures.ts @@ -581,6 +581,25 @@ export async function closeElectronApp( closePromise.catch(() => undefined), electronAppKillWaitMs ); + + // SIGTERM asks Electron for a graceful quit, which the app can + // legitimately refuse — the unsaved-settings close guard cancels the + // quit while it waits for an answer. A process that survives here + // would outlive the test, hold its data dir, and time out the worker + // teardown, so escalate to SIGKILL. + if ( + childProcess.exitCode === null && + childProcess.signalCode === null + ) { + console.warn( + 'Electron app survived SIGTERM; escalating to SIGKILL' + ); + childProcess.kill('SIGKILL'); + await waitForPromiseWithTimeout( + closePromise.catch(() => undefined), + electronAppKillWaitMs + ); + } } catch (error) { console.warn('Failed to close Electron app cleanly:', error); } diff --git a/apps/electron-backend-e2e/src/epg.e2e.ts b/apps/electron-backend-e2e/src/epg.e2e.ts index 28c6f9a5c..4e5f55835 100644 --- a/apps/electron-backend-e2e/src/epg.e2e.ts +++ b/apps/electron-backend-e2e/src/epg.e2e.ts @@ -110,6 +110,11 @@ test.describe('Electron EPG', () => { await expect(app.mainWindow.locator('.epg-source-row')).toHaveCount( 0 ); + // Adding and removing the source row leaves the settings form + // dirty; save so the unsaved-changes close guard has nothing to + // ask about when the app closes below — an unanswered dialog + // would block the close and leak the Electron process. + await saveSettings(app.mainWindow); await app.mainWindow .getByRole('button', { name: 'Clear EPG data' }) diff --git a/apps/electron-backend-e2e/src/settings.e2e.ts b/apps/electron-backend-e2e/src/settings.e2e.ts index fe28297c1..3e1d623de 100644 --- a/apps/electron-backend-e2e/src/settings.e2e.ts +++ b/apps/electron-backend-e2e/src/settings.e2e.ts @@ -481,6 +481,69 @@ test.describe('Electron Settings', () => { } }); + test('@settings @electron intercepts window close while settings edits are unsaved', async ({ + dataDir, + }) => { + const app = await launchElectronApp(dataDir); + + try { + await openSettings(app.mainWindow); + await selectSettingsOption(app.mainWindow, 'select-language', 'de'); + await expect( + app.mainWindow.getByTestId('settings-unsaved-bar') + ).toBeVisible(); + // Round-trip on the same renderer->main IPC pipe: once this + // resolves, the earlier close-guard arming has been processed. + await app.mainWindow.evaluate(() => + window.electron.getWindowState() + ); + + const requestWindowClose = () => + app.electronApp.evaluate(({ BrowserWindow }) => { + BrowserWindow.getAllWindows()[0]?.close(); + }); + + await requestWindowClose(); + + // The close is intercepted: the window stays open and the same + // save/discard/stay dialog the router guard shows takes over. + await expect( + app.mainWindow.getByTestId('unsaved-dialog-stay') + ).toBeVisible(); + expect(app.electronApp.windows()).toHaveLength(1); + + await app.mainWindow.getByTestId('unsaved-dialog-stay').click(); + await expect( + app.mainWindow.getByTestId('unsaved-dialog-stay') + ).toHaveCount(0); + expect(app.electronApp.windows()).toHaveLength(1); + + // Second attempt, this time saving: the close then completes. + await requestWindowClose(); + await expect( + app.mainWindow.getByTestId('unsaved-dialog-save') + ).toBeVisible(); + await app.mainWindow + .getByTestId('unsaved-dialog-save') + .click({ noWaitAfter: true }); + await expect.poll(() => app.electronApp.windows().length).toBe(0); + } finally { + await closeElectronApp(app); + } + + // The save the dialog promised actually landed before the close. + const relaunch = await launchElectronApp(dataDir); + + try { + await openSettings(relaunch.mainWindow); + await expect( + relaunch.mainWindow.getByTestId('select-language') + ).toContainText('Deutsch'); + } finally { + await closeElectronApp(relaunch); + } + }); + test('@settings @persistence @electron persists the EPG view mode across app restart', async ({ dataDir, }) => { diff --git a/apps/electron-backend/src/app/api/main.preload.ts b/apps/electron-backend/src/app/api/main.preload.ts index 36379161e..0f4298aec 100644 --- a/apps/electron-backend/src/app/api/main.preload.ts +++ b/apps/electron-backend/src/app/api/main.preload.ts @@ -80,6 +80,10 @@ const WINDOW_TOGGLE_MAXIMIZE = 'WINDOW:TOGGLE_MAXIMIZE'; const WINDOW_CLOSE = 'WINDOW:CLOSE'; const WINDOW_GET_STATE = 'WINDOW:GET_STATE'; const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED'; +const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD'; +const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE'; +const WINDOW_CANCEL_CLOSE = 'WINDOW:CANCEL_CLOSE'; +const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED'; const dbSaveContentProgressListeners = new Set< ( @@ -418,6 +422,19 @@ const electronApi: ElectronBridgeApi = { ipcRenderer.on(WINDOW_STATE_CHANGED, handler); return () => ipcRenderer.off(WINDOW_STATE_CHANGED, handler); }, + setWindowCloseGuard: (active: boolean) => + ipcRenderer.invoke(WINDOW_SET_CLOSE_GUARD, active), + confirmWindowClose: () => ipcRenderer.invoke(WINDOW_CONFIRM_CLOSE), + cancelWindowClose: (requestId?: number) => + ipcRenderer.invoke(WINDOW_CANCEL_CLOSE, requestId), + onWindowCloseRequested: (callback: (requestId: number) => void) => { + const handler = ( + _event: Electron.IpcRendererEvent, + requestId: number + ) => callback(requestId); + ipcRenderer.on(WINDOW_CLOSE_REQUESTED, handler); + return () => ipcRenderer.off(WINDOW_CLOSE_REQUESTED, handler); + }, fetchPlaylistByUrl: ( url: string, title?: string, diff --git a/apps/electron-backend/src/app/services/app-update.service.spec.ts b/apps/electron-backend/src/app/services/app-update.service.spec.ts index 042b7bab4..ae266b8b6 100644 --- a/apps/electron-backend/src/app/services/app-update.service.spec.ts +++ b/apps/electron-backend/src/app/services/app-update.service.spec.ts @@ -82,6 +82,8 @@ function createService( isPackaged?: boolean; platform?: NodeJS.Platform; env?: NodeJS.ProcessEnv; + prepareQuit?: () => void; + cancelPreparedQuit?: () => void; } = {} ) { const updater = new FakeUpdater(); @@ -92,7 +94,9 @@ function createService( isPackaged: overrides.isPackaged ?? true, }, getMainWindow: () => win, + cancelPreparedQuit: overrides.cancelPreparedQuit, platform: overrides.platform ?? 'darwin', + prepareQuit: overrides.prepareQuit, processEnv: overrides.env ?? {}, releaseFetcher: overrides.fetcher, updater, @@ -449,4 +453,85 @@ describe('AppUpdateService', () => { ); expect(updater.quitAndInstall).toHaveBeenCalledTimes(1); }); + + it('stands the close guard down before quitAndInstall', () => { + // quitAndInstall closes the windows before 'before-quit' (macOS), so + // an armed unsaved-settings guard would intercept that close and + // strand the requested install. prepareQuit must run first. + const calls: string[] = []; + const { service, updater } = createService({ + prepareQuit: () => calls.push('prepareQuit'), + }); + updater.quitAndInstall.mockImplementation(() => { + calls.push('quitAndInstall'); + }); + service.handleUpdateAvailable({ version: '0.23.0' }); + service.handleUpdateDownloaded({ version: '0.23.0' }); + + service.installUpdate(); + + expect(calls).toEqual(['prepareQuit', 'quitAndInstall']); + }); + + it('does not stand the close guard down when nothing is installable', () => { + const prepareQuit = jest.fn(); + const { service, updater } = createService({ prepareQuit }); + + service.installUpdate(); + + expect(prepareQuit).not.toHaveBeenCalled(); + expect(updater.quitAndInstall).not.toHaveBeenCalled(); + }); + + it('takes the close-guard bypass back when quitAndInstall fails', () => { + // A synchronous updater failure means no quit is coming: the + // prepared one-shot bypass must not leak into the next genuine + // close, and the renderer must see a non-Downloaded status so it + // restores its own unload guard. + const cancelPreparedQuit = jest.fn(); + const prepareQuit = jest.fn(); + const { service, updater } = createService({ + cancelPreparedQuit, + prepareQuit, + }); + updater.quitAndInstall.mockImplementation(() => { + throw new Error('spawn failed'); + }); + service.handleUpdateAvailable({ version: '0.23.0' }); + service.handleUpdateDownloaded({ version: '0.23.0' }); + + const status = service.installUpdate(); + + expect(prepareQuit).toHaveBeenCalledTimes(1); + expect(cancelPreparedQuit).toHaveBeenCalledTimes(1); + expect(status.status).toBe(ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error); + }); + + it('takes the bypass back when the updater emits the failure as an error event', () => { + // electron-updater's BaseUpdater catches synchronous install + // failures internally and emits 'error' instead of throwing, and + // MacUpdater can fail after quitAndInstall already returned — the + // revocation must ride the error path. + const cancelPreparedQuit = jest.fn(); + const { service } = createService({ cancelPreparedQuit }); + service.handleUpdateAvailable({ version: '0.23.0' }); + service.handleUpdateDownloaded({ version: '0.23.0' }); + + service.installUpdate(); + expect(cancelPreparedQuit).not.toHaveBeenCalled(); + + // What attachUpdaterEvents forwards from the updater 'error' event. + service.handleError(new Error('ShipIt failed')); + + expect(cancelPreparedQuit).toHaveBeenCalledTimes(1); + }); + + it('does not revoke a bypass for errors unrelated to an install', () => { + const cancelPreparedQuit = jest.fn(); + const { service } = createService({ cancelPreparedQuit }); + + service.handleError(new Error('check failed')); + + expect(cancelPreparedQuit).not.toHaveBeenCalled(); + }); }); diff --git a/apps/electron-backend/src/app/services/app-update.service.ts b/apps/electron-backend/src/app/services/app-update.service.ts index b3c9a05d3..6fe2d9cd7 100644 --- a/apps/electron-backend/src/app/services/app-update.service.ts +++ b/apps/electron-backend/src/app/services/app-update.service.ts @@ -110,6 +110,19 @@ export interface AppUpdateServiceOptions { platform?: NodeJS.Platform; processEnv?: NodeJS.ProcessEnv; releaseFetcher?: ReleaseFetcher; + /** + * Runs right before `quitAndInstall()`. Lets the unsaved-settings close + * guard stand down for the updater-driven window close, which on macOS + * happens before `before-quit` and would otherwise be intercepted as a + * plain close — abandoning the requested install. + */ + prepareQuit?: () => void; + /** + * Undoes {@link prepareQuit} when `quitAndInstall()` failed synchronously + * and no quit is coming — the prepared one-shot close bypass must not + * leak into the next genuine close. + */ + cancelPreparedQuit?: () => void; } function isSelfUpdateSupported( @@ -240,6 +253,8 @@ export class AppUpdateService { private checkForUpdatesPromise: Promise | null = null; private status: ElectronBridgeAppUpdateStatus; + /** True between prepareQuit() and the quit — or the error that voids it. */ + private quitPreparationPending = false; constructor(private readonly options: AppUpdateServiceOptions) { this.currentVersion = resolveCurrentVersion( @@ -401,7 +416,19 @@ export class AppUpdateService { this.status.status === ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded ) { - this.updater.quitAndInstall(); + // Consumed by handleError: electron-updater's BaseUpdater + // catches its own synchronous install failures and emits + // 'error' instead of throwing, and MacUpdater can return here + // and fail asynchronously — the revocation must ride the error + // path, not a try/catch around this call. + this.quitPreparationPending = true; + this.options.prepareQuit?.(); + + try { + this.updater.quitAndInstall(); + } catch (error) { + this.handleError(error); + } } return this.getStatus(); @@ -448,6 +475,14 @@ export class AppUpdateService { } handleError(error: unknown): void { + // An error after a prepared quit means no quit is coming: take back + // the one-shot close-guard bypass, whether the failure was thrown + // synchronously or emitted later as an updater 'error' event. + if (this.quitPreparationPending) { + this.quitPreparationPending = false; + this.options.cancelPreparedQuit?.(); + } + this.setStatus({ error: normalizeError(error), status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, diff --git a/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts b/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts new file mode 100644 index 000000000..f679a5c20 --- /dev/null +++ b/apps/electron-backend/src/app/services/window-close-guard.service.spec.ts @@ -0,0 +1,443 @@ +/** + * Regression coverage for the unsaved-settings close guard: while the + * renderer arms it, window close and app quit must be intercepted and + * replayed only after the renderer confirms — and the interception must + * never survive a renderer that can no longer answer (navigation, crash, + * destroyed webContents), or the window would become unclosable. + */ + +jest.mock('electron', () => ({ + app: { + on: jest.fn(), + quit: jest.fn(), + }, + ipcMain: { + handle: jest.fn(), + }, +})); + +import { ipcMain } from 'electron'; +import { + WINDOW_CANCEL_CLOSE, + WINDOW_CLOSE_REQUESTED, + WINDOW_CONFIRM_CLOSE, + WINDOW_SET_CLOSE_GUARD, +} from '@iptvnator/shared/interfaces'; +import { + bootstrapWindowCloseGuard, + CloseGuardApp, + CloseGuardWindow, + WindowCloseGuard, +} from './window-close-guard.service'; + +type Listener = (...args: unknown[]) => void; + +interface FakeApp extends CloseGuardApp { + quit: jest.Mock; + fireBeforeQuit(): void; +} + +function createFakeApp(): FakeApp { + const beforeQuitListeners: Listener[] = []; + + return { + on: jest.fn((event: string, listener: Listener) => { + if (event === 'before-quit') { + beforeQuitListeners.push(listener); + } + }), + quit: jest.fn(), + fireBeforeQuit(): void { + beforeQuitListeners.forEach((listener) => listener()); + }, + }; +} + +interface FakeWindow extends CloseGuardWindow { + close: jest.Mock; + isDestroyed: jest.Mock; + webContents: CloseGuardWindow['webContents'] & { + isDestroyed: jest.Mock; + send: jest.Mock; + }; + /** Fires the window 'close' event; returns whether it was prevented. */ + fireClose(): boolean; + fireClosed(): void; + fireWebContentsEvent(event: string): void; +} + +function createFakeWindow(): FakeWindow { + const windowListeners = new Map(); + const webContentsListeners = new Map(); + + const addListener = + (target: Map) => + (event: string, listener: Listener) => { + target.set(event, [...(target.get(event) ?? []), listener]); + }; + + return { + close: jest.fn(), + isDestroyed: jest.fn(() => false), + on: jest.fn(addListener(windowListeners)), + webContents: { + isDestroyed: jest.fn(() => false), + send: jest.fn(), + on: jest.fn(addListener(webContentsListeners)), + }, + fireClose(): boolean { + let prevented = false; + const event = { + preventDefault: () => { + prevented = true; + }, + }; + + (windowListeners.get('close') ?? []).forEach((listener) => + listener(event) + ); + return prevented; + }, + fireClosed(): void { + (windowListeners.get('closed') ?? []).forEach((listener) => + listener() + ); + }, + fireWebContentsEvent(event: string): void { + (webContentsListeners.get(event) ?? []).forEach((listener) => + listener() + ); + }, + }; +} + +function createArmedGuard(): { + app: FakeApp; + guard: WindowCloseGuard; + win: FakeWindow; +} { + const app = createFakeApp(); + const guard = new WindowCloseGuard(app); + const win = createFakeWindow(); + + guard.trackQuitLifecycle(); + guard.attachToWindow(win); + guard.setGuardActive(true); + + return { app, guard, win }; +} + +describe('WindowCloseGuard', () => { + it('lets the window close while the guard is inactive', () => { + const app = createFakeApp(); + const guard = new WindowCloseGuard(app); + const win = createFakeWindow(); + + guard.trackQuitLifecycle(); + guard.attachToWindow(win); + + expect(win.fireClose()).toBe(false); + expect(win.webContents.send).not.toHaveBeenCalled(); + }); + + it('intercepts an armed close and asks the renderer instead', () => { + const { win } = createArmedGuard(); + + expect(win.fireClose()).toBe(true); + expect(win.webContents.send).toHaveBeenCalledWith( + WINDOW_CLOSE_REQUESTED, + 1 + ); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('replays an intercepted window close once the renderer confirms', () => { + const { app, guard, win } = createArmedGuard(); + + win.fireClose(); + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + // The replayed close must pass through even though the guard is + // still armed — the renderer already decided. + expect(win.fireClose()).toBe(false); + }); + + it('resumes a quit as a quit, not as a plain window close', () => { + const { app, guard, win } = createArmedGuard(); + + app.fireBeforeQuit(); + expect(win.fireClose()).toBe(true); + + guard.confirmClose(); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('forgets a cancelled quit before the next close attempt', () => { + const { app, guard, win } = createArmedGuard(); + + // Quit intercepted, user stays: the renderer cancels the request. + app.fireBeforeQuit(); + win.fireClose(); + guard.cancelClose(); + + // A later plain window close must not resurrect the quit. + win.fireClose(); + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('ignores a cancellation citing an older interception', () => { + // Stay's cancel raced a fresh Cmd+Q: the late cancellation must not + // wipe the newer quit intent, or Save on the follow-up dialog would + // downgrade the quit to a plain window close. + const { app, guard, win } = createArmedGuard(); + + win.fireClose(); // request 1 + app.fireBeforeQuit(); + win.fireClose(); // request 2, quit + guard.cancelClose(1); + + guard.confirmClose(); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('honors a cancellation citing the current interception', () => { + const { app, guard, win } = createArmedGuard(); + + app.fireBeforeQuit(); + win.fireClose(); // request 1, quit + guard.cancelClose(1); + + win.fireClose(); // request 2 + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('keeps a pending quit when close is clicked again before the user decides', () => { + const { app, guard, win } = createArmedGuard(); + + // Cmd+Q intercepted; the dialog is open. An impatient second click + // on the window's close button must not downgrade the quit — saving + // would then close only the window and leave the app running. + app.fireBeforeQuit(); + win.fireClose(); + win.fireClose(); + + guard.confirmClose(); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('escalates a pending close to a quit when the user quits mid-decision', () => { + const { app, guard, win } = createArmedGuard(); + + win.fireClose(); + app.fireBeforeQuit(); + win.fireClose(); + + guard.confirmClose(); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('lets exactly one close through after allowNextClose', () => { + const { guard, win } = createArmedGuard(); + + guard.allowNextClose(); + + expect(win.fireClose()).toBe(false); + // The bypass is consumed: the guard is back for the next attempt. + expect(win.fireClose()).toBe(true); + }); + + it('revokes an unused close bypass when the prepared quit never starts', () => { + const { guard, win } = createArmedGuard(); + + guard.allowNextClose(); + guard.revokeAllowedClose(); + + // The stale bypass must not let the next genuine close skip the + // guard. + expect(win.fireClose()).toBe(true); + }); + + it('confirms as a plain close when nothing was intercepted', () => { + const { app, guard, win } = createArmedGuard(); + + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('disarms when the renderer navigates away', () => { + const { win } = createArmedGuard(); + + win.fireWebContentsEvent('did-navigate'); + + expect(win.fireClose()).toBe(false); + }); + + it('completes a close left unanswered when the renderer disarms', () => { + // Teardown race: close intercepted, then the settings page is torn + // down and disarms before answering — the close must not be + // swallowed. + const { guard, win } = createArmedGuard(); + + win.fireClose(); + guard.setGuardActive(false); + + expect(win.close).toHaveBeenCalledTimes(1); + }); + + it('resumes an unanswered quit when the renderer disarms', () => { + const { app, guard, win } = createArmedGuard(); + + app.fireBeforeQuit(); + win.fireClose(); + guard.setGuardActive(false); + + expect(app.quit).toHaveBeenCalledTimes(1); + expect(win.close).not.toHaveBeenCalled(); + }); + + it('closes nothing on a plain disarm without a pending intent', () => { + const { app, guard, win } = createArmedGuard(); + + guard.setGuardActive(false); + + expect(win.close).not.toHaveBeenCalled(); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('drops an unanswered quit intent when the renderer navigates away', () => { + const { app, guard, win } = createArmedGuard(); + + app.fireBeforeQuit(); + win.fireClose(); + win.fireWebContentsEvent('did-navigate'); + + // A fresh interception cycle must not inherit the stale quit. + guard.setGuardActive(true); + win.fireClose(); + guard.confirmClose(); + + expect(win.close).toHaveBeenCalledTimes(1); + expect(app.quit).not.toHaveBeenCalled(); + }); + + it('disarms when the render process is gone', () => { + const { win } = createArmedGuard(); + + win.fireWebContentsEvent('render-process-gone'); + + expect(win.fireClose()).toBe(false); + }); + + it('never blocks a close whose webContents is already destroyed', () => { + const { win } = createArmedGuard(); + + win.webContents.isDestroyed.mockReturnValue(true); + + expect(win.fireClose()).toBe(false); + }); + + it('drops all state once the window is closed', () => { + const { guard, win } = createArmedGuard(); + + win.fireClose(); + win.fireClosed(); + guard.confirmClose(); + + expect(win.close).not.toHaveBeenCalled(); + }); + + it('ignores confirmations for a destroyed window', () => { + const { guard, win } = createArmedGuard(); + + win.fireClose(); + win.isDestroyed.mockReturnValue(true); + guard.confirmClose(); + + expect(win.close).not.toHaveBeenCalled(); + }); + + it('starts a re-created window with a disarmed guard', () => { + const { guard } = createArmedGuard(); + const nextWin = createFakeWindow(); + + guard.attachToWindow(nextWin); + + expect(nextWin.fireClose()).toBe(false); + }); +}); + +describe('bootstrapWindowCloseGuard', () => { + beforeEach(() => { + (ipcMain.handle as jest.Mock).mockClear(); + }); + + function getIpcHandler(channel: string): Listener { + const call = (ipcMain.handle as jest.Mock).mock.calls.find( + ([registered]) => registered === channel + ); + + if (!call) { + throw new Error(`No IPC handler registered for ${channel}`); + } + + return call[1] as Listener; + } + + it('wires the IPC handlers to the guard', () => { + const attachListeners: Array<(win: Electron.BrowserWindow) => void> = + []; + + bootstrapWindowCloseGuard((listener) => + attachListeners.push(listener) + ); + + const win = createFakeWindow(); + attachListeners.forEach((listener) => + listener(win as unknown as Electron.BrowserWindow) + ); + + getIpcHandler(WINDOW_SET_CLOSE_GUARD)({}, true); + expect(win.fireClose()).toBe(true); + + getIpcHandler(WINDOW_CANCEL_CLOSE)({}); + + getIpcHandler(WINDOW_CONFIRM_CLOSE)({}); + expect(win.close).toHaveBeenCalledTimes(1); + }); + + it('treats non-boolean guard payloads as disarm', () => { + const attachListeners: Array<(win: Electron.BrowserWindow) => void> = + []; + + bootstrapWindowCloseGuard((listener) => + attachListeners.push(listener) + ); + + const win = createFakeWindow(); + attachListeners.forEach((listener) => + listener(win as unknown as Electron.BrowserWindow) + ); + + getIpcHandler(WINDOW_SET_CLOSE_GUARD)({}, 'yes'); + + expect(win.fireClose()).toBe(false); + }); +}); diff --git a/apps/electron-backend/src/app/services/window-close-guard.service.ts b/apps/electron-backend/src/app/services/window-close-guard.service.ts new file mode 100644 index 000000000..48fe53732 --- /dev/null +++ b/apps/electron-backend/src/app/services/window-close-guard.service.ts @@ -0,0 +1,249 @@ +/** + * Main-process side of the unsaved-changes close protection. + * + * The settings page arms the guard while its form is dirty + * (`WINDOW:SET_CLOSE_GUARD`). While armed, closing the window — title-bar + * button, custom window controls, Cmd+W, or an app quit — is intercepted + * here and handed back to the renderer as a `WINDOW:CLOSE_REQUESTED` push, + * where the same save/discard/stay dialog the router guard uses decides the + * outcome. `WINDOW:CONFIRM_CLOSE` then replays the original intent (window + * close vs. app quit) with the guard bypassed; staying simply never confirms. + * + * The BrowserWindow `close` event fires before the DOM `beforeunload` + * (Electron contract), so an armed guard is always consulted first and the + * renderer's own `beforeunload` handler only ever sees reloads. + */ + +import { app, ipcMain } from 'electron'; +import { + WINDOW_CANCEL_CLOSE, + WINDOW_CLOSE_REQUESTED, + WINDOW_CONFIRM_CLOSE, + WINDOW_SET_CLOSE_GUARD, +} from '@iptvnator/shared/interfaces'; + +type CloseIntent = 'close' | 'quit'; + +/** The slice of `Electron.App` the guard needs — injectable for tests. */ +export interface CloseGuardApp { + on(event: 'before-quit', listener: () => void): unknown; + quit(): void; +} + +/** The slice of `Electron.BrowserWindow` the guard needs. */ +export interface CloseGuardWindow { + isDestroyed(): boolean; + close(): void; + on(event: string, listener: (...args: unknown[]) => void): unknown; + webContents: { + isDestroyed(): boolean; + send(channel: string, requestId?: number): void; + on(event: string, listener: (...args: unknown[]) => void): unknown; + }; +} + +export class WindowCloseGuard { + private guardActive = false; + private bypassClose = false; + private quitInProgress = false; + private pendingIntent: CloseIntent | null = null; + /** Increments per interception; cancellations must cite the current one. */ + private requestSeq = 0; + private window: CloseGuardWindow | null = null; + + constructor(private readonly electronApp: CloseGuardApp) {} + + /** + * Distinguishes "close this window" from "quit the app": `before-quit` + * fires before the quit closes the windows, so a close intercepted with + * this flag set must resume as a quit — on macOS a plain `win.close()` + * would leave the app running when the user asked it to exit. + */ + trackQuitLifecycle(): void { + this.electronApp.on('before-quit', () => { + this.quitInProgress = true; + }); + } + + /** + * Called for every main window (macOS can rebuild it while the process + * lives on); per-window guard state starts over with each new window. + */ + attachToWindow(win: CloseGuardWindow): void { + this.window = win; + this.guardActive = false; + this.bypassClose = false; + this.pendingIntent = null; + + win.on('close', (event) => + this.handleClose(event as { preventDefault(): void }, win) + ); + win.on('closed', () => { + if (this.window === win) { + this.window = null; + this.guardActive = false; + this.bypassClose = false; + this.pendingIntent = null; + } + }); + // A full navigation (reload included) discards the renderer state the + // guard was protecting; a gone renderer can no longer answer the + // close request. Either way an armed guard would make the window + // unclosable, and a pending intent from an unanswered request would + // leak into the next interception cycle. + win.webContents.on('did-navigate', () => { + this.guardActive = false; + this.pendingIntent = null; + }); + win.webContents.on('render-process-gone', () => { + this.guardActive = false; + this.pendingIntent = null; + }); + } + + setGuardActive(active: boolean): void { + this.guardActive = active; + + // A disarm with an unanswered intercepted close is the teardown + // race: the renderer left settings between the interception and + // this disarm reaching us, so nobody remains to confirm and the + // user's close would be silently swallowed. The guard is mount-long + // in the renderer, so a disarm only ever means "settings is gone" — + // its form was already settled by the router guard, nothing is left + // to protect, and the intercepted intent can complete. + if (!active && this.pendingIntent !== null) { + this.confirmClose(); + } + } + + /** + * Lets exactly one close through even while the guard is armed. Used for + * the self-updater's `quitAndInstall()`: on macOS it closes the windows + * *before* `before-quit` fires, so an intercepted close would downgrade + * the install into a plain window close and abandon the update. The user + * explicitly asked to install, so that close must win. + */ + allowNextClose(): void { + this.bypassClose = true; + } + + /** + * Revokes an unused {@link allowNextClose} when the quit it prepared + * never started (`quitAndInstall()` failed synchronously). Left armed, + * the stale bypass would let the next genuine close skip the guard. + */ + revokeAllowedClose(): void { + this.bypassClose = false; + } + + /** + * Renderer verdict: safe to leave (settings saved or discarded). Replays + * the intercepted intent with the guard bypassed for exactly one close. + */ + confirmClose(): void { + const win = this.window; + + if (!win || win.isDestroyed()) { + return; + } + + const intent = this.pendingIntent ?? 'close'; + this.pendingIntent = null; + this.bypassClose = true; + + if (intent === 'quit') { + this.electronApp.quit(); + } else { + win.close(); + } + } + + /** + * Renderer verdict: the user stays. Clears the pending intent so a later + * close attempt starts fresh — without this, choosing Stay on a quit and + * clicking the window's close button minutes later would quit the app. + * + * A cancellation citing an older request than the current interception + * is stale and ignored: a quit intercepted while the cancel was in + * flight must keep its intent, or Save on the follow-up dialog would + * downgrade the user's quit to a plain window close. + */ + cancelClose(requestId?: number): void { + if (requestId !== undefined && requestId !== this.requestSeq) { + return; + } + + this.pendingIntent = null; + } + + private handleClose( + event: { preventDefault(): void }, + win: CloseGuardWindow + ): void { + // Consumed per close attempt: if this close is allowed through, the + // quit continues on its own; if it is intercepted, the quit is + // aborted and only `pendingIntent` remembers it. + const wasQuit = this.quitInProgress; + this.quitInProgress = false; + + if (this.bypassClose) { + this.bypassClose = false; + return; + } + + if (!this.guardActive) { + return; + } + + if (win.webContents.isDestroyed()) { + return; + } + + event.preventDefault(); + // A quit is never downgraded while a decision is pending: the + // renderer drops repeated requests while its dialog is open, so a + // second close click must not overwrite an intercepted Cmd+Q — + // saving would then close only the window and leave the app running. + if (wasQuit) { + this.pendingIntent = 'quit'; + } else { + this.pendingIntent = this.pendingIntent ?? 'close'; + } + this.requestSeq += 1; + win.webContents.send(WINDOW_CLOSE_REQUESTED, this.requestSeq); + } +} + +/** + * Creates the app-wide guard and registers its IPC handlers. The main-window + * hook is passed in (instead of importing `App`) to keep this module free of + * bootstrap-order side effects. + */ +export function bootstrapWindowCloseGuard( + onMainWindowCreated: ( + listener: (win: Electron.BrowserWindow) => void + ) => void +): WindowCloseGuard { + const guard = new WindowCloseGuard(app); + + guard.trackQuitLifecycle(); + // BrowserWindow satisfies CloseGuardWindow at runtime; the cast only + // bridges Electron's per-event `on` overloads to the structural type. + onMainWindowCreated((win) => + guard.attachToWindow(win as unknown as CloseGuardWindow) + ); + + ipcMain.handle(WINDOW_SET_CLOSE_GUARD, (_event, active: boolean) => { + guard.setGuardActive(active === true); + }); + ipcMain.handle(WINDOW_CONFIRM_CLOSE, () => { + guard.confirmClose(); + }); + ipcMain.handle(WINDOW_CANCEL_CLOSE, (_event, requestId?: number) => { + guard.cancelClose( + typeof requestId === 'number' ? requestId : undefined + ); + }); + + return guard; +} diff --git a/apps/electron-backend/src/main.ts b/apps/electron-backend/src/main.ts index 1dfc50e9e..357a3f1f6 100644 --- a/apps/electron-backend/src/main.ts +++ b/apps/electron-backend/src/main.ts @@ -30,6 +30,7 @@ import { registerStaticHeaderShims } from './app/services/request-header-overrid import { AppUpdateService } from './app/services/app-update.service'; import { databaseWorkerClient } from './app/services/database-worker-client'; import WindowEvents from './app/events/window.events'; +import { bootstrapWindowCloseGuard } from './app/services/window-close-guard.service'; import { registerStreamProbeHandlers } from './app/events/stream-probe'; import XtreamEvents from './app/events/xtream.events'; import { environment } from './environments/environment'; @@ -134,11 +135,19 @@ export default class Main { trace('startup', 'bootstrap-events:start'); } + const windowCloseGuard = bootstrapWindowCloseGuard((listener) => + App.onMainWindowCreated(listener) + ); const appUpdateService = new AppUpdateService({ app, appVersion: environment.version, getMainWindow: () => App.mainWindow, updater: () => autoUpdater, + // quitAndInstall() closes the windows before 'before-quit' fires + // (macOS), so without this an armed close guard would intercept + // the install's window close and strand the update. + prepareQuit: () => windowCloseGuard.allowNextClose(), + cancelPreparedQuit: () => windowCloseGuard.revokeAllowedClose(), }); AppUpdateEvents.bootstrapAppUpdateEvents(appUpdateService); @@ -260,7 +269,13 @@ runEmbeddedMpvRuntimeDiagnosticOrContinue(process.argv, () => { await Main.bootstrapAppEvents(); }); - app.on('before-quit', () => { + // 'will-quit', not 'before-quit': the unsaved-settings close guard can + // cancel a quit mid-flight by preventing the window close, and + // 'before-quit' has already fired by then — tearing sessions and the DB + // worker down there would leave a cancelled quit with the app open but + // playback and database work destroyed. 'will-quit' only fires once + // every window close was allowed through. + app.on('will-quit', () => { shutdownEmbeddedMpv(); shutdownMpvSession(); shutdownVlcSession(); diff --git a/apps/web/src/app/app-update-notification-panel.component.ts b/apps/web/src/app/app-update-notification-panel.component.ts index a597e6d94..6045d01dc 100644 --- a/apps/web/src/app/app-update-notification-panel.component.ts +++ b/apps/web/src/app/app-update-notification-panel.component.ts @@ -14,6 +14,7 @@ import { ELECTRON_BRIDGE_APP_UPDATE_STATUSES, ElectronBridgeAppUpdateStatus, } from '@iptvnator/shared/interfaces'; +import { AppUpdateInstallService } from './services/app-update-install.service'; import { AppUpdateReleaseNotesDialogComponent } from './settings/app-update-release-notes-dialog.component'; @Component({ @@ -176,6 +177,7 @@ import { AppUpdateReleaseNotesDialogComponent } from './settings/app-update-rele }) export class AppUpdateNotificationPanelComponent implements OnInit, OnDestroy { private readonly dialog = inject(MatDialog); + private readonly installService = inject(AppUpdateInstallService); private unsubscribeStatus: (() => void) | null = null; readonly appUpdateStatuses = ELECTRON_BRIDGE_APP_UPDATE_STATUSES; @@ -268,11 +270,15 @@ export class AppUpdateNotificationPanelComponent implements OnInit, OnDestroy { } async installUpdate(): Promise { - if (!window.electron?.installAppUpdate) { - return; - } + // Never window.electron.installAppUpdate() directly: the install + // quits the app, and AppUpdateInstallService stands the settings + // unload guard down first — this panel is global, so the settings + // form can be dirty right now. + const status = await this.installService.installAppUpdate(); - this.status.set(await window.electron.installAppUpdate()); + if (status) { + this.status.set(status); + } } dismiss(): void { diff --git a/apps/web/src/app/services/app-update-install.service.spec.ts b/apps/web/src/app/services/app-update-install.service.spec.ts new file mode 100644 index 000000000..dd2639428 --- /dev/null +++ b/apps/web/src/app/services/app-update-install.service.spec.ts @@ -0,0 +1,200 @@ +import { TestBed } from '@angular/core/testing'; +import { + ELECTRON_BRIDGE_APP_UPDATE_STATUSES, + ElectronBridgeAppUpdateStatus, +} from '@iptvnator/shared/interfaces'; +import { + AppQuitUnloadGuardHooks, + AppUpdateInstallService, +} from './app-update-install.service'; + +const BASE_STATUS: ElectronBridgeAppUpdateStatus = { + currentVersion: '0.23.0', + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Idle, + supportedSelfUpdate: true, +}; + +/** + * The install choreography shared by every entry point (settings About + * section, global update notification panel): suspend the unsaved-settings + * unload guard for the updater-driven quit, and restore it on every path + * that proves the quit is not happening. + */ +describe('AppUpdateInstallService', () => { + const originalElectron = window.electron; + + let pushStatus: (status: ElectronBridgeAppUpdateStatus) => void; + let electronStub: { + installAppUpdate: jest.Mock; + onAppUpdateStatusChange: jest.Mock; + }; + let guard: AppQuitUnloadGuardHooks & { + suspendForAppQuit: jest.Mock; + resumeAfterAbortedAppQuit: jest.Mock; + }; + + beforeEach(() => { + pushStatus = () => undefined; + electronStub = { + installAppUpdate: jest.fn().mockResolvedValue(BASE_STATUS), + onAppUpdateStatusChange: jest.fn( + (callback: (status: ElectronBridgeAppUpdateStatus) => void) => { + pushStatus = callback; + return jest.fn(); + } + ), + }; + window.electron = electronStub as unknown as typeof window.electron; + guard = { + resumeAfterAbortedAppQuit: jest.fn(), + suspendForAppQuit: jest.fn(), + }; + + TestBed.configureTestingModule({}); + }); + + afterEach(() => { + window.electron = originalElectron; + }); + + function createService(): AppUpdateInstallService { + return TestBed.inject(AppUpdateInstallService); + } + + it('returns null and touches no guard without the desktop bridge', async () => { + const service = createService(); + service.registerUnloadGuard(guard); + window.electron = undefined; + + expect(await service.installAppUpdate()).toBeNull(); + expect(guard.suspendForAppQuit).not.toHaveBeenCalled(); + }); + + it('keeps the guard down while a real install quits the app', async () => { + electronStub.installAppUpdate.mockResolvedValue({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + const service = createService(); + service.registerUnloadGuard(guard); + + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + expect(guard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); + }); + + it('restores the guard when nothing was installed', async () => { + const service = createService(); + service.registerUnloadGuard(guard); + + // BASE_STATUS reply stays Idle: nothing installable, no quit. + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('restores the guard when the install IPC rejects', async () => { + const failure = new Error('ipc failed'); + electronStub.installAppUpdate.mockRejectedValue(failure); + const service = createService(); + service.registerUnloadGuard(guard); + + await expect(service.installAppUpdate()).rejects.toThrow(failure); + + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('restores the guard when an error push follows a quitting install', async () => { + // electron-updater reports install failures as a later 'error' + // status push, not as a rejection of the install IPC. + electronStub.installAppUpdate.mockResolvedValue({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + const service = createService(); + service.registerUnloadGuard(guard); + await service.installAppUpdate(); + + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + + // One-shot: later unrelated pushes must not resume again. + pushStatus(BASE_STATUS); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('ignores benign pushes while the install quit is pending', async () => { + // A 'checking' push (update check clicked while the quit winds up) + // proves nothing about the quit; resuming the beforeunload handler + // on it would cancel the updater's window close and strand the + // install. + electronStub.installAppUpdate.mockResolvedValue({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + const service = createService(); + service.registerUnloadGuard(guard); + await service.installAppUpdate(); + + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Checking, + }); + + expect(guard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); + }); + + it('survives an error push that beats the install reply', async () => { + // IPC ordering between the invoke reply and status pushes is not + // guaranteed; a stale 'downloaded' reply must not re-suspend the + // protection the failure already restored. + let resolveInstall: ( + status: ElectronBridgeAppUpdateStatus + ) => void = () => undefined; + electronStub.installAppUpdate.mockImplementation( + () => + new Promise((resolve) => { + resolveInstall = resolve; + }) + ); + const service = createService(); + service.registerUnloadGuard(guard); + + const install = service.installAppUpdate(); + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + + resolveInstall({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded, + }); + await install; + + expect(guard.suspendForAppQuit).toHaveBeenCalledTimes(1); + pushStatus({ + ...BASE_STATUS, + status: ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error, + }); + expect(guard.resumeAfterAbortedAppQuit).toHaveBeenCalledTimes(1); + }); + + it('stops reaching a guard after it unregisters', async () => { + const service = createService(); + const unregister = service.registerUnloadGuard(guard); + + unregister(); + await service.installAppUpdate(); + + expect(guard.suspendForAppQuit).not.toHaveBeenCalled(); + expect(guard.resumeAfterAbortedAppQuit).not.toHaveBeenCalled(); + }); +}); diff --git a/apps/web/src/app/services/app-update-install.service.ts b/apps/web/src/app/services/app-update-install.service.ts new file mode 100644 index 000000000..cdc41ad70 --- /dev/null +++ b/apps/web/src/app/services/app-update-install.service.ts @@ -0,0 +1,95 @@ +import { Injectable } from '@angular/core'; +import { + ELECTRON_BRIDGE_APP_UPDATE_STATUSES, + ElectronBridgeAppUpdateStatus, +} from '@iptvnator/shared/interfaces'; + +/** What an unload guard must offer to survive an updater-driven quit. */ +export interface AppQuitUnloadGuardHooks { + suspendForAppQuit(): void; + resumeAfterAbortedAppQuit(): void; +} + +/** + * The one place that installs a downloaded update without the + * unsaved-settings unload guard fighting the quit. Every install entry + * point — the settings About section and the app-wide update notification + * panel — must go through it: installing quits the app and closes the + * window while the settings form may be dirty, and only a root service can + * reach the settings-scoped unload guard from both. + * + * The choreography: suspend every registered guard, invoke the updater, + * and restore the protection whenever the promised quit provably did not + * happen — a non-'downloaded' reply, a rejected IPC, or a later + * error-status push (electron-updater reports install failures that way + * rather than throwing). `installQuitPending` is armed before the IPC + * round trip because the ordering between the reply and status pushes is + * not guaranteed; the 'downloaded' reply never re-arms it, so a failure + * push that wins the race cannot be undone by the stale reply. + */ +@Injectable({ providedIn: 'root' }) +export class AppUpdateInstallService { + private readonly guards = new Set(); + private installQuitPending = false; + + constructor() { + // App-lifetime subscription on purpose: the failure push can arrive + // after the UI that requested the install is gone. Only an Error + // push aborts — that is how electron-updater reports an install + // failure. Benign pushes (e.g. a 'checking' from an update-check + // clicked while the quit is still winding up) prove nothing about + // the quit and must not resurrect the beforeunload handler mid- + // install. + window.electron?.onAppUpdateStatusChange?.((status) => { + if ( + this.installQuitPending && + status.status === ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Error + ) { + this.abortInstallQuit(); + } + }); + } + + /** + * The settings unload guard registers itself while settings is open. + * @returns the matching unregister + */ + registerUnloadGuard(hooks: AppQuitUnloadGuardHooks): () => void { + this.guards.add(hooks); + return () => this.guards.delete(hooks); + } + + /** + * @returns the updater's reply, or null when the bridge is unavailable + */ + async installAppUpdate(): Promise { + if (!window.electron?.installAppUpdate) { + return null; + } + + this.guards.forEach((guard) => guard.suspendForAppQuit()); + this.installQuitPending = true; + + try { + const status = await window.electron.installAppUpdate(); + + if ( + status?.status !== + ELECTRON_BRIDGE_APP_UPDATE_STATUSES.Downloaded + ) { + this.abortInstallQuit(); + } + + return status; + } catch (error) { + this.abortInstallQuit(); + throw error; + } + } + + /** The promised quit is not happening: restore the unload protection. */ + private abortInstallQuit(): void { + this.installQuitPending = false; + this.guards.forEach((guard) => guard.resumeAfterAbortedAppQuit()); + } +} diff --git a/apps/web/src/app/settings/settings-app-update.facade.spec.ts b/apps/web/src/app/settings/settings-app-update.facade.spec.ts index 79fa59389..3feba5a49 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.spec.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.spec.ts @@ -70,15 +70,17 @@ describe('SettingsAppUpdateFacade', () => { facade.init(); await flush(); - const statusHandler = ( + // The last subscriber is the facade's own — AppUpdateInstallService + // (injected by the facade) also subscribes at construction time. + const statusCalls = ( window.electron.onAppUpdateStatusChange as jest.Mock - ).mock.calls[0][0] as (status: ElectronBridgeAppUpdateStatus) => void; + ).mock.calls; + const statusHandler = statusCalls[statusCalls.length - 1][0] as ( + status: ElectronBridgeAppUpdateStatus + ) => void; statusHandler(pushedStatus); expect(window.electron.getAppUpdateStatus).toHaveBeenCalledTimes(1); - expect(window.electron.onAppUpdateStatusChange).toHaveBeenCalledTimes( - 1 - ); expect(facade.status()).toEqual(pushedStatus); }); diff --git a/apps/web/src/app/settings/settings-app-update.facade.ts b/apps/web/src/app/settings/settings-app-update.facade.ts index 05659f6f7..9c1410759 100644 --- a/apps/web/src/app/settings/settings-app-update.facade.ts +++ b/apps/web/src/app/settings/settings-app-update.facade.ts @@ -7,6 +7,7 @@ import { } from '@iptvnator/shared/interfaces'; import { TranslateService } from '@ngx-translate/core'; import { take } from 'rxjs'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; import { SettingsService } from '../services/settings.service'; import { AppUpdateReleaseNotesDialogComponent } from './app-update-release-notes-dialog.component'; @@ -24,6 +25,7 @@ export class SettingsAppUpdateFacade { private readonly runtime = inject(RuntimeCapabilitiesService); private readonly settingsService = inject(SettingsService); private readonly translate = inject(TranslateService); + private readonly installService = inject(AppUpdateInstallService); /** Latest updater status, polled once and then pushed by the backend */ readonly status = signal(null); @@ -64,11 +66,18 @@ export class SettingsAppUpdateFacade { } async installAppUpdate(): Promise { - if (!this.runtime.isElectron || !window.electron?.installAppUpdate) { + if (!this.runtime.isElectron) { return; } - this.status.set(await window.electron.installAppUpdate()); + // Installing quits the app; AppUpdateInstallService owns the whole + // choreography of standing the unsaved-settings unload guard down + // for that quit and restoring it when the quit provably fails. + const status = await this.installService.installAppUpdate(); + + if (status) { + this.status.set(status); + } } openManualAppUpdate(): void { diff --git a/apps/web/src/app/settings/settings-unload-guard.service.spec.ts b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts new file mode 100644 index 000000000..fe6185755 --- /dev/null +++ b/apps/web/src/app/settings/settings-unload-guard.service.spec.ts @@ -0,0 +1,381 @@ +import { TestBed } from '@angular/core/testing'; +import { FormBuilder } from '@angular/forms'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; +import { SettingsForm } from './settings-form.utils'; +import { + SettingsUnloadGuardHost, + SettingsUnloadGuardService, +} from './settings-unload-guard.service'; + +/** + * Regression coverage for the non-router exits from settings: window close / + * app quit (Electron main-process interception) and page reload + * (`beforeunload`). The router-navigation path is covered by + * `settings-unsaved-changes.guard.spec.ts`. + */ +describe('SettingsUnloadGuardService', () => { + let service: SettingsUnloadGuardService; + let form: SettingsForm; + let host: SettingsUnloadGuardHost & { confirmClose: jest.Mock }; + const originalElectron = window.electron; + + let closeRequestCallback: ((requestId: number) => void) | null; + let unsubscribeCloseRequests: jest.Mock; + let electronStub: { + setWindowCloseGuard: jest.Mock; + confirmWindowClose: jest.Mock; + cancelWindowClose: jest.Mock; + onWindowCloseRequested: jest.Mock; + }; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [SettingsUnloadGuardService], + }); + + service = TestBed.inject(SettingsUnloadGuardService); + // The service only touches `events`, `dirty` and the pristine + // markers, so a minimal group stands in for the real settings form. + form = new FormBuilder().group({ + theme: [''], + }) as unknown as SettingsForm; + host = { + confirmClose: jest.fn().mockResolvedValue(true), + form, + }; + + closeRequestCallback = null; + unsubscribeCloseRequests = jest.fn(); + electronStub = { + setWindowCloseGuard: jest.fn().mockResolvedValue(undefined), + confirmWindowClose: jest.fn().mockResolvedValue(undefined), + cancelWindowClose: jest.fn().mockResolvedValue(undefined), + installAppUpdate: jest.fn().mockResolvedValue({ + currentVersion: '0.23.0', + status: 'idle', + supportedSelfUpdate: true, + }), + onWindowCloseRequested: jest.fn( + (callback: (requestId: number) => void) => { + closeRequestCallback = callback; + return unsubscribeCloseRequests; + } + ), + }; + }); + + afterEach(() => { + service.ngOnDestroy(); + window.electron = originalElectron; + }); + + function activateInElectron(): void { + window.electron = electronStub as unknown as typeof window.electron; + service.activate(host); + } + + function activateInPwa(): void { + window.electron = undefined; + service.activate(host); + } + + function dispatchBeforeUnload(): Event { + const event = new Event('beforeunload', { cancelable: true }); + window.dispatchEvent(event); + return event; + } + + async function flushAsyncWork(): Promise { + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + } + + describe('beforeunload (PWA)', () => { + it('blocks the unload while the form is dirty', () => { + activateInPwa(); + form.markAsDirty(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(true); + }); + + it('lets a pristine form unload silently', () => { + activateInPwa(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(false); + }); + + it('stops blocking after destroy', () => { + activateInPwa(); + form.markAsDirty(); + service.ngOnDestroy(); + + const event = dispatchBeforeUnload(); + + expect(event.defaultPrevented).toBe(false); + }); + }); + + describe('close guard mirroring (Electron)', () => { + it('arms the main-process guard for the whole settings mount', () => { + // Mount-long, not per dirty transition: arming on the first + // edit would race the very close it protects against, since + // the IPC is asynchronous. A pristine close auto-confirms. + activateInElectron(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenCalledWith( + true + ); + }); + + it('disarms the guard on destroy and unsubscribes the push', () => { + activateInElectron(); + + service.ngOnDestroy(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + false + ); + expect(unsubscribeCloseRequests).toHaveBeenCalled(); + }); + }); + + describe('updater-driven app quit (Electron)', () => { + it('suspends both protection layers for the quit', () => { + activateInElectron(); + form.markAsDirty(); + + service.suspendForAppQuit(); + + // The main-process mirror is disarmed... + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + false + ); + // ...and the DOM layer no longer cancels the unload, so the + // updater's window close passes without turning into a reload. + const event = dispatchBeforeUnload(); + expect(event.defaultPrevented).toBe(false); + }); + + it('restores the protection when the quit did not happen', () => { + activateInElectron(); + form.markAsDirty(); + service.suspendForAppQuit(); + + service.resumeAfterAbortedAppQuit(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + true + ); + const event = dispatchBeforeUnload(); + expect(event.defaultPrevented).toBe(true); + }); + + it('stands down for any install entry point via the install service', async () => { + // The global update panel installs through the same root + // service; while settings is mounted, that must reach this + // guard. An Idle reply (nothing installable) suspends and + // restores in one round trip. + const installService = TestBed.inject(AppUpdateInstallService); + activateInElectron(); + form.markAsDirty(); + + await installService.installAppUpdate(); + + expect(electronStub.setWindowCloseGuard).toHaveBeenLastCalledWith( + true + ); + const event = dispatchBeforeUnload(); + expect(event.defaultPrevented).toBe(true); + }); + + it('unregisters from the install service on destroy', async () => { + const installService = TestBed.inject(AppUpdateInstallService); + activateInElectron(); + service.ngOnDestroy(); + const callsAfterDestroy = + electronStub.setWindowCloseGuard.mock.calls.length; + + await installService.installAppUpdate(); + + expect(electronStub.setWindowCloseGuard.mock.calls.length).toBe( + callsAfterDestroy + ); + }); + }); + + describe('intercepted window close (Electron)', () => { + it('confirms the close once the user saves or discards', async () => { + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(1); + await flushAsyncWork(); + + expect(host.confirmClose).toHaveBeenCalledTimes(1); + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + + it('keeps the window open when the user stays or the save fails', async () => { + host.confirmClose.mockResolvedValue(false); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(1); + await flushAsyncWork(); + + expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); + // Staying abandons the request in the main process too, so the + // remembered close-vs-quit intent cannot leak into a later + // attempt. + expect(electronStub.cancelWindowClose).toHaveBeenCalledTimes(1); + }); + + it('escalates an open reload confirmation into the requested close', async () => { + let resolveConfirm: ((value: boolean) => void) | null = null; + host.confirmClose.mockImplementation( + () => + new Promise((resolve) => { + resolveConfirm = resolve; + }) + ); + const reloadPage = jest.fn(); + activateInElectron(); + service.reloadPage = reloadPage; + form.markAsDirty(); + + // Reload confirmation opens first... + dispatchBeforeUnload(); + await flushAsyncWork(); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + // ...then the user closes the window while it is on screen. + closeRequestCallback?.(1); + resolveConfirm?.(true); + await flushAsyncWork(); + + // Save/Discard completes the close — not the stale reload. + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + expect(reloadPage).not.toHaveBeenCalled(); + }); + + it('cancels the escalated close when the user stays', async () => { + let resolveConfirm: ((value: boolean) => void) | null = null; + host.confirmClose.mockImplementation( + () => + new Promise((resolve) => { + resolveConfirm = resolve; + }) + ); + activateInElectron(); + form.markAsDirty(); + + dispatchBeforeUnload(); + await flushAsyncWork(); + closeRequestCallback?.(1); + resolveConfirm?.(false); + await flushAsyncWork(); + + // The main process remembered a close; staying must clear it. + expect(electronStub.cancelWindowClose).toHaveBeenCalledTimes(1); + expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); + }); + + it('re-asks a close that raced the Stay cancellation', async () => { + // Stay's cancelWindowClose is awaited; a close arriving while + // that acknowledgment is in flight must be re-asked afterwards + // — never answered against the stale intent the cancel clears, + // and never silently swallowed. + let resolveCancel: (() => void) | null = null; + electronStub.cancelWindowClose.mockImplementation( + () => + new Promise((resolve) => { + resolveCancel = resolve; + }) + ); + host.confirmClose + .mockResolvedValueOnce(false) + .mockResolvedValueOnce(true); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(1); + await flushAsyncWork(); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + // Second close while the cancel acknowledgment is pending. + closeRequestCallback?.(2); + await flushAsyncWork(); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + resolveCancel?.(); + await flushAsyncWork(); + + // The cancellation cited the request its dialog answered, so a + // newer interception's intent survives it in the main process. + expect(electronStub.cancelWindowClose).toHaveBeenCalledWith(1); + expect(host.confirmClose).toHaveBeenCalledTimes(2); + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + + it('ignores repeated close requests while a dialog is open', async () => { + let resolveConfirm: ((value: boolean) => void) | null = null; + host.confirmClose.mockImplementation( + () => + new Promise((resolve) => { + resolveConfirm = resolve; + }) + ); + activateInElectron(); + form.markAsDirty(); + + closeRequestCallback?.(1); + closeRequestCallback?.(1); + await flushAsyncWork(); + + expect(host.confirmClose).toHaveBeenCalledTimes(1); + + resolveConfirm?.(true); + await flushAsyncWork(); + + expect(electronStub.confirmWindowClose).toHaveBeenCalledTimes(1); + }); + }); + + describe('intercepted reload (Electron)', () => { + it('re-triggers the reload after the user saves or discards', async () => { + const reloadPage = jest.fn(); + activateInElectron(); + service.reloadPage = reloadPage; + form.markAsDirty(); + + const event = dispatchBeforeUnload(); + await flushAsyncWork(); + + expect(event.defaultPrevented).toBe(true); + expect(host.confirmClose).toHaveBeenCalledTimes(1); + expect(reloadPage).toHaveBeenCalledTimes(1); + // A reload decision must never complete a window close. + expect(electronStub.confirmWindowClose).not.toHaveBeenCalled(); + }); + + it('cancels the reload when the user stays', async () => { + const reloadPage = jest.fn(); + host.confirmClose.mockResolvedValue(false); + activateInElectron(); + service.reloadPage = reloadPage; + form.markAsDirty(); + + dispatchBeforeUnload(); + await flushAsyncWork(); + + expect(reloadPage).not.toHaveBeenCalled(); + // A reload was never remembered by the main process, so there + // is nothing to cancel there. + expect(electronStub.cancelWindowClose).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/web/src/app/settings/settings-unload-guard.service.ts b/apps/web/src/app/settings/settings-unload-guard.service.ts new file mode 100644 index 000000000..c487f04a3 --- /dev/null +++ b/apps/web/src/app/settings/settings-unload-guard.service.ts @@ -0,0 +1,252 @@ +import { inject, Injectable, NgZone, OnDestroy } from '@angular/core'; +import { AppUpdateInstallService } from '../services/app-update-install.service'; +import { SettingsForm } from './settings-form.utils'; + +export interface SettingsUnloadGuardHost { + /** The shared settings form whose dirty state arms every protection. */ + form: SettingsForm; + /** + * The same save/discard/stay flow the router guard runs. Resolves true + * when leaving is safe — saved or discarded; a failed save resolves + * false and must cancel the close. + */ + confirmClose: () => Promise; +} + +/** + * Protects unsaved settings edits on the exits the router never sees: + * closing the window, quitting the app, and reloading the page. + * + * Three cooperating layers: + * + * - A `beforeunload` handler that engages while the form is dirty. In a + * browser (PWA) it triggers the native leave-page prompt — custom UI is + * not possible there. In Electron it silently cancels the unload (reloads + * only — see below) and follows up with the app's own save/discard/stay + * dialog. + * - In Electron, a main-process close guard (`setWindowCloseGuard`) armed + * for the WHOLE settings mount, not per dirty transition — arming on the + * first edit would race the very close it protects against, since the + * IPC is asynchronous. It intercepts window close / app quit *before* + * `beforeunload` fires and pushes the decision back here; a pristine + * form auto-confirms without showing anything. + * - `confirmWindowClose` completes an intercepted close once the user + * saved or discarded; staying — or a save that failed — never confirms, + * so the window stays open with the edits intact. + * + * Provided by `SettingsComponent`, so its `ngOnDestroy` runs when the user + * leaves the settings area and every hook is released. + */ +@Injectable() +export class SettingsUnloadGuardService implements OnDestroy { + private readonly zone = inject(NgZone); + private readonly installService = inject(AppUpdateInstallService); + private host: SettingsUnloadGuardHost | null = null; + private unsubscribeCloseRequests: (() => void) | null = null; + private unregisterFromInstallService: (() => void) | null = null; + private confirmationPending = false; + /** + * Intent of the confirmation currently on screen. A close request + * arriving while a reload confirmation is open escalates this to + * 'close' — the dialog is the same, only the continuation differs, and + * the user's most recent ask must be the one that completes. + */ + private activeIntent: 'close' | 'reload' | null = null; + /** True while a Stay's cancelWindowClose acknowledgment is in flight. */ + private cancelling = false; + /** A close requested during that window, re-asked once the cancel lands. */ + private queuedCloseRequest = false; + /** + * Id of the latest intercepted request, cited in cancellations so a + * cancel arriving after a newer interception cannot wipe its intent. + */ + private latestCloseRequestId: number | undefined; + /** Last guard state mirrored to the main process. */ + private guardArmed = false; + /** True while an updater-driven app quit must pass unchallenged. */ + private suspended = false; + + /** Indirection because `window.location.reload` cannot be stubbed. */ + reloadPage: () => void = () => window.location.reload(); + + activate(host: SettingsUnloadGuardHost): void { + this.dispose(); + this.host = host; + + window.addEventListener('beforeunload', this.beforeUnloadHandler); + + this.unsubscribeCloseRequests = + window.electron?.onWindowCloseRequested?.((requestId) => { + this.latestCloseRequestId = requestId; + this.zone.run(() => void this.handleCloseRequest('close')); + }) ?? null; + + // Whatever surface installs an app update — the settings About + // section or the app-wide notification panel — must be able to + // stand this guard down before the updater closes the window. + this.unregisterFromInstallService = + this.installService.registerUnloadGuard({ + resumeAfterAbortedAppQuit: () => + this.resumeAfterAbortedAppQuit(), + suspendForAppQuit: () => this.suspendForAppQuit(), + }); + + // Armed before the user can possibly stage an edit; a close with a + // pristine form auto-confirms through the dialog-less path, so the + // only visible effect of mount-long arming is race-free protection. + this.syncCloseGuard(true); + } + + ngOnDestroy(): void { + this.dispose(); + } + + /** + * Stands every protection layer down for an app quit the user explicitly + * requested from inside settings — installing a downloaded update. The + * updater closes the window while the form may still be dirty; a + * `beforeunload` cancellation at the DOM layer would strand the install + * (and even morph it into a reload), so the quit must pass unchallenged. + */ + suspendForAppQuit(): void { + if (!this.host || this.suspended) { + return; + } + + this.suspended = true; + window.removeEventListener('beforeunload', this.beforeUnloadHandler); + this.syncCloseGuard(true); + } + + /** Restores the protection when the requested quit did not happen. */ + resumeAfterAbortedAppQuit(): void { + if (!this.host || !this.suspended) { + return; + } + + this.suspended = false; + window.addEventListener('beforeunload', this.beforeUnloadHandler); + this.syncCloseGuard(true); + } + + private dispose(): void { + window.removeEventListener('beforeunload', this.beforeUnloadHandler); + this.unsubscribeCloseRequests?.(); + this.unsubscribeCloseRequests = null; + this.unregisterFromInstallService?.(); + this.unregisterFromInstallService = null; + this.suspended = false; + this.queuedCloseRequest = false; + this.syncCloseGuard(false); + this.host = null; + } + + private readonly beforeUnloadHandler = ( + event: BeforeUnloadEvent + ): void => { + if (!this.host?.form.dirty) { + return; + } + + event.preventDefault(); + // Chromium's legacy trigger for the native prompt; harmless in + // Electron, where cancellation comes from preventDefault(). + event.returnValue = ''; + + if (window.electron) { + // Electron cancelled the unload without any prompt. Window close + // never lands here while the guard is armed (the main process + // intercepts it first), and an updater-driven quit suspends this + // handler entirely — so this can only be a reload: ask, then + // re-trigger it. + setTimeout(() => { + this.zone.run(() => void this.handleCloseRequest('reload')); + }); + } + }; + + private async handleCloseRequest( + intent: 'close' | 'reload' + ): Promise { + const host = this.host; + + if (!host) { + return; + } + + if (this.confirmationPending) { + if (intent === 'close') { + if (this.cancelling) { + // The dialog already resolved and Stay's cancellation is + // still in flight. This is a fresh user action — queue + // it and re-ask once the cancel has landed, so the new + // dialog can never answer against the stale intent the + // cancel is about to clear. + this.queuedCloseRequest = true; + } else { + // A close outranks a reload, never the other way + // around: the open dialog stays, but Save/Discard must + // complete the close the user just asked for, not the + // earlier reload. + this.activeIntent = 'close'; + } + } + return; + } + + this.confirmationPending = true; + this.activeIntent = intent; + let reAskClose = false; + + try { + const proceed = await host.confirmClose(); + // Re-read after the dialog: a cross-intent request may have + // escalated it while the user was deciding. + const finalIntent = this.activeIntent ?? intent; + + if (!proceed) { + if (finalIntent === 'close') { + // Staying must clear the intent the main process + // remembered, or a later close attempt would replay a + // stale quit — and the clearing must be AWAITED, or a + // close racing this cancellation could still consume + // the stale intent. + this.cancelling = true; + + try { + await window.electron?.cancelWindowClose?.( + this.latestCloseRequestId + ); + } finally { + this.cancelling = false; + } + + reAskClose = this.queuedCloseRequest; + this.queuedCloseRequest = false; + } + } else if (finalIntent === 'close') { + await window.electron?.confirmWindowClose?.(); + } else { + this.reloadPage(); + } + } finally { + this.confirmationPending = false; + this.activeIntent = null; + } + + if (reAskClose) { + void this.handleCloseRequest('close'); + } + } + + private syncCloseGuard(active: boolean): void { + const effective = active && !this.suspended; + + if (this.guardArmed === effective) { + return; + } + + this.guardArmed = effective; + void window.electron?.setWindowCloseGuard?.(effective); + } +} diff --git a/apps/web/src/app/settings/settings.component.ts b/apps/web/src/app/settings/settings.component.ts index e136cc871..56a07510c 100644 --- a/apps/web/src/app/settings/settings.component.ts +++ b/apps/web/src/app/settings/settings.component.ts @@ -49,6 +49,7 @@ import { SettingsUnsavedChangesDialogComponent, } from './settings-unsaved-changes-dialog.component'; import { SettingsLeaveConfirmation } from './settings-unsaved-changes.guard'; +import { SettingsUnloadGuardService } from './settings-unload-guard.service'; import { SettingsBackupFacade } from './settings-backup.facade'; import { SettingsPlaylistResetFacade } from './settings-playlist-reset.facade'; import { SettingsSnackbarService } from './settings-snackbar.service'; @@ -97,6 +98,7 @@ export const SETTINGS_DEFAULT_SECTION = 'general'; SettingsPlaylistResetFacade, SettingsRemoteControlFacade, SettingsSnackbarService, + SettingsUnloadGuardService, ], }) export class SettingsComponent @@ -112,6 +114,7 @@ export class SettingsComponent private readonly settingsCtx = inject(SettingsContextService); private readonly settingsSnackbar = inject(SettingsSnackbarService); + private readonly unloadGuard = inject(SettingsUnloadGuardService); private readonly runtime = inject(RuntimeCapabilitiesService); private readonly vodSourceDiscovery = inject(VodSourceDiscoveryService); private readonly matDialog = inject(MatDialog); @@ -214,6 +217,13 @@ export class SettingsComponent * storage (indexed db) */ async ngOnInit(): Promise { + // The router guard only covers in-app navigation; this protects the + // same edits against window close, app quit, and page reload. + this.unloadGuard.activate({ + form: this.settingsForm, + confirmClose: () => this.confirmLeaveWithUnsavedChanges(), + }); + // Wait for settings to load before setting the form await this.form.loadSettings(); this.form.hydrateFromStore(); diff --git a/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts b/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts index 2dd8cd1c5..a5b89d662 100644 --- a/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts +++ b/apps/web/src/app/settings/test-stubs/settings-test-harness.stub.ts @@ -241,6 +241,10 @@ export function createElectronStub(): typeof window.electron { .fn() .mockResolvedValue(DEFAULT_APP_UPDATE_STATUS), onAppUpdateStatusChange: jest.fn(() => jest.fn()), + onWindowCloseRequested: jest.fn(() => jest.fn()), + setWindowCloseGuard: jest.fn().mockResolvedValue(undefined), + confirmWindowClose: jest.fn().mockResolvedValue(undefined), + cancelWindowClose: jest.fn().mockResolvedValue(undefined), openInMpv: jest.fn(), openInVlc: jest.fn(), platform: 'linux', diff --git a/libs/shared/interfaces/src/lib/electron-api.interface.ts b/libs/shared/interfaces/src/lib/electron-api.interface.ts index 28cfa0707..33bba2ce6 100644 --- a/libs/shared/interfaces/src/lib/electron-api.interface.ts +++ b/libs/shared/interfaces/src/lib/electron-api.interface.ts @@ -618,6 +618,27 @@ export interface ElectronBridgeApi { onWindowStateChange: ( callback: (state: ElectronBridgeWindowState) => void ) => () => void; + /** + * While active, the main process intercepts window close / app quit and + * pushes a close request to the renderer instead of closing. Used by the + * settings page while its form holds unsaved edits. + */ + setWindowCloseGuard: (active: boolean) => Promise; + /** + * Completes a close the guard intercepted: the main process re-runs the + * original intent (window close or app quit) with the guard bypassed. + */ + confirmWindowClose: () => Promise; + /** + * Abandons a close the guard intercepted (the user stays), so the + * remembered close-vs-quit intent cannot leak into a later attempt. + * Carries the id of the request being abandoned: a cancellation that + * arrives after a NEWER interception must not wipe that newer intent. + */ + cancelWindowClose: (requestId?: number) => Promise; + onWindowCloseRequested: ( + callback: (requestId: number) => void + ) => () => void; fetchPlaylistByUrl: ( url: string, title?: string, diff --git a/libs/shared/interfaces/src/lib/ipc-commands.ts b/libs/shared/interfaces/src/lib/ipc-commands.ts index fc95950dd..b1f812746 100644 --- a/libs/shared/interfaces/src/lib/ipc-commands.ts +++ b/libs/shared/interfaces/src/lib/ipc-commands.ts @@ -113,3 +113,10 @@ export const WINDOW_TOGGLE_MAXIMIZE = 'WINDOW:TOGGLE_MAXIMIZE'; export const WINDOW_CLOSE = 'WINDOW:CLOSE'; export const WINDOW_GET_STATE = 'WINDOW:GET_STATE'; export const WINDOW_STATE_CHANGED = 'WINDOW:STATE_CHANGED'; + +// Close guard: while active, closing/quitting the app is intercepted in the +// main process and handed to the renderer for a save/discard/stay decision +export const WINDOW_SET_CLOSE_GUARD = 'WINDOW:SET_CLOSE_GUARD'; +export const WINDOW_CONFIRM_CLOSE = 'WINDOW:CONFIRM_CLOSE'; +export const WINDOW_CANCEL_CLOSE = 'WINDOW:CANCEL_CLOSE'; +export const WINDOW_CLOSE_REQUESTED = 'WINDOW:CLOSE_REQUESTED';