diff --git a/src/core/utils/changelog-parse.test.ts b/src/core/utils/changelog-parse.test.ts index fe371535..f75e15df 100644 --- a/src/core/utils/changelog-parse.test.ts +++ b/src/core/utils/changelog-parse.test.ts @@ -166,6 +166,14 @@ test('changelog parser reads prerelease and build metadata version headings', () assert.equal(entries[1]?.date, '2026-05-29'); assert.equal(entries[1]?.groupKey, '0.15'); assert.equal(entries[0]?.sections.length, 1); + // The prerelease body has to land on its own entry, not fold into 0.16.0. + assert.deepEqual(entries[1]?.sections, [ + { + heading: 'Added', + items: [{ text: 'Release candidate note.', children: [] }], + internal: false, + }, + ]); }); test('changelog parser handles the repo CHANGELOG.md', () => { diff --git a/src/main/runtime/changelog/changelog-runtime.test.ts b/src/main/runtime/changelog/changelog-runtime.test.ts new file mode 100644 index 00000000..bafa45c5 --- /dev/null +++ b/src/main/runtime/changelog/changelog-runtime.test.ts @@ -0,0 +1,87 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + CHANGELOG_REQUEST_TIMEOUT_MS, + createChangelogRuntime, + withRequestTimeout, +} from './changelog-runtime'; +import type { FetchLike, FetchResponseLike } from '../update/release-assets'; + +function okResponse(body: string): FetchResponseLike { + return { + ok: true, + status: 200, + json: async () => JSON.parse(body), + text: async () => body, + arrayBuffer: async () => new ArrayBuffer(0), + }; +} + +test('request timeout wrapper attaches an abort signal to every request', async () => { + const seen: Array<{ url: string; init?: Record }> = []; + const wrapped = withRequestTimeout(async (url, init) => { + seen.push({ url, init }); + return okResponse('body'); + }, 1_234); + + await wrapped('https://example.test/a'); + await wrapped('https://example.test/b', { headers: { 'User-Agent': 'SubMiner' } }); + + assert.equal(seen.length, 2); + for (const request of seen) { + assert.ok(request.init?.signal instanceof AbortSignal, 'each request carries a signal'); + assert.equal((request.init?.signal as AbortSignal).aborted, false); + } + // Existing init is preserved rather than replaced. + assert.deepEqual(seen[1]?.init?.headers, { 'User-Agent': 'SubMiner' }); +}); + +test('request timeout wrapper aborts a request that never settles', async () => { + let observed: AbortSignal | undefined; + const wrapped = withRequestTimeout((_url, init) => { + observed = init?.signal as AbortSignal; + return new Promise(() => { + // Never resolves, standing in for a stalled connection. + }); + }, 10); + + void wrapped('https://example.test/stalled'); + await new Promise((resolve) => setTimeout(resolve, 40)); + + assert.equal(observed?.aborted, true); +}); + +test('changelog runtime times out both requests instead of hanging the modal', async () => { + const signals: Array = []; + const failing: FetchLike = (_url, init) => { + signals.push(init?.signal as AbortSignal | undefined); + return Promise.reject(new Error('stalled')); + }; + + const runtime = createChangelogRuntime({ + getInstalledVersion: () => '0.19.2', + getUpdateChannel: () => 'stable', + resourcesPath: '/res', + appPath: '/app', + dirname: '/app/dist/main', + joinPath: (...parts) => parts.join('/'), + fileExists: () => false, + readFile: () => '', + logWarn: () => {}, + createFetch: () => failing, + }); + + const snapshot = await runtime.getChangelogSnapshot(); + + // Release lookup and changelog download both go through the timeout wrapper. + assert.equal(signals.length, 2); + assert.ok(signals.every((signal) => signal instanceof AbortSignal)); + // No bundled copy is readable here, so the failure surfaces rather than hangs. + assert.match(snapshot.error ?? '', /stalled/); +}); + +test('changelog request timeout is finite', () => { + assert.ok(Number.isFinite(CHANGELOG_REQUEST_TIMEOUT_MS)); + assert.ok(CHANGELOG_REQUEST_TIMEOUT_MS > 0); +}); diff --git a/src/main/runtime/changelog/changelog-runtime.ts b/src/main/runtime/changelog/changelog-runtime.ts index 1f2208e1..f8c25ed6 100644 --- a/src/main/runtime/changelog/changelog-runtime.ts +++ b/src/main/runtime/changelog/changelog-runtime.ts @@ -19,14 +19,30 @@ export interface ChangelogRuntimeDeps { createFetch?: () => FetchLike; } +/** + * curl enforces its own `--max-time`, but the global-fetch transport has no + * deadline: without this a stalled connection leaves the modal on "Loading + * changelog..." with no way back except closing it. + */ +export const CHANGELOG_REQUEST_TIMEOUT_MS = 30_000; + +export function withRequestTimeout(fetchImpl: FetchLike, timeoutMs: number): FetchLike { + return (url, init) => { + if (typeof AbortSignal?.timeout !== 'function') return fetchImpl(url, init); + return fetchImpl(url, { ...init, signal: AbortSignal.timeout(timeoutMs) }); + }; +} + export function createChangelogRuntime(deps: ChangelogRuntimeDeps): { getChangelogSnapshot: (options?: { refresh?: boolean }) => Promise; } { // curl matches the updater's transport choice: Electron's global fetch is // unreliable for GitHub on some Linux builds. - const fetchImpl = + const fetchImpl = withRequestTimeout( deps.createFetch?.() ?? - (process.platform === 'win32' ? createGlobalFetch() : createCurlFetch()); + (process.platform === 'win32' ? createGlobalFetch() : createCurlFetch()), + CHANGELOG_REQUEST_TIMEOUT_MS, + ); const source = createChangelogSource({ fetchLatestReleaseTag: async () => { diff --git a/src/renderer/modals/modal-focus-guard.test.ts b/src/renderer/modals/modal-focus-guard.test.ts index 121cb1f6..e4e1b82e 100644 --- a/src/renderer/modals/modal-focus-guard.test.ts +++ b/src/renderer/modals/modal-focus-guard.test.ts @@ -29,6 +29,7 @@ type Harness = { focused: () => string[]; documentListeners: () => string[]; windowListeners: () => string[]; + handlerFor: (scope: 'document' | 'window', type: string) => Listener | undefined; setActiveElement: (value: unknown) => void; advanceClock: (ms: number) => void; runTimers: () => void; @@ -41,6 +42,8 @@ function createHarness( isModalLayer?: boolean; contains?: boolean; preferredVisible?: boolean; + preferredAcceptsFocus?: boolean; + extraPreferred?: boolean; fallback?: 'element' | null; } = {}, ): Harness { @@ -62,8 +65,22 @@ function createHarness( const realDateNow = Date.now; Date.now = () => now; const focused: string[] = []; - const documentListeners: string[] = []; - const windowListeners: string[] = []; + const documentListeners: Array<{ type: string; listener: Listener }> = []; + const windowListeners: Array<{ type: string; listener: Listener }> = []; + + const register = (registry: Array<{ type: string; listener: Listener }>) => ({ + add: (type: string, listener: Listener) => { + registry.push({ type, listener }); + }, + remove: (type: string, listener: Listener) => { + const index = registry.findIndex( + (entry) => entry.type === type && entry.listener === listener, + ); + if (index >= 0) registry.splice(index, 1); + }, + }); + const windowRegistry = register(windowListeners); + const documentRegistry = register(documentListeners); const timers: Array<() => void> = []; let activeElement: unknown = null; @@ -75,7 +92,15 @@ function createHarness( getClientRects: () => (options.preferredVisible === false ? [] : [{ width: 10, height: 10 }]), focus: () => { focused.push('preferred'); - activeElement = preferred; + // A rendered-but-unfocusable target (e.g. disabled) never becomes active. + if (options.preferredAcceptsFocus !== false) activeElement = preferred; + }, + }); + const secondPreferred = Object.assign(new TestElement(), { + getClientRects: () => [{ width: 10, height: 10 }], + focus: () => { + focused.push('second'); + activeElement = secondPreferred; }, }); const fallback = @@ -100,13 +125,8 @@ function createHarness( focus: () => { focused.push('window'); }, - addEventListener: (type: string) => { - windowListeners.push(type); - }, - removeEventListener: (type: string) => { - const index = windowListeners.indexOf(type); - if (index >= 0) windowListeners.splice(index, 1); - }, + addEventListener: windowRegistry.add, + removeEventListener: windowRegistry.remove, setTimeout: (callback: () => void) => { timers.push(callback); return timers.length; @@ -120,20 +140,18 @@ function createHarness( get activeElement() { return activeElement; }, - addEventListener: (type: string) => { - documentListeners.push(type); - }, - removeEventListener: (type: string) => { - const index = documentListeners.indexOf(type); - if (index >= 0) documentListeners.splice(index, 1); - }, + addEventListener: documentRegistry.add, + removeEventListener: documentRegistry.remove, }, }); const guard = createModalFocusGuard({ isOpen: options.isOpen ?? (() => true), getModalRoot: () => root as unknown as Element, - getPreferredFocusTargets: () => [preferred as unknown as HTMLElement], + getPreferredFocusTargets: () => + (options.extraPreferred + ? [preferred, secondPreferred] + : [preferred]) as unknown as HTMLElement[], getFallbackFocusTarget: () => fallback as unknown as Element | null, isModalLayer: options.isModalLayer ?? true, }); @@ -143,8 +161,12 @@ function createHarness( root, focusMainWindowCalls: () => focusMainWindowCalls, focused: () => focused, - documentListeners: () => documentListeners, - windowListeners: () => windowListeners, + documentListeners: () => documentListeners.map((entry) => entry.type), + windowListeners: () => windowListeners.map((entry) => entry.type), + handlerFor: (scope: 'document' | 'window', type: string) => + (scope === 'document' ? documentListeners : windowListeners).find( + (entry) => entry.type === type, + )?.listener, setActiveElement: (value: unknown) => { activeElement = value; }, @@ -190,6 +212,34 @@ test('modal focus guard attaches once and detaches every listener', () => { } }); +test('modal focus guard pulls focus back when focusin lands outside the modal', () => { + const harness = createHarness(); + try { + harness.guard.attach(); + + const focusin = harness.handlerFor('document', 'focusin'); + assert.ok(focusin, 'attach registers a focusin handler'); + + // focusin is not cancelable, so recovery is the only observable effect. + focusin?.({ target: {} }); + assert.deepEqual(harness.focused(), ['preferred']); + } finally { + harness.restore(); + } +}); + +test('modal focus guard ignores focusin while the modal is closed', () => { + const harness = createHarness({ isOpen: () => false }); + try { + harness.guard.attach(); + harness.handlerFor('document', 'focusin')?.({ target: {} }); + + assert.deepEqual(harness.focused(), []); + } finally { + harness.restore(); + } +}); + test('modal focus guard restores focus to the first rendered target', () => { // The target is position:fixed (null offsetParent) yet visible, so it must // still win over the fallback. @@ -202,6 +252,26 @@ test('modal focus guard restores focus to the first rendered target', () => { } }); +test('modal focus guard tries the next target when one refuses focus', () => { + const harness = createHarness({ preferredAcceptsFocus: false, extraPreferred: true }); + try { + assert.equal(harness.guard.focusFallbackTarget(), true); + assert.deepEqual(harness.focused(), ['preferred', 'second']); + } finally { + harness.restore(); + } +}); + +test('modal focus guard reaches the fallback when no preferred target takes focus', () => { + const harness = createHarness({ preferredAcceptsFocus: false }); + try { + assert.equal(harness.guard.focusFallbackTarget(), true); + assert.deepEqual(harness.focused(), ['preferred', 'fallback']); + } finally { + harness.restore(); + } +}); + test('modal focus guard falls back when no preferred target is rendered', () => { const harness = createHarness({ preferredVisible: false }); try { diff --git a/src/renderer/modals/modal-focus-guard.ts b/src/renderer/modals/modal-focus-guard.ts index 68ffa1b5..0c30c297 100644 --- a/src/renderer/modals/modal-focus-guard.ts +++ b/src/renderer/modals/modal-focus-guard.ts @@ -23,6 +23,7 @@ export function createModalFocusGuard(deps: ModalFocusGuardDeps) { let focusinGuard: ((event: FocusEvent) => void) | null = null; let windowFocusGuard: (() => void) | null = null; let pointerFocusGuard: ((event: Event) => void) | null = null; + let pointerFocusRoot: Element | null = null; let isRecovering = false; let lastRecoveryAt = 0; @@ -43,12 +44,12 @@ export function createModalFocusGuard(deps: ModalFocusGuardDeps) { // getClientRects() rather than offsetParent: the latter is null for // position:fixed elements, which would skip a perfectly visible target. - const preferred = deps - .getPreferredFocusTargets() - .find((target) => target.getClientRects().length > 0); - if (preferred) { - preferred.focus({ preventScroll: true }); - return document.activeElement === preferred; + // Rendered is not the same as focusable, so keep trying until one sticks + // instead of giving up on the first candidate that refuses focus. + for (const target of deps.getPreferredFocusTargets()) { + if (target.getClientRects().length === 0) continue; + target.focus({ preventScroll: true }); + if (document.activeElement === target) return true; } const fallback = deps.getFallbackFocusTarget(); @@ -80,10 +81,11 @@ export function createModalFocusGuard(deps: ModalFocusGuardDeps) { /** Idempotent; safe to call on every open. */ function attach(): void { if (focusinGuard === null) { + // focusin is not cancelable, so there is nothing to preventDefault here; + // focus is taken back afterwards instead. focusinGuard = (event: FocusEvent) => { if (!deps.isOpen()) return; if (!isModalFocusTarget(event.target)) { - event.preventDefault(); enforceModalFocus(); } }; @@ -95,9 +97,11 @@ export function createModalFocusGuard(deps: ModalFocusGuardDeps) { requestOverlayFocus(); enforceModalFocus(); }; - const root = deps.getModalRoot(); - root.addEventListener('pointerdown', pointerFocusGuard); - root.addEventListener('click', pointerFocusGuard); + // Remember the root we bound to: resolving it again on detach could + // return a different element and leak the listeners on the old one. + pointerFocusRoot = deps.getModalRoot(); + pointerFocusRoot.addEventListener('pointerdown', pointerFocusGuard); + pointerFocusRoot.addEventListener('click', pointerFocusGuard); } if (windowFocusGuard === null) { @@ -117,10 +121,10 @@ export function createModalFocusGuard(deps: ModalFocusGuardDeps) { } if (pointerFocusGuard) { - const root = deps.getModalRoot(); - root.removeEventListener('pointerdown', pointerFocusGuard); - root.removeEventListener('click', pointerFocusGuard); + pointerFocusRoot?.removeEventListener('pointerdown', pointerFocusGuard); + pointerFocusRoot?.removeEventListener('click', pointerFocusGuard); pointerFocusGuard = null; + pointerFocusRoot = null; } if (windowFocusGuard) {