From 361c0f1334e319238f933e4bccc39ca4cc8ddaeb Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 8 Jun 2026 01:12:42 -0700 Subject: [PATCH] fix(notifications): gate overlay delivery on visible overlay; default to - Default notificationType fallback changed from 'overlay' to 'osd' - isVisibleOverlayContentReady guards on overlay visible + window ready - All overlay hide paths dismiss loading OSD notification - notifyConfiguredStatus falls back to desktop when overlay not ready - anilist deps builder preserves undefined optional callbacks as undefined - settingsEnumValues field added to ConfigOptionRegistryEntry - Drop !important from z-index; lower yomitan popup z-index below notification stack --- src/anki-integration.ts | 2 +- src/anki-integration/ui-feedback.test.ts | 19 +++++++ src/anki-integration/ui-feedback.ts | 2 +- src/config/definitions/shared.ts | 9 ++++ src/main.ts | 18 ++++--- src/main/main-wiring.test.ts | 51 +++++++++++++++++-- .../anilist-setup-protocol-main-deps.test.ts | 12 +++++ .../anilist-setup-protocol-main-deps.ts | 6 ++- .../configured-status-notification.test.ts | 8 +-- .../runtime/configured-status-notification.ts | 3 +- .../runtime/startup-osd-sequencer.test.ts | 6 +-- src/renderer/style.css | 4 +- 12 files changed, 114 insertions(+), 26 deletions(-) diff --git a/src/anki-integration.ts b/src/anki-integration.ts index 025d5e90..9afa1834 100644 --- a/src/anki-integration.ts +++ b/src/anki-integration.ts @@ -886,7 +886,7 @@ export class AnkiIntegration { } private getNotificationType(): NotificationType { - return this.config.behavior?.notificationType ?? 'overlay'; + return this.config.behavior?.notificationType ?? 'osd'; } private shouldUseOsdNotifications(): boolean { diff --git a/src/anki-integration/ui-feedback.test.ts b/src/anki-integration/ui-feedback.test.ts index 5bbcaec7..9b453050 100644 --- a/src/anki-integration/ui-feedback.test.ts +++ b/src/anki-integration/ui-feedback.test.ts @@ -83,6 +83,25 @@ test('showStatusNotification falls back to system when overlay delivery is unava assert.deepEqual(calls, ['system:SubMiner:Waiting for card update']); }); +test('showStatusNotification defaults to mpv osd when notification type is unset', () => { + const calls: string[] = []; + + showStatusNotification('Card updated', { + getNotificationType: () => undefined, + showOsd: (message) => { + calls.push(`osd:${message}`); + }, + showOverlayNotification: (payload) => { + calls.push(`overlay:${payload.body}`); + }, + showSystemNotification: (title, options) => { + calls.push(`system:${title}:${options.body}`); + }, + }); + + assert.deepEqual(calls, ['osd:Card updated']); +}); + test('showStatusNotification does not duplicate system notifications for both', () => { const calls: string[] = []; diff --git a/src/anki-integration/ui-feedback.ts b/src/anki-integration/ui-feedback.ts index 7614fea4..efc6f472 100644 --- a/src/anki-integration/ui-feedback.ts +++ b/src/anki-integration/ui-feedback.ts @@ -38,7 +38,7 @@ export function showStatusNotification( message: string, context: UiFeedbackNotificationContext, ): void { - const type = context.getNotificationType() ?? 'overlay'; + const type = context.getNotificationType() ?? 'osd'; if (type === 'none') { return; diff --git a/src/config/definitions/shared.ts b/src/config/definitions/shared.ts index 2b6c13b4..71f939a0 100644 --- a/src/config/definitions/shared.ts +++ b/src/config/definitions/shared.ts @@ -27,7 +27,16 @@ export interface ConfigOptionRegistryEntry { kind: ConfigValueKind; defaultValue: unknown; description: string; + /** + * Complete runtime-valid enum options, including legacy file-config values such as + * `osd` and `osd-system` in NOTIFICATION_TYPE_VALUES. + */ enumValues?: readonly string[]; + /** + * Optional settings UI subset when legacy/runtime-valid enum options should remain + * editable in config files but hidden from new UI choices, for example + * SETTINGS_NOTIFICATION_TYPE_VALUES. + */ settingsEnumValues?: readonly string[]; runtime?: RuntimeOptionRegistryEntry; } diff --git a/src/main.ts b/src/main.ts index 94820e68..bf192322 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3432,7 +3432,11 @@ function broadcastToOverlayWindows(channel: string, ...args: unknown[]): void { function isVisibleOverlayContentReady(): boolean { const overlayWindow = overlayManager.getMainWindow(); - return Boolean(overlayWindow && isOverlayWindowContentReady(overlayWindow)); + return Boolean( + overlayManager.getVisibleOverlayVisible() && + overlayWindow && + isOverlayWindowReadyForNotification(overlayWindow), + ); } function getConfiguredStatusNotificationType(): NotificationType { @@ -3451,20 +3455,15 @@ function isOverlayWindowReadyForNotification(window: BrowserWindow): boolean { return currentURL !== '' && currentURL !== 'about:blank'; } -function hasReadyOverlayNotificationWindow(): boolean { - return getOverlayWindows().some((window) => isOverlayWindowReadyForNotification(window)); -} - const overlayNotificationDelivery = createOverlayNotificationDelivery({ - hasReadyOverlayWindow: () => hasReadyOverlayNotificationWindow(), + hasReadyOverlayWindow: () => isVisibleOverlayContentReady(), send: (payload) => { broadcastToOverlayWindows(IPC_CHANNELS.event.overlayNotification, payload); }, scheduleFlushRetry: (callback, delayMs) => setTimeout(callback, delayMs), clearFlushRetry: (handle) => clearTimeout(handle as ReturnType), }); -let overlayLoadingOsdController: ReturnType | null = - null; +let overlayLoadingOsdController: ReturnType | null = null; function flushQueuedOverlayNotifications(): void { overlayNotificationDelivery.flush(); @@ -7850,6 +7849,7 @@ function notifyMpvPluginVisibleOverlayVisibility(visible: boolean): void { function setVisibleOverlayVisible(visible: boolean): void { ensureOverlayWindowsReadyForVisibilityActions(); if (!visible) { + dismissOverlayLoadingStatusNotification(); autoplayReadyGate.markCurrentMediaAutoplayReady(); cancelVisibleOverlaySubtitleRefreshAfterFirstPaint(); cancelPendingLinuxMpvFullscreenOverlayRefreshBurst(); @@ -7871,6 +7871,7 @@ function toggleVisibleOverlay(): void { ensureOverlayWindowsReadyForVisibilityActions(); const nextVisible = !overlayManager.getVisibleOverlayVisible(); if (!nextVisible) { + dismissOverlayLoadingStatusNotification(); autoplayReadyGate.markCurrentMediaAutoplayReady(); cancelVisibleOverlaySubtitleRefreshAfterFirstPaint(); cancelPendingLinuxMpvFullscreenOverlayRefreshBurst(); @@ -7888,6 +7889,7 @@ function toggleVisibleOverlay(): void { } function setOverlayVisible(visible: boolean): void { if (!visible) { + dismissOverlayLoadingStatusNotification(); cancelVisibleOverlaySubtitleRefreshAfterFirstPaint(); resetVisibleOverlayInputState(); autoplayReadyGate.markCurrentMediaAutoplayReady(); diff --git a/src/main/main-wiring.test.ts b/src/main/main-wiring.test.ts index 1e26170c..294dc425 100644 --- a/src/main/main-wiring.test.ts +++ b/src/main/main-wiring.test.ts @@ -112,7 +112,7 @@ test('manual visible overlay toggles only release current-media autoplay when hi assert.ok(actionBlock); assert.match( actionBlock, - /if \(!nextVisible\) \{\s+autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);\s+cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);\s+cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);/, + /if \(!nextVisible\) \{[\s\S]*?autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);[\s\S]*?cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);[\s\S]*?cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);/, ); }); @@ -133,15 +133,15 @@ test('all visible overlay hide paths clear stale overlay input state', () => { assert.ok(setOverlayBlock); assert.match( setVisibleBlock, - /if \(!visible\) \{\s+autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);\s+cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);\s+cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);\s+resetVisibleOverlayInputState\(\);/, + /if \(!visible\) \{[\s\S]*?autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);[\s\S]*?cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);[\s\S]*?cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);[\s\S]*?resetVisibleOverlayInputState\(\);/, ); assert.match( toggleBlock, - /if \(!nextVisible\) \{\s+autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);\s+cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);\s+cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);\s+resetVisibleOverlayInputState\(\);/, + /if \(!nextVisible\) \{[\s\S]*?autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);[\s\S]*?cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);[\s\S]*?cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);[\s\S]*?resetVisibleOverlayInputState\(\);/, ); assert.match( setOverlayBlock, - /if \(!visible\) \{\s+cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);\s+resetVisibleOverlayInputState\(\);\s+autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);\s+cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);/, + /if \(!visible\) \{[\s\S]*?cancelVisibleOverlaySubtitleRefreshAfterFirstPaint\(\);[\s\S]*?resetVisibleOverlayInputState\(\);[\s\S]*?autoplayReadyGate\.markCurrentMediaAutoplayReady\(\);[\s\S]*?cancelPendingLinuxMpvFullscreenOverlayRefreshBurst\(\);/, ); }); @@ -418,6 +418,49 @@ test('manual visible overlay changes notify mpv plugin visibility state', () => assert.match(toggleBlock, /notifyMpvPluginVisibleOverlayVisibility\(nextVisible\);/); }); +test('manual visible overlay hide dismisses loading OSD', () => { + const source = readMainSource(); + const setBlock = source.match( + /function setVisibleOverlayVisible\(visible: boolean\): void \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + const toggleBlock = source.match( + /function toggleVisibleOverlay\(\): void \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + const setOverlayBlock = source.match( + /function setOverlayVisible\(visible: boolean\): void \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + + assert.ok(setBlock); + assert.ok(toggleBlock); + assert.ok(setOverlayBlock); + assert.match(setBlock, /if \(!visible\) \{[\s\S]*?dismissOverlayLoadingStatusNotification\(\);/); + assert.match( + toggleBlock, + /if \(!nextVisible\) \{[\s\S]*?dismissOverlayLoadingStatusNotification\(\);/, + ); + assert.match( + setOverlayBlock, + /if \(!visible\) \{[\s\S]*?dismissOverlayLoadingStatusNotification\(\);/, + ); +}); + +test('configured overlay notifications require visible ready overlay window', () => { + const source = readMainSource(); + const readinessBlock = source.match( + /function isVisibleOverlayContentReady\(\): boolean \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + const statusBlock = source.match( + /function showConfiguredStatusNotification\([\s\S]*?\): void \{(?[\s\S]*?)\n\}/, + )?.groups?.body; + + assert.ok(readinessBlock); + assert.ok(statusBlock); + assert.match(readinessBlock, /overlayManager\.getVisibleOverlayVisible\(\)/); + assert.match(readinessBlock, /isOverlayWindowReadyForNotification\(overlayWindow\)/); + assert.doesNotMatch(readinessBlock, /isOverlayWindowContentReady\(overlayWindow\)/); + assert.match(statusBlock, /isOverlayReady: \(\) => isVisibleOverlayContentReady\(\)/); +}); + test('manual visible overlay show primes current subtitle from mpv before relying on live events', () => { const source = readMainSource(); const setBlock = source.match( diff --git a/src/main/runtime/anilist-setup-protocol-main-deps.test.ts b/src/main/runtime/anilist-setup-protocol-main-deps.test.ts index dc975e61..d02fd972 100644 --- a/src/main/runtime/anilist-setup-protocol-main-deps.test.ts +++ b/src/main/runtime/anilist-setup-protocol-main-deps.test.ts @@ -23,6 +23,18 @@ test('notify anilist setup main deps builder maps callbacks', () => { assert.deepEqual(calls, ['osd:ok', 'notify:SubMiner', 'log:done']); }); +test('notify anilist setup main deps builder preserves optional notification callbacks', () => { + const deps = createBuildNotifyAnilistSetupMainDepsHandler({ + hasMpvClient: () => true, + showMpvOsd: () => {}, + showDesktopNotification: () => {}, + logInfo: () => {}, + })(); + + assert.equal(deps.getNotificationType, undefined); + assert.equal(deps.showOverlayNotification, undefined); +}); + test('consume anilist setup token main deps builder maps callbacks', () => { const calls: string[] = []; const deps = createBuildConsumeAnilistSetupTokenFromUrlMainDepsHandler({ diff --git a/src/main/runtime/anilist-setup-protocol-main-deps.ts b/src/main/runtime/anilist-setup-protocol-main-deps.ts index daff6eec..a9201ec7 100644 --- a/src/main/runtime/anilist-setup-protocol-main-deps.ts +++ b/src/main/runtime/anilist-setup-protocol-main-deps.ts @@ -18,10 +18,12 @@ type RegisterSubminerProtocolClientMainDeps = Parameters< export function createBuildNotifyAnilistSetupMainDepsHandler(deps: NotifyAnilistSetupMainDeps) { return (): NotifyAnilistSetupMainDeps => ({ - getNotificationType: () => deps.getNotificationType?.(), + getNotificationType: deps.getNotificationType ? () => deps.getNotificationType?.() : undefined, hasMpvClient: () => deps.hasMpvClient(), showMpvOsd: (message: string) => deps.showMpvOsd(message), - showOverlayNotification: (payload) => deps.showOverlayNotification?.(payload), + showOverlayNotification: deps.showOverlayNotification + ? (payload) => deps.showOverlayNotification?.(payload) + : undefined, showDesktopNotification: (title: string, options: { body: string }) => deps.showDesktopNotification(title, options), logInfo: (message: string) => deps.logInfo(message), diff --git a/src/main/runtime/configured-status-notification.test.ts b/src/main/runtime/configured-status-notification.test.ts index a3460ea7..174ed52a 100644 --- a/src/main/runtime/configured-status-notification.test.ts +++ b/src/main/runtime/configured-status-notification.test.ts @@ -28,7 +28,7 @@ test('notifyConfiguredStatus routes both to overlay and system without osd', () ]); }); -test('notifyConfiguredStatus queues pre-overlay both status through overlay sender and desktop', () => { +test('notifyConfiguredStatus falls back to desktop for pre-overlay both status', () => { const calls: string[] = []; notifyConfiguredStatus('Overlay loading...', { @@ -43,10 +43,10 @@ test('notifyConfiguredStatus queues pre-overlay both status through overlay send calls.push(`desktop:${title}:${options.body ?? ''}`), }); - assert.deepEqual(calls, ['overlay::Overlay loading...', 'desktop:SubMiner:Overlay loading...']); + assert.deepEqual(calls, ['desktop:SubMiner:Overlay loading...']); }); -test('notifyConfiguredStatus queues pre-overlay overlay-only status without osd fallback', () => { +test('notifyConfiguredStatus falls back to desktop for pre-overlay overlay-only status', () => { const calls: string[] = []; notifyConfiguredStatus('Overlay loading...', { @@ -61,7 +61,7 @@ test('notifyConfiguredStatus queues pre-overlay overlay-only status without osd calls.push(`desktop:${title}:${options.body ?? ''}`), }); - assert.deepEqual(calls, ['overlay::Overlay loading...']); + assert.deepEqual(calls, ['desktop:SubMiner:Overlay loading...']); }); test('notifyConfiguredStatus routes pre-overlay system status to desktop only', () => { diff --git a/src/main/runtime/configured-status-notification.ts b/src/main/runtime/configured-status-notification.ts index f6489d50..e9a264e6 100644 --- a/src/main/runtime/configured-status-notification.ts +++ b/src/main/runtime/configured-status-notification.ts @@ -50,7 +50,8 @@ export function notifyConfiguredStatus( } if (showOverlay) { - if (deps.showOverlayNotification) { + const overlayReady = deps.isOverlayReady?.() ?? true; + if (deps.showOverlayNotification && overlayReady) { deps.showOverlayNotification({ id: options.id, title: options.title ?? 'SubMiner', diff --git a/src/main/runtime/startup-osd-sequencer.test.ts b/src/main/runtime/startup-osd-sequencer.test.ts index b3d70117..6fdbc39c 100644 --- a/src/main/runtime/startup-osd-sequencer.test.ts +++ b/src/main/runtime/startup-osd-sequencer.test.ts @@ -170,7 +170,7 @@ test('startup OSD reset preserves in-flight tokenization loading for ready updat }, showOverlayNotification: (payload) => { calls.push( - `overlay:${payload.id}:${payload.body}:${payload.variant}:${payload.persistent ? 'pin' : 'auto'}`, + `overlay:${payload.id}:${payload.title}:${payload.body}:${payload.variant}:${payload.persistent ? 'pin' : 'auto'}`, ); }, showDesktopNotification: (title, options) => { @@ -183,8 +183,8 @@ test('startup OSD reset preserves in-flight tokenization loading for ready updat sequencer.markTokenizationReady(); assert.deepEqual(calls, [ - 'overlay:startup-tokenization:Loading subtitle tokenization...:progress:pin', - 'overlay:startup-tokenization:Subtitle tokenization ready:success:auto', + 'overlay:startup-tokenization:Subtitle tokenization:Loading subtitle tokenization...:progress:pin', + 'overlay:startup-tokenization:Subtitle tokenization:Subtitle tokenization ready:success:auto', 'desktop:SubMiner:Subtitle tokenization ready', ]); }); diff --git a/src/renderer/style.css b/src/renderer/style.css index a5bc306d..45663ab5 100644 --- a/src/renderer/style.css +++ b/src/renderer/style.css @@ -175,7 +175,7 @@ body:focus-visible, flex-direction: column; gap: 8px; pointer-events: auto; - z-index: 2147483647 !important; + z-index: 2147483647; } .overlay-notification-stack.position-top-left { @@ -1900,7 +1900,7 @@ iframe.yomitan-popup, iframe[id^='yomitan-popup'], [data-subminer-yomitan-popup-host='true'] { pointer-events: auto !important; - z-index: 2147483647 !important; + z-index: 2147483645; } .kiku-info-text {