diff --git a/.changes/shell-reload-in-app-route.md b/.changes/shell-reload-in-app-route.md new file mode 100644 index 000000000..475da5b69 --- /dev/null +++ b/.changes/shell-reload-in-app-route.md @@ -0,0 +1,9 @@ +--- +type: fix +area: shell +--- + +Reloading the desktop app while on any page no longer breaks it: View › Reload +on macOS used to leave a blank, dead window until restart, and the reload +offered by the settings unsaved-changes dialog did nothing. Both now reload the +app back onto the page you were on. diff --git a/CLAUDE.md b/CLAUDE.md index 2f29857b9..dd9b76ee1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -770,6 +770,7 @@ This project uses modern Angular signal-based APIs and patterns. **ALWAYS** use - Registers event handlers for IPC communication - Creates the main window per the startup window mode (`app/app.ts` `initMainWindow`, resolver in `app/services/startup-window-mode.ts`): the electron-conf `STARTUP_WINDOW_MODE` mirror or the one-shot `--fullscreen` switch (consumed by the first window, so a window the macOS Dock re-creates in the same process follows the stored setting); `fullscreen: true` is a constructor option that Windows/Linux honour before the first paint, while macOS ignores it on a hidden window, so `ready-to-show` repeats the request after `show()` only when `isFullScreen()` is still false, through the same tracker the F11 toggle uses (`app/services/native-fullscreen-transitions.ts`: per-window fullscreen state seeded once at creation via `trackNativeFullScreen` and fed only by the enter/leave events afterwards, plus a pending record holding the latest target, cleared when an event lands on it, kept when an event lands on the other state, and ignored after 2 s; the tracker only observes and never issues a request itself, since a "repeat on mismatch" cannot be told apart from reversing the user's own green-button action — a toggle is never decided against `isFullScreen()`, which is stale mid-transition and, on Windows, even during the event), so F11 during the startup animation exits instead of re-requesting; `maximize()` waits for `ready-to-show` too (it would show a hidden window early). `attachWindowStateEvents` tracks native and HTML-element fullscreen as two flags OR-ed into `WINDOW:STATE_CHANGED`, because Electron leaves only the HTML state when the window was already natively fullscreen - Persists the app zoom level (issue #1109): the preload restores it with `webFrame.setZoomLevel` (temporary, frame-bound zoom; level answered over the synchronous `WINDOW:GET_ZOOM_LEVEL` IPC, applied at `DOMContentLoaded` and acknowledged with `WINDOW:ZOOM_LEVEL_APPLIED` — any earlier `webFrame.setZoomLevel` leaves a hidden Linux/Windows window without `ready-to-show`), never `webContents.setZoomLevel` — under `file://` Chromium keys zoom by the full URL, so the app's path routing would reset it on the next resize after a section change, and dev mode (`http://localhost`) never shows that. `app/services/window-zoom-level.ts` writes the live level to electron-conf `ZOOM_LEVEL` on close, `before-quit` and before every cross-document navigation (a reload drops the temporary level). Contract: `docs/architecture/workspace-shell.md`, "Zoom level" +- Recovers a renderer reload on an in-app route: the packaged renderer is `index.html` over `file://` with path routing, so a reload of `file:///…/web/workspace/sources` asks for a path with no file behind it. A main-process reload (the macOS default menu's View › Reload, DevTools) failed with `ERR_FILE_NOT_FOUND` and stranded the window on Chromium's error page; a renderer-initiated one (the settings unsaved-changes guard's confirmed `location.reload()`) was cancelled by the `will-navigate` trust check and silently did nothing. `app/services/renderer-reload-fallback.ts` handles both — `attachRendererReloadFallback` answers the main-frame `did-fail-load` (deferred to the error page's `dom-ready`: a load issued from inside the failure event yields a document that never paints), and `handleRendererNavigation` recognizes a routed renderer URL (`resolveRoutedRendererUrl`) — by re-loading the packaged index with the route in the `restoreRoute` query parameter (`restoreRendererRoute`); `apps/web/src/main.ts` consumes it before Angular bootstraps (`resolveRestoredRendererRoute` in `libs/shared/interfaces`, resolved against `document.baseURI` and confined to the renderer directory). A failed `index.html` itself is never re-requested. Contract: `docs/architecture/workspace-shell.md`, "Reloading the renderer on an in-app route" - Holds a single-instance lock (`app/services/single-instance.ts`), requested after the `userData` override so E2E runs with their own data dir keep independent locks. A second launch quits and focuses the running window; concurrent instances would otherwise share a Chromium profile whose IndexedDB only one of them can lock, silently breaking renderer-side settings persistence. `IPTVNATOR_ALLOW_MULTIPLE_INSTANCES=1` opts out for local debugging. The guard also forwards that launch's argv and working directory, so `iptvnator playlist.m3u` against a running app opens the playlist instead of being discarded. **Database**: diff --git a/apps/electron-backend-e2e/src/renderer-reload.e2e.ts b/apps/electron-backend-e2e/src/renderer-reload.e2e.ts new file mode 100644 index 000000000..8184bb2ef --- /dev/null +++ b/apps/electron-backend-e2e/src/renderer-reload.e2e.ts @@ -0,0 +1,82 @@ +import { + closeElectronApp, + expect, + expectPathname, + launchElectronApp, + openSettings, + openSettingsSection, + openSources, + test, +} from './electron-test-fixtures'; +import { + expectRendererReloadedOnRoute, + reloadFromMainProcess, + reloadFromRenderer, +} from './renderer-reload.support'; + +/** + * The packaged renderer is index.html over file:// with path routing, so + * once the user is on a section the document URL names a path with no file + * behind it. A main-process reload of that URL used to fail with + * ERR_FILE_NOT_FOUND and leave the window on Chromium's error page until the + * app restarted, and a renderer-initiated reload was cancelled by the + * navigation guard and silently did nothing. Both now boot the app straight + * back into the route it was on. + */ +test.describe('Renderer reload on an in-app route', () => { + test('@electron @window a main-process reload keeps the app on the Sources page', async ({ + dataDir, + }) => { + const app = await launchElectronApp(dataDir); + + try { + await openSources(app.mainWindow); + + await reloadFromMainProcess(app); + + await expectRendererReloadedOnRoute( + app.mainWindow, + /\/workspace\/sources$/ + ); + // A fresh data dir has no sources: the page shows its empty state. + await expect( + app.mainWindow.getByRole('heading', { + name: 'Add your first playlist', + }) + ).toBeVisible(); + // The page is alive, not a leftover paint: navigation still works. + await app.mainWindow + .getByRole('link', { name: 'Dashboard', exact: true }) + .click(); + await expectPathname(app.mainWindow, /\/workspace\/dashboard$/); + } finally { + await closeElectronApp(app); + } + }); + + test('@electron @window a renderer-initiated reload keeps the app on its settings section', async ({ + dataDir, + }) => { + const app = await launchElectronApp(dataDir); + + try { + await openSettings(app.mainWindow); + await openSettingsSection(app.mainWindow, 'playback'); + + await reloadFromRenderer(app); + + await expectRendererReloadedOnRoute( + app.mainWindow, + /\/workspace\/settings\/playback$/ + ); + await expect( + app.mainWindow.getByTestId('settings-container') + ).toBeVisible(); + await expect( + app.mainWindow.getByTestId('settings-section-playback') + ).toBeVisible(); + } finally { + await closeElectronApp(app); + } + }); +}); diff --git a/apps/electron-backend-e2e/src/renderer-reload.support.ts b/apps/electron-backend-e2e/src/renderer-reload.support.ts new file mode 100644 index 000000000..569adf408 --- /dev/null +++ b/apps/electron-backend-e2e/src/renderer-reload.support.ts @@ -0,0 +1,69 @@ +import { expect, Page } from '@playwright/test'; +import { LaunchedElectronApp } from './electron-test-fixtures'; + +/** + * Helpers for E2E that reload the packaged renderer on an in-app route. + * + * The URL before and after a recovered reload is the same routed `file://` + * URL, so waiting on the URL alone passes before anything happened. The + * current document is marked instead, and the wait is for a document + * WITHOUT the mark that has reached the route. + */ + +const DOCUMENT_MARK = 'data-e2e-pre-reload'; + +async function markCurrentDocument(page: Page): Promise { + await page.evaluate((attribute) => { + document.documentElement.setAttribute(attribute, ''); + }, DOCUMENT_MARK); +} + +/** What the macOS View › Reload menu role does: `webContents.reload()`. */ +export async function reloadFromMainProcess( + app: LaunchedElectronApp +): Promise { + await markCurrentDocument(app.mainWindow); + await app.electronApp.evaluate(({ BrowserWindow }) => { + const [win] = BrowserWindow.getAllWindows(); + win.webContents.reload(); + }); +} + +/** + * What the settings unsaved-changes guard does after a confirmed reload: + * `window.location.reload()`. The evaluate may lose its execution context + * to the navigation it starts; that is not a failure. + */ +export async function reloadFromRenderer( + app: LaunchedElectronApp +): Promise { + await markCurrentDocument(app.mainWindow); + await app.mainWindow + .evaluate(() => { + window.location.reload(); + }) + .catch(() => undefined); +} + +/** + * Waits until a NEW document is rendered on `pathname` — the app re-booted + * on the route rather than the old document still being on screen — and + * checks that the restore parameter was consumed on the way. + */ +export async function expectRendererReloadedOnRoute( + page: Page, + pathname: RegExp +): Promise { + await expect(page.locator(`html[${DOCUMENT_MARK}]`)).toHaveCount(0); + await expect(page).toHaveURL(pathname); + await expect + .poll(() => + page.evaluate( + () => + document.querySelector('app-root')?.innerHTML.trim() + .length ?? 0 + ) + ) + .toBeGreaterThan(0); + expect(new URL(page.url()).searchParams.has('restoreRoute')).toBe(false); +} diff --git a/apps/electron-backend-e2e/src/window-zoom-level.e2e.ts b/apps/electron-backend-e2e/src/window-zoom-level.e2e.ts index e8a27c64e..0fe5714ea 100644 --- a/apps/electron-backend-e2e/src/window-zoom-level.e2e.ts +++ b/apps/electron-backend-e2e/src/window-zoom-level.e2e.ts @@ -1,4 +1,3 @@ -import { join } from 'path'; import { closeElectronApp, expect, @@ -7,8 +6,11 @@ import { openSources, restartElectronApp, test, - workspaceRoot, } from './electron-test-fixtures'; +import { + expectRendererReloadedOnRoute, + reloadFromMainProcess, +} from './renderer-reload.support'; /** * Chromium's zoom factor as the renderer actually renders it: the window's @@ -36,15 +38,16 @@ async function zoomInFromMenu(app: LaunchedElectronApp): Promise { /** * A cross-document navigation of the renderer (what a reload is for zoom: * Chromium drops the temporary level and the new document's preload must - * restore it). Loads the packaged index the way startup does — a plain - * `page.reload()` on a routed `file://` URL has no file behind it. + * restore it). A real reload of the routed `file://` URL: the main process + * recovers the missing file by re-loading the index on the same route + * (`renderer-reload.e2e.ts`). */ async function reloadRenderer(app: LaunchedElectronApp): Promise { - await app.electronApp.evaluate(({ BrowserWindow }, indexPath) => { - const [win] = BrowserWindow.getAllWindows(); - return win.loadFile(indexPath); - }, join(workspaceRoot, 'dist/apps/web/index.html')); - await app.mainWindow.waitForSelector('app-root'); + await reloadFromMainProcess(app); + await expectRendererReloadedOnRoute( + app.mainWindow, + /\/workspace\/sources$/ + ); } async function resizeWindowBy( diff --git a/apps/electron-backend/src/app/app.spec.ts b/apps/electron-backend/src/app/app.spec.ts index 37cb9033e..029227e2e 100644 --- a/apps/electron-backend/src/app/app.spec.ts +++ b/apps/electron-backend/src/app/app.spec.ts @@ -55,7 +55,7 @@ import { isTrustedRendererNavigationUrl, } from './app'; import App from './app'; -import { app as electronApp, BrowserWindow, screen } from 'electron'; +import { app as electronApp, BrowserWindow, screen, shell } from 'electron'; import * as path from 'path'; import { pathToFileURL } from 'url'; import { store } from './services/store.service'; @@ -504,6 +504,9 @@ describe('Electron app security helpers', () => { return mainWindow; } + // Every listener of the event fires, since the window registers + // more than one `did-start-navigation` listener (zoom persistence + // and the renderer reload recovery). function fireHandlers( calls: Array<[string, (...args: unknown[]) => void]>, eventName: string, @@ -513,8 +516,10 @@ describe('Electron app security helpers', () => { .filter(([name]) => name === eventName) .map(([, handler]) => handler); - expect(handlers).toHaveLength(1); - handlers[0](...args); + expect(handlers.length).toBeGreaterThanOrEqual(1); + for (const handler of handlers) { + handler(...args); + } } function fireWindowEvent(win: MockMainWindow, eventName: string): void { @@ -613,6 +618,99 @@ describe('Electron app security helpers', () => { }); }); + describe('renderer reload recovery', () => { + const rendererRoot = path.dirname( + path.resolve(__dirname, '..', 'web', 'index.html') + ); + const routedUrl = pathToFileURL( + path.join(rendererRoot, 'workspace', 'sources') + ).href; + + function fireWebContentsEvent( + win: MockMainWindow, + eventName: string, + ...args: unknown[] + ): void { + const handlers = win.webContents.on.mock.calls + .filter(([name]) => name === eventName) + .map(([, handler]) => handler); + + expect(handlers).toHaveLength(1); + handlers[0](...args); + } + + function createPackagedWindow(): MockMainWindow { + process.env.ELECTRON_IS_DEV = '0'; + const mainWindow = createMockMainWindow(); + (BrowserWindow as unknown as jest.Mock).mockReturnValue(mainWindow); + getAppInternals().onReady(); + return mainWindow; + } + + it('re-loads the packaged index with the route a failed file:// reload named', () => { + const mainWindow = createPackagedWindow(); + + fireWebContentsEvent( + mainWindow, + 'did-fail-load', + {}, + -6, + 'ERR_FILE_NOT_FOUND', + routedUrl, + true + ); + // Deferred to the error page's dom-ready, or the new document + // never paints. + expect(mainWindow.loadFile).not.toHaveBeenCalled(); + fireWebContentsEvent(mainWindow, 'dom-ready'); + + expect(mainWindow.loadFile).toHaveBeenCalledWith( + path.join(rendererRoot, 'index.html'), + { query: { restoreRoute: 'workspace/sources' } } + ); + }); + + it('sends a renderer-initiated reload of a routed URL to the index instead of cancelling it', () => { + const mainWindow = createPackagedWindow(); + const event = { preventDefault: jest.fn() }; + + fireWebContentsEvent(mainWindow, 'will-navigate', event, routedUrl); + + expect(event.preventDefault).toHaveBeenCalled(); + expect(mainWindow.loadFile).toHaveBeenCalledWith( + path.join(rendererRoot, 'index.html'), + { query: { restoreRoute: 'workspace/sources' } } + ); + expect(shell.openExternal).not.toHaveBeenCalled(); + }); + + it('still opens external URLs in the browser and blocks other navigations', () => { + const mainWindow = createPackagedWindow(); + const external = { preventDefault: jest.fn() }; + const foreignFile = { preventDefault: jest.fn() }; + + fireWebContentsEvent( + mainWindow, + 'will-navigate', + external, + 'https://example.com/' + ); + fireWebContentsEvent( + mainWindow, + 'will-navigate', + foreignFile, + pathToFileURL(path.join(rendererRoot, '..', 'other')).href + ); + + expect(external.preventDefault).toHaveBeenCalled(); + expect(shell.openExternal).toHaveBeenCalledWith( + 'https://example.com/' + ); + expect(foreignFile.preventDefault).toHaveBeenCalled(); + expect(mainWindow.loadFile).not.toHaveBeenCalled(); + }); + }); + it('creates the main window immediately when Electron is already ready', () => { const mainWindow = createMockMainWindow(); (BrowserWindow as unknown as jest.Mock).mockReturnValue(mainWindow); diff --git a/apps/electron-backend/src/app/app.ts b/apps/electron-backend/src/app/app.ts index 7d1ef0c1f..ee9eae415 100644 --- a/apps/electron-backend/src/app/app.ts +++ b/apps/electron-backend/src/app/app.ts @@ -22,6 +22,11 @@ import { attachZoomLevelPersistence, persistZoomLevel, } from './services/window-zoom-level'; +import { + attachRendererReloadFallback, + resolveRoutedRendererUrl, + restoreRendererRoute, +} from './services/renderer-reload-fallback'; import { isEmbeddedMpvFeatureEnabled } from './services/embedded-mpv-runtime-policy.util'; import { FULLSCREEN_LAUNCH_SWITCH, @@ -358,6 +363,20 @@ export default class App { event.preventDefault(); + // A renderer-initiated reload (`location.reload()`, e.g. the + // settings unsaved-changes guard) on an in-app route arrives here as + // a routed file:// URL with no file behind it. Send it straight to + // the packaged index with that route instead of cancelling it. + if (!App.isDevelopmentMode() && App.mainWindow) { + const rendererIndexPath = getPackagedRendererIndexPath(); + const route = resolveRoutedRendererUrl(url, rendererIndexPath); + + if (route !== null) { + restoreRendererRoute(App.mainWindow, rendererIndexPath, route); + return; + } + } + if (isExternalBrowserUrl(url)) { shell.openExternal(url); } @@ -559,6 +578,14 @@ export default class App { // main process only saves it back before a reload drops it. attachZoomLevelPersistence(App.mainWindow); + // A reload on an in-app route asks file:// for a path that does not + // exist; re-load the packaged index with that route instead of + // leaving Chromium's error page (see renderer-reload-fallback.ts). + attachRendererReloadFallback( + App.mainWindow, + getPackagedRendererIndexPath() + ); + // Emitted when the window is closed. App.mainWindow.on('closed', () => { // Dereference the window object, usually you would store windows diff --git a/apps/electron-backend/src/app/services/renderer-reload-fallback.spec.ts b/apps/electron-backend/src/app/services/renderer-reload-fallback.spec.ts new file mode 100644 index 000000000..877b54abd --- /dev/null +++ b/apps/electron-backend/src/app/services/renderer-reload-fallback.spec.ts @@ -0,0 +1,351 @@ +import { join, resolve } from 'path'; +import { pathToFileURL } from 'url'; +import { + attachRendererReloadFallback, + ERR_FILE_NOT_FOUND, + RendererReloadFallbackWindow, + resolveReloadedRendererRoute, + resolveRoutedRendererUrl, + restoreRendererRoute, +} from './renderer-reload-fallback'; + +const rendererRoot = resolve('/opt/iptvnator/resources/app/web'); +const rendererIndexPath = join(rendererRoot, 'index.html'); + +function routedUrl(route: string): string { + return pathToFileURL(join(rendererRoot, route)).href; +} + +function failure( + validatedUrl: string, + overrides: { errorCode?: number; isMainFrame?: boolean } = {} +) { + return { + errorCode: ERR_FILE_NOT_FOUND, + isMainFrame: true, + validatedUrl, + ...overrides, + }; +} + +describe('resolveRoutedRendererUrl', () => { + it('maps a routed file URL under the renderer root to its route', () => { + expect( + resolveRoutedRendererUrl( + routedUrl('workspace/sources'), + rendererIndexPath + ) + ).toBe('workspace/sources'); + expect( + resolveRoutedRendererUrl( + routedUrl('workspace/settings/playback'), + rendererIndexPath + ) + ).toBe('workspace/settings/playback'); + }); + + it('rejects the index itself, http URLs and paths outside the root', () => { + expect( + resolveRoutedRendererUrl( + pathToFileURL(rendererIndexPath).href, + rendererIndexPath + ) + ).toBe(null); + expect( + resolveRoutedRendererUrl( + 'http://localhost:4200/workspace/sources', + rendererIndexPath + ) + ).toBe(null); + expect( + resolveRoutedRendererUrl( + pathToFileURL(resolve('/opt/iptvnator/other')).href, + rendererIndexPath + ) + ).toBe(null); + }); +}); + +describe('resolveReloadedRendererRoute', () => { + it('maps a routed file URL under the renderer root to its route', () => { + expect( + resolveReloadedRendererRoute( + failure(routedUrl('workspace/sources')), + rendererIndexPath + ) + ).toBe('workspace/sources'); + }); + + it('keeps the query and fragment of the failed URL', () => { + expect( + resolveReloadedRendererRoute( + failure( + `${routedUrl('workspace/xtreams/3/search')}?q=dune#top` + ), + rendererIndexPath + ) + ).toBe('workspace/xtreams/3/search?q=dune#top'); + }); + + it('decodes percent-encoded path segments', () => { + expect( + resolveReloadedRendererRoute( + failure(routedUrl('workspace/playlists/a b')), + rendererIndexPath + ) + ).toBe('workspace/playlists/a b'); + }); + + it('never re-requests the index itself, which would loop', () => { + expect( + resolveReloadedRendererRoute( + failure(pathToFileURL(rendererIndexPath).href), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure( + `${pathToFileURL(rendererIndexPath).href}?restoreRoute=x` + ), + rendererIndexPath + ) + ).toBe(null); + }); + + it('ignores paths outside the renderer root', () => { + expect( + resolveReloadedRendererRoute( + failure(pathToFileURL(resolve('/opt/iptvnator/other')).href), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure(pathToFileURL(rendererRoot).href), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure( + pathToFileURL(join(rendererRoot, '..', 'sibling')).href + ), + rendererIndexPath + ) + ).toBe(null); + }); + + it('ignores other error codes, subframes and non-file URLs', () => { + expect( + resolveReloadedRendererRoute( + failure(routedUrl('workspace/sources'), { errorCode: -3 }), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure(routedUrl('workspace/sources'), { isMainFrame: false }), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure('http://localhost:4200/workspace/sources'), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure('chrome-error://chromewebdata/'), + rendererIndexPath + ) + ).toBe(null); + expect( + resolveReloadedRendererRoute( + failure('not a url'), + rendererIndexPath + ) + ).toBe(null); + }); +}); + +describe('restoreRendererRoute', () => { + it('loads the index with the route in the query string', () => { + const win = { + isDestroyed: jest.fn(() => false), + loadFile: jest.fn(() => Promise.resolve()), + }; + + restoreRendererRoute(win, rendererIndexPath, 'workspace/sources?q=1'); + + expect(win.loadFile).toHaveBeenCalledWith(rendererIndexPath, { + query: { restoreRoute: 'workspace/sources?q=1' }, + }); + }); +}); + +describe('attachRendererReloadFallback', () => { + type Listener = (...args: unknown[]) => void; + + function createWindow() { + const listeners = new Map(); + const win = { + isDestroyed: jest.fn(() => false), + loadFile: jest.fn(() => Promise.resolve()), + webContents: { + on: jest.fn((event: string, listener: Listener) => { + listeners.set(event, [ + ...(listeners.get(event) ?? []), + listener, + ]); + }), + }, + }; + attachRendererReloadFallback( + win as unknown as RendererReloadFallbackWindow, + rendererIndexPath + ); + const emit = (event: string, ...args: unknown[]) => { + const handlers = listeners.get(event) ?? []; + expect(handlers).toHaveLength(1); + handlers[0](...args); + }; + const failRoutedLoad = (url = routedUrl('workspace/sources')) => { + emit('did-start-navigation', { + isMainFrame: true, + isSameDocument: false, + }); + emit( + 'did-fail-load', + {}, + ERR_FILE_NOT_FOUND, + 'ERR_FILE_NOT_FOUND', + url, + true + ); + }; + return { win, emit, failRoutedLoad }; + } + + it('re-loads the index with the failed route once the error page is ready', () => { + const { win, emit, failRoutedLoad } = createWindow(); + + failRoutedLoad(); + // Never from inside did-fail-load: that document would never paint. + expect(win.loadFile).not.toHaveBeenCalled(); + + emit('dom-ready'); + + expect(win.loadFile).toHaveBeenCalledTimes(1); + expect(win.loadFile).toHaveBeenCalledWith(rendererIndexPath, { + query: { restoreRoute: 'workspace/sources' }, + }); + }); + + it('recovers only once per failure', () => { + const { win, emit, failRoutedLoad } = createWindow(); + + failRoutedLoad(); + emit('dom-ready'); + // The recovery load's own document. + emit('did-start-navigation', { + isMainFrame: true, + isSameDocument: false, + }); + emit('dom-ready'); + + expect(win.loadFile).toHaveBeenCalledTimes(1); + }); + + it('withdraws the recovery when another navigation starts first', () => { + const { win, emit, failRoutedLoad } = createWindow(); + + failRoutedLoad(); + emit('did-start-navigation', { + isMainFrame: true, + isSameDocument: false, + }); + emit('dom-ready'); + + expect(win.loadFile).not.toHaveBeenCalled(); + }); + + it('keeps the recovery across in-page and subframe navigations', () => { + const { win, emit, failRoutedLoad } = createWindow(); + + failRoutedLoad(); + emit('did-start-navigation', { + isMainFrame: true, + isSameDocument: true, + }); + emit('did-start-navigation', { + isMainFrame: false, + isSameDocument: false, + }); + emit('dom-ready'); + + expect(win.loadFile).toHaveBeenCalledTimes(1); + }); + + it('leaves failures it cannot recover alone', () => { + const { win, emit } = createWindow(); + + emit( + 'did-fail-load', + {}, + -3, + 'ERR_ABORTED', + routedUrl('workspace/sources'), + true + ); + emit( + 'did-fail-load', + {}, + ERR_FILE_NOT_FOUND, + 'ERR_FILE_NOT_FOUND', + routedUrl('workspace/sources'), + false + ); + emit( + 'did-fail-load', + {}, + ERR_FILE_NOT_FOUND, + 'ERR_FILE_NOT_FOUND', + pathToFileURL(rendererIndexPath).href, + true + ); + emit('dom-ready'); + + expect(win.loadFile).not.toHaveBeenCalled(); + }); + + it('does not touch a destroyed window', () => { + const { win, emit, failRoutedLoad } = createWindow(); + win.isDestroyed.mockReturnValue(true); + + failRoutedLoad(); + emit('dom-ready'); + + expect(win.loadFile).not.toHaveBeenCalled(); + }); + + it('reports a failed recovery load instead of rejecting unhandled', async () => { + const { win, emit, failRoutedLoad } = createWindow(); + const errorSpy = jest + .spyOn(console, 'error') + .mockImplementation(() => undefined); + win.loadFile.mockReturnValue(Promise.reject(new Error('gone'))); + + failRoutedLoad(); + emit('dom-ready'); + await Promise.resolve(); + await Promise.resolve(); + + expect(errorSpy).toHaveBeenCalledWith( + 'Failed to restore the renderer after a reload:', + expect.any(Error) + ); + errorSpy.mockRestore(); + }); +}); diff --git a/apps/electron-backend/src/app/services/renderer-reload-fallback.ts b/apps/electron-backend/src/app/services/renderer-reload-fallback.ts new file mode 100644 index 000000000..fb9bbcf6e --- /dev/null +++ b/apps/electron-backend/src/app/services/renderer-reload-fallback.ts @@ -0,0 +1,216 @@ +/** + * Recovery for a reload of the packaged renderer on an in-app route. + * + * The packaged renderer is `dist/apps/web/index.html` over `file://` and + * Angular routes by PATH, so after the user opens a section the document URL + * is `file:///…/web/workspace/sources`. Nothing exists at that path, and a + * reload of it goes wrong in one of two ways depending on who starts it: + * + * - A main-process reload (`webContents.reload()`: the macOS View › Reload + * menu role, DevTools) fires no `will-navigate`. It fails with + * `ERR_FILE_NOT_FOUND`, Chromium commits `chrome-error://chromewebdata/` + * and the window stays dead until the app restarts. + * `attachRendererReloadFallback` answers that exact main-frame failure by + * loading `index.html` again with the failed URL's route in the query. + * - A renderer-initiated reload (`window.location.reload()`: the settings + * unsaved-changes guard after the user confirmed a reload intent) does + * fire `will-navigate`, where the routed URL is not the trusted index and + * the navigation guard cancels it — silently, so the confirmed reload + * never happens. `resolveRoutedRendererUrl` lets that guard recognize the + * URL and `restoreRendererRoute` sends it straight to the index with the + * route, without a failed load in between. + * + * The renderer's `main.ts` restores the route before Angular bootstraps + * (`resolveRestoredRendererRoute`). A failed `index.html` itself is never + * re-requested (it would loop), and dev mode serves `http://localhost`, + * whose dev server already falls back to the index. + */ + +import { RENDERER_RESTORE_ROUTE_QUERY_PARAM } from '@iptvnator/shared/interfaces'; +import { dirname, isAbsolute, relative, resolve, sep } from 'path'; +import { fileURLToPath } from 'url'; +import { isWindowTraceEnabled, trace } from './debug-trace'; + +/** Chromium net error for a `file://` URL with no file behind it. */ +export const ERR_FILE_NOT_FOUND = -6; + +export interface RendererLoadFailure { + errorCode: number; + isMainFrame: boolean; + validatedUrl: string; +} + +/** The slice of `Electron.WebContents` the fallback listens on. */ +export interface RendererReloadFallbackWebContents { + on( + event: 'did-fail-load', + listener: ( + event: unknown, + errorCode: number, + errorDescription: string, + validatedURL: string, + isMainFrame: boolean + ) => void + ): unknown; + on( + event: 'did-start-navigation', + listener: (details: { + isMainFrame: boolean; + isSameDocument: boolean; + }) => void + ): unknown; + on(event: 'dom-ready', listener: () => void): unknown; +} + +/** The slice of `Electron.BrowserWindow` the recovery needs. */ +export interface RendererReloadFallbackWindow { + isDestroyed(): boolean; + loadFile( + filePath: string, + options: { query: Record } + ): Promise; + webContents: RendererReloadFallbackWebContents; +} + +/** + * The in-app route a routed renderer URL stands for — a `file://` path + * under the renderer root, relative to it, plus its query and fragment — + * or `null` for anything else, including the index itself (which exists + * and must never be rewritten, or the recovery would loop). + */ +export function resolveRoutedRendererUrl( + url: string, + rendererIndexPath: string +): string | null { + let parsedUrl: URL; + + try { + parsedUrl = new URL(url); + } catch { + return null; + } + + if (parsedUrl.protocol !== 'file:') { + return null; + } + + let filePath: string; + + try { + filePath = resolve(fileURLToPath(parsedUrl)); + } catch { + return null; + } + + const indexPath = resolve(rendererIndexPath); + const rendererRoot = dirname(indexPath); + const relativePath = relative(rendererRoot, filePath); + + if ( + relativePath === '' || + isAbsolute(relativePath) || + relativePath === '..' || + relativePath.startsWith(`..${sep}`) || + filePath === indexPath + ) { + return null; + } + + return `${relativePath.split(sep).join('/')}${parsedUrl.search}${parsedUrl.hash}`; +} + +/** + * The route a failed load stood for, or `null` when the failure is not a + * main-frame `ERR_FILE_NOT_FOUND` for a routed renderer URL. + */ +export function resolveReloadedRendererRoute( + failure: RendererLoadFailure, + rendererIndexPath: string +): string | null { + if (failure.errorCode !== ERR_FILE_NOT_FOUND || !failure.isMainFrame) { + return null; + } + + return resolveRoutedRendererUrl(failure.validatedUrl, rendererIndexPath); +} + +/** + * Loads the packaged index with `route` in the query string, for the + * renderer to restore before Angular bootstraps. A no-op on a destroyed + * window; a failed load is reported, never left as an unhandled rejection. + */ +export function restoreRendererRoute( + win: Pick, + rendererIndexPath: string, + route: string +): void { + if (win.isDestroyed()) { + return; + } + + if (isWindowTraceEnabled()) { + trace('window', 'restore-route', { route }); + } + + void win + .loadFile(rendererIndexPath, { + query: { [RENDERER_RESTORE_ROUTE_QUERY_PARAM]: route }, + }) + .catch((error) => { + console.error( + 'Failed to restore the renderer after a reload:', + error + ); + }); +} + +/** + * Re-loads the packaged index with the failed route whenever a main-frame + * load of a routed `file://` URL under the renderer root fails with + * `ERR_FILE_NOT_FOUND`. + * + * The recovery load is deferred to the error page's `dom-ready`, never + * issued from inside `did-fail-load`: a `loadFile` started while Chromium + * is still committing the error page produces a document that never + * receives animation frames — the splash stays on screen and the window + * never paints, with `document.visibilityState` still `visible` — whereas + * the same load after `dom-ready` paints normally (verified on the packaged + * build; `did-fail-load` → `dom-ready` is the order Electron emits them). + * A cross-document navigation that starts in between, anything other than + * the failed load itself, withdraws the pending recovery, so the error + * page's `dom-ready` can never re-load the index over a newer navigation. + */ +export function attachRendererReloadFallback( + win: RendererReloadFallbackWindow, + rendererIndexPath: string +): void { + let pendingRoute: string | null = null; + + win.webContents.on('did-start-navigation', (details) => { + if (details.isMainFrame && !details.isSameDocument) { + pendingRoute = null; + } + }); + win.webContents.on( + 'did-fail-load', + (_event, errorCode, _errorDescription, validatedURL, isMainFrame) => { + const route = resolveReloadedRendererRoute( + { errorCode, isMainFrame, validatedUrl: validatedURL }, + rendererIndexPath + ); + + if (route !== null) { + pendingRoute = route; + } + } + ); + win.webContents.on('dom-ready', () => { + if (pendingRoute === null) { + return; + } + + const route = pendingRoute; + pendingRoute = null; + restoreRendererRoute(win, rendererIndexPath, route); + }); +} diff --git a/apps/web/src/main.ts b/apps/web/src/main.ts index cf089c664..b19bf15d1 100644 --- a/apps/web/src/main.ts +++ b/apps/web/src/main.ts @@ -1,10 +1,23 @@ import { bootstrapApplication } from '@angular/platform-browser'; +import { resolveRestoredRendererRoute } from '@iptvnator/shared/interfaces'; import { registerAppDateLocales } from './app/app-date-locales'; import { AppComponent } from './app/app.component'; import { appConfig } from './app/app.config'; registerAppDateLocales(); +// A reloaded packaged renderer arrives on index.html with the route it was +// on carried in the query string (the Electron main process recovers the +// file:// reload that way). Put that route back before the router reads the +// URL for its initial navigation. +const restoredHref = resolveRestoredRendererRoute( + window.location.href, + document.baseURI +); +if (restoredHref !== null) { + window.history.replaceState(window.history.state, '', restoredHref); +} + bootstrapApplication(AppComponent, appConfig) .then(() => { // Splash is rendered eagerly by index.html so the user sees something diff --git a/docs/architecture/workspace-shell.md b/docs/architecture/workspace-shell.md index 731e5cb6f..617e74136 100644 --- a/docs/architecture/workspace-shell.md +++ b/docs/architecture/workspace-shell.md @@ -461,6 +461,80 @@ Zoom level (Cmd/Ctrl and +/−, issue #1109): the rendered factor (content width ÷ `window.innerWidth`) across a section change, a resize, a reload and a restart. +Reloading the renderer on an in-app route: + +1. The packaged renderer is `dist/apps/web/index.html` over `file://` and + Angular routes by path (no hash strategy), so once the user is on a + section the document URL is `file:///…/web/workspace/sources` — a path + with no file behind it. A reload of that URL fails with + `ERR_FILE_NOT_FOUND` (-6) or is cancelled outright, depending on who + starts it. Two user-reachable triggers: the macOS default application + menu (nothing calls `Menu.setApplicationMenu`, so View › Reload / Force + Reload are live; Windows/Linux drop the menu bar via `setMenu(null)`), + and the settings unsaved-changes guard, which calls + `window.location.reload()` after the user confirms a reload intent on + `/workspace/settings/
`. Dev mode (`http://localhost:4200`) never + shows either — the dev server serves the index for every path. +2. Both legs live in `services/renderer-reload-fallback.ts` and end in the + same `restoreRendererRoute`: load the packaged index with the routed + URL's route — its path relative to the renderer root plus query and + fragment (`resolveRoutedRendererUrl`) — in the `restoreRoute` query + parameter. + - A main-process reload (`webContents.reload()`, the menu role, + DevTools) fires no `will-navigate`, so it cannot be redirected up + front: it fails, Chromium commits `chrome-error://chromewebdata/` + and `app-root` stays empty until the app restarts. + `attachRendererReloadFallback` recovers it after the fact from the + main-frame `did-fail-load` with `ERR_FILE_NOT_FOUND` + (`resolveReloadedRendererRoute`); other error codes, subframes and + non-`file:` URLs are left alone. The recovery load is deferred to + the error page's `dom-ready` and never issued from inside + `did-fail-load`: a `loadFile` started while Chromium is still + committing the error page yields a document that never receives + animation frames — the splash stays, nothing paints, while + `document.visibilityState` still says `visible` — and the same load + after `dom-ready` paints normally (Electron emits `did-fail-load` + before that `dom-ready`). A cross-document navigation starting in + between withdraws the pending recovery, so a stale `dom-ready` can + never re-load the index over a newer navigation. + - A renderer-initiated reload (`location.reload()`, the settings + guard) does fire `will-navigate`, where the routed URL is not the + trusted index and `handleRendererNavigation` would cancel it — + silently, so the confirmed reload simply never happened. The handler + now recognizes a routed renderer URL and sends it straight to the + index with its route, with no failed load in between; every other + untrusted navigation is still blocked (external URLs still open in + the browser). + A failed `index.html` itself is never re-requested (it would loop): + `resolveRoutedRendererUrl` rejects the index, and the recovery load + carries `index.html` as its path, so a second failure cannot recurse. +3. The renderer consumes the parameter before Angular bootstraps: + `apps/web/src/main.ts` calls `resolveRestoredRendererRoute` + (`libs/shared/interfaces/src/lib/renderer-reload-route.util.ts`, which + also owns the parameter name) and installs the result with + `history.replaceState`, so the router's initial navigation lands on the + route the user was on. The route is resolved against `document.baseURI` + (the packaged ``, i.e. the renderer directory — the same + prefix Angular strips from `location.pathname`), and anything that would + leave that directory (an absolute URL, another scheme, a `..` escape) + is dropped with only the parameter removed, so the app boots at its + default route instead of following an arbitrary target. +4. Zoom persistence is unaffected: the failed reload's + `did-start-navigation` already saved the level and released ownership, + the recovery load's `did-start-navigation` is then a no-op, and the new + document's preload restores the level as after any other reload. The + main-process close guard also treats the recovery like any full + navigation (`did-navigate` disarms it). +5. Regression coverage: `renderer-reload.e2e.ts` reloads from the main + process (`webContents.reload()`, the menu role) on Sources and from the + renderer (`window.location.reload()`, the settings guard) on a settings + section and asserts a NEW document is rendered on the same route with + the parameter gone (`renderer-reload.support.ts` marks the old document, + since the URL alone is identical before and after); + `window-zoom-level.e2e.ts` reloads the same way. Unit coverage: + `renderer-reload-fallback.spec.ts`, `renderer-reload-route.util.spec.ts`, + `app.spec.ts` ("renderer reload recovery"). + Layout integration: 1. `document.body` gets a `frameless-platform` class (set in diff --git a/libs/shared/interfaces/src/index.ts b/libs/shared/interfaces/src/index.ts index 470e6ed23..9c76d4de0 100644 --- a/libs/shared/interfaces/src/index.ts +++ b/libs/shared/interfaces/src/index.ts @@ -47,6 +47,7 @@ export * from './lib/provider-overview.util'; export * from './lib/random-id.util'; export * from './lib/recording-metadata.interface'; export * from './lib/recording-program-overlap.util'; +export * from './lib/renderer-reload-route.util'; export * from './lib/security-policy-error.utils'; export * from './lib/settings.interface'; export * from './lib/stalker-auth-failure.util'; diff --git a/libs/shared/interfaces/src/lib/renderer-reload-route.util.spec.ts b/libs/shared/interfaces/src/lib/renderer-reload-route.util.spec.ts new file mode 100644 index 000000000..84bc3e9b7 --- /dev/null +++ b/libs/shared/interfaces/src/lib/renderer-reload-route.util.spec.ts @@ -0,0 +1,85 @@ +import { + RENDERER_RESTORE_ROUTE_QUERY_PARAM, + resolveRestoredRendererRoute, +} from './renderer-reload-route.util'; + +const rendererRoot = 'file:///Applications/IPTVnator.app/Contents/web/'; +const packagedIndex = `${rendererRoot}index.html`; + +function reloadedIndex(route: string): string { + const url = new URL(packagedIndex); + url.searchParams.set(RENDERER_RESTORE_ROUTE_QUERY_PARAM, route); + return url.href; +} + +describe('resolveRestoredRendererRoute', () => { + it('leaves a document without a restore request alone', () => { + expect(resolveRestoredRendererRoute(packagedIndex, packagedIndex)).toBe( + null + ); + expect( + resolveRestoredRendererRoute( + `${packagedIndex}?other=1`, + packagedIndex + ) + ).toBe(null); + }); + + it('restores a route relative to the renderer directory', () => { + expect( + resolveRestoredRendererRoute( + reloadedIndex('workspace/sources'), + packagedIndex + ) + ).toBe(`${rendererRoot}workspace/sources`); + }); + + it('keeps the restored route query and fragment', () => { + expect( + resolveRestoredRendererRoute( + reloadedIndex('workspace/xtreams/3/search?q=dune#top'), + packagedIndex + ) + ).toBe(`${rendererRoot}workspace/xtreams/3/search?q=dune#top`); + }); + + it('resolves against the base URI, not the document URL', () => { + // The packaged resolves to the renderer directory + // even when the document itself sits deeper. + expect( + resolveRestoredRendererRoute( + `${rendererRoot}workspace/sources?${RENDERER_RESTORE_ROUTE_QUERY_PARAM}=workspace%2Fdashboard`, + rendererRoot + ) + ).toBe(`${rendererRoot}workspace/dashboard`); + }); + + it('works for an http origin with a root base href', () => { + expect( + resolveRestoredRendererRoute( + `http://localhost:4200/?${RENDERER_RESTORE_ROUTE_QUERY_PARAM}=workspace%2Fsettings%2Fplayback`, + 'http://localhost:4200/' + ) + ).toBe('http://localhost:4200/workspace/settings/playback'); + }); + + it.each([ + ['an absolute URL', 'https://example.com/phish'], + ['another scheme', 'javascript:alert(1)'], + ['a directory escape', '../../etc/passwd'], + ['a root-absolute path outside the renderer', '/etc/passwd'], + ['a scheme-relative URL', '//example.com/'], + ['the renderer directory itself', './'], + ['an empty route', ''], + ])('drops %s and only removes the parameter', (_label, route) => { + expect( + resolveRestoredRendererRoute(reloadedIndex(route), packagedIndex) + ).toBe(packagedIndex); + }); + + it('returns null for an unparsable document URL', () => { + expect(resolveRestoredRendererRoute('not a url', packagedIndex)).toBe( + null + ); + }); +}); diff --git a/libs/shared/interfaces/src/lib/renderer-reload-route.util.ts b/libs/shared/interfaces/src/lib/renderer-reload-route.util.ts new file mode 100644 index 000000000..4190060b7 --- /dev/null +++ b/libs/shared/interfaces/src/lib/renderer-reload-route.util.ts @@ -0,0 +1,72 @@ +/** + * Route hand-off for a reloaded packaged renderer. + * + * The packaged renderer is loaded from `dist/apps/web/index.html` over + * `file://` and Angular uses PATH routing, so after in-app navigation the + * document URL is `file:///…/web/workspace/sources` — a path with no file + * behind it. A reload (the macOS View › Reload menu, DevTools, the settings + * unsaved-changes guard's confirmed reload) therefore fails with + * `ERR_FILE_NOT_FOUND` and strands the window on Chromium's error page. + * + * The Electron main process recovers such a failure by loading `index.html` + * again with the failed URL's route (its path relative to the renderer root, + * plus query and fragment) carried in this query parameter. The renderer + * consumes it before Angular bootstraps: `resolveRestoredRendererRoute` + * turns the current document URL into the in-app URL the router should + * start from, and `main.ts` installs it with `history.replaceState`, so the + * router's initial navigation lands on the route the user was on. + */ + +/** Query parameter carrying the route to restore on the reloaded index. */ +export const RENDERER_RESTORE_ROUTE_QUERY_PARAM = 'restoreRoute'; + +/** + * The URL to present instead of `currentHref` before the router's initial + * navigation, or `null` when the document carries no restore request. + * + * `baseUri` is `document.baseURI`: the packaged build's `` + * resolves to the renderer's directory, which is also what Angular strips + * from `location.pathname` to obtain the route. A route that would leave + * that directory — an absolute URL, another scheme, a `..` escape — is + * dropped and only the parameter is removed, so the app boots at its + * default route rather than following an arbitrary target. + */ +export function resolveRestoredRendererRoute( + currentHref: string, + baseUri: string +): string | null { + let current: URL; + let baseDirectory: URL; + + try { + current = new URL(currentHref); + baseDirectory = new URL('./', baseUri); + } catch { + return null; + } + + const route = current.searchParams.get(RENDERER_RESTORE_ROUTE_QUERY_PARAM); + + if (route === null) { + return null; + } + + current.searchParams.delete(RENDERER_RESTORE_ROUTE_QUERY_PARAM); + const stripped = current.href; + + let target: URL; + + try { + target = new URL(route, baseDirectory); + } catch { + return stripped; + } + + const staysInsideRenderer = + target.protocol === baseDirectory.protocol && + target.host === baseDirectory.host && + target.pathname.startsWith(baseDirectory.pathname) && + target.pathname !== baseDirectory.pathname; + + return staysInsideRenderer ? target.href : stripped; +}