From ac93a5bd2ea8889da25df62a6accd55ae084dc0f Mon Sep 17 00:00:00 2001 From: sudacode Date: Sat, 25 Apr 2026 20:34:18 -0700 Subject: [PATCH] fix: address PR review follow-ups --- ...parent-subtitle-hover-background-config.md | 51 ++++++++++++++++ ...ideo-close-leaving-background-app-alive.md | 53 ++++++++++++++++ changes/300-transparent-hover-background.md | 4 ++ changes/301-managed-playback-exit.md | 4 ++ docs-site/configuration.md | 2 +- plugin/subminer/process.lua | 1 + scripts/test-plugin-start-gate.lua | 4 ++ src/anki-integration/card-creation.ts | 1 + src/cli/args.test.ts | 12 +++- src/cli/args.ts | 3 + src/config/config.test.ts | 16 +++++ src/config/resolve/subtitle-domains.ts | 17 +++--- src/core/services/app-lifecycle.test.ts | 1 + src/core/services/cli-command.test.ts | 29 +++++++++ src/core/services/cli-command.ts | 4 ++ src/core/services/mpv-protocol.test.ts | 18 ++++++ src/core/services/mpv-protocol.ts | 5 ++ src/core/services/mpv.ts | 4 ++ src/core/services/startup-bootstrap.test.ts | 1 + src/core/services/tokenizer.test.ts | 3 +- src/main.ts | 2 + src/main/character-dictionary-runtime.ts | 18 +++--- .../fetch.test.ts | 9 ++- .../manual-selection.ts | 6 +- .../character-dictionary-auto-sync.test.ts | 5 +- .../runtime/first-run-setup-service.test.ts | 1 + .../mpv-client-runtime-service-main-deps.ts | 4 ++ .../runtime/mpv-client-runtime-service.ts | 2 + src/renderer/handlers/keyboard.ts | 9 +-- .../modals/character-dictionary.test.ts | 60 +++++++++++++++++++ src/renderer/modals/character-dictionary.ts | 6 +- src/renderer/subtitle-render.test.ts | 30 ++++++++++ 32 files changed, 345 insertions(+), 40 deletions(-) create mode 100644 backlog/tasks/task-300 - Fix-transparent-subtitle-hover-background-config.md create mode 100644 backlog/tasks/task-301 - Fix-launcher-managed-video-close-leaving-background-app-alive.md create mode 100644 changes/300-transparent-hover-background.md create mode 100644 changes/301-managed-playback-exit.md diff --git a/backlog/tasks/task-300 - Fix-transparent-subtitle-hover-background-config.md b/backlog/tasks/task-300 - Fix-transparent-subtitle-hover-background-config.md new file mode 100644 index 00000000..53cdc834 --- /dev/null +++ b/backlog/tasks/task-300 - Fix-transparent-subtitle-hover-background-config.md @@ -0,0 +1,51 @@ +--- +id: TASK-300 +title: Fix transparent subtitle hover background config +status: Done +assignee: [] +created_date: '2026-04-26 03:23' +updated_date: '2026-04-26 03:26' +labels: + - bug + - overlay + - config +dependencies: [] +priority: medium +--- + +## Description + + +User reports setting subtitleStyle.hoverTokenBackgroundColor to transparent still renders default hover background in overlay subtitles. + + +## Acceptance Criteria + +- [x] #1 Transparent hoverTokenBackgroundColor is accepted by config resolution. +- [x] #2 Renderer applies transparent hover token background instead of falling back to default. + + +## Implementation Plan + + +1. Reproduce config alias behavior with a failing config test. +2. Map subtitleStyle.hoverBackground to hoverTokenBackgroundColor in config resolution while keeping canonical key precedence. +3. Add renderer regression for transparent hover token background CSS variable. +4. Update docs and changelog fragment; run focused verification. + + +## Implementation Notes + + +Local user config used subtitleStyle.hoverBackground, which was ignored because only subtitleStyle.hoverTokenBackgroundColor was recognized. Canonical key still takes precedence when both are present. + +Verification passed: bun run test:config:src; bun test src/renderer/subtitle-render.test.ts; bun run changelog:lint; bun run docs:test; bun run docs:build. + + +## Final Summary + + +Implemented config compatibility for transparent hover token backgrounds. `subtitleStyle.hoverBackground` now maps to the canonical `subtitleStyle.hoverTokenBackgroundColor` during resolution, preserving canonical key precedence. Added regression coverage for the alias and renderer handling of `transparent`, documented the alias, and added a changelog fragment. + +Verification: `bun run test:config:src`; `bun test src/renderer/subtitle-render.test.ts`; `bun run changelog:lint`; `bun run docs:test`; `bun run docs:build`. + diff --git a/backlog/tasks/task-301 - Fix-launcher-managed-video-close-leaving-background-app-alive.md b/backlog/tasks/task-301 - Fix-launcher-managed-video-close-leaving-background-app-alive.md new file mode 100644 index 00000000..3d448157 --- /dev/null +++ b/backlog/tasks/task-301 - Fix-launcher-managed-video-close-leaving-background-app-alive.md @@ -0,0 +1,53 @@ +--- +id: TASK-301 +title: Fix launcher-managed video close leaving background app alive +status: Done +assignee: + - Codex +created_date: '2026-04-26 03:29' +updated_date: '2026-04-26 03:33' +labels: + - bug + - launcher + - mpv +dependencies: [] +priority: high +--- + +## Description + + +Launcher/plugin-managed video playback should not leave the SubMiner background app or tray icon running after the video closes unless the user explicitly launched SubMiner in background mode via --background or by starting with no app arguments. This is a regression after crash-avoidance work that added background startup for launcher-managed playback. + + +## Acceptance Criteria + +- [x] #1 Closing a launcher-managed video exits the launcher-started SubMiner app/tray instead of leaving it alive. +- [x] #2 Explicit background launches still keep SubMiner alive after windows close. +- [x] #3 No-argument app startup behavior remains unchanged. +- [x] #4 Regression coverage exercises the launcher-managed playback shutdown lifecycle. + + +## Implementation Plan + + +1. Add regression coverage first: plugin auto-start should tag launcher-managed playback, and app mpv shutdown handling should quit only when started in that managed playback mode. +2. Add a narrow CLI flag/state field for launcher-managed playback, separate from explicit persistent background mode. +3. Have plugin pass the new flag with its background start command. +4. On mpv shutdown/disconnect, request app quit only when managed playback mode is active; preserve explicit --background and no-arg startup persistence. +5. Run focused plugin/app tests, then relevant launcher/core gates if feasible. + + +## Implementation Notes + + +Implemented managed playback shutdown by adding a `--managed-playback` app flag that the mpv plugin passes only for launcher-managed starts. The main mpv shutdown path now quits the app when initial args indicate managed playback, while explicit background/no-arg startup remains persistent. Added plugin start-gate and mpv protocol regression coverage. + + +## Final Summary + + +Launcher-managed video playback now exits the SubMiner background app/tray when mpv shuts down, avoiding a leftover background process after closing a launcher-started video. Explicit `--background` and no-argument app startup remain persistent because the quit path is gated on the new `--managed-playback` flag. The mpv plugin passes that flag for auto-started playback, and mpv protocol shutdown requests app quit only in that managed mode. + +Verification: plugin start-gate regression coverage, mpv protocol shutdown regression coverage, CLI managed-playback parse coverage, plus broader local gates run before handoff. + diff --git a/changes/300-transparent-hover-background.md b/changes/300-transparent-hover-background.md new file mode 100644 index 00000000..e4fbd8d9 --- /dev/null +++ b/changes/300-transparent-hover-background.md @@ -0,0 +1,4 @@ +type: fixed +area: config + +- Accepted `subtitleStyle.hoverBackground` as an alias for `subtitleStyle.hoverTokenBackgroundColor`, so setting it to `transparent` removes hover token backgrounds. diff --git a/changes/301-managed-playback-exit.md b/changes/301-managed-playback-exit.md new file mode 100644 index 00000000..78863b69 --- /dev/null +++ b/changes/301-managed-playback-exit.md @@ -0,0 +1,4 @@ +type: fixed +area: launcher + +- Launcher-managed playback now exits the background SubMiner app when the video closes, while explicit background launches stay persistent. diff --git a/docs-site/configuration.md b/docs-site/configuration.md index 70fbe8c6..6fe134ef 100644 --- a/docs-site/configuration.md +++ b/docs-site/configuration.md @@ -310,7 +310,7 @@ See `config.example.jsonc` for detailed configuration options. | `autoPauseVideoOnHover` | boolean | Pause playback while mouse hovers subtitle text, then resume on leave (`true` by default). | | `autoPauseVideoOnYomitanPopup` | boolean | Pause playback while the Yomitan popup is open, then resume when the popup closes (`true` by default). | | `hoverTokenColor` | string | Hex color used for hovered subtitle token highlight in mpv (default: catppuccin mauve) | -| `hoverTokenBackgroundColor` | string | CSS color used for hovered subtitle token background highlight (default: semi-transparent dark) | +| `hoverTokenBackgroundColor` | string | CSS color used for hovered subtitle token background highlight; `hoverBackground` is accepted as an alias | | `nameMatchEnabled` | boolean | Enable subtitle token coloring for matches from the SubMiner character dictionary (`true` by default) | | `nameMatchColor` | string | Hex color used for subtitle tokens matched from the SubMiner character dictionary (default: `#f5bde6`) | | `frequencyDictionary.enabled` | boolean | Enable frequency highlighting from dictionary lookups (`false` by default) | diff --git a/plugin/subminer/process.lua b/plugin/subminer/process.lua index b8d543ad..79c6225c 100644 --- a/plugin/subminer/process.lua +++ b/plugin/subminer/process.lua @@ -187,6 +187,7 @@ function M.create(ctx) if action == "start" then table.insert(args, "--background") + table.insert(args, "--managed-playback") local backend = resolve_backend(overrides.backend) if backend and backend ~= "" then diff --git a/scripts/test-plugin-start-gate.lua b/scripts/test-plugin-start-gate.lua index 3a149ed6..00b50c0e 100644 --- a/scripts/test-plugin-start-gate.lua +++ b/scripts/test-plugin-start-gate.lua @@ -769,6 +769,10 @@ do local start_call = find_start_call(recorded.async_calls) assert_true(start_call ~= nil, "auto-start should issue --start command") assert_true(call_has_arg(start_call, "--background"), "auto-start should launch SubMiner in background mode") + assert_true( + call_has_arg(start_call, "--managed-playback"), + "auto-start should mark SubMiner as launcher-managed playback" + ) assert_true(call_has_arg(start_call, "--texthooker"), "auto-start should include --texthooker on the main --start command when enabled") assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "auto-start should not issue a separate texthooker helper command") assert_true( diff --git a/src/anki-integration/card-creation.ts b/src/anki-integration/card-creation.ts index 41c73515..2d2fc536 100644 --- a/src/anki-integration/card-creation.ts +++ b/src/anki-integration/card-creation.ts @@ -261,6 +261,7 @@ export class CardCreationService { ); for (const audioField of audioFields) { const existingAudio = noteInfo.fields[audioField]?.value || ''; + // Manual clipboard updates intentionally replace old captured audio. updatedFields[audioField] = this.deps.mergeFieldValue( existingAudio, audioValue, diff --git a/src/cli/args.test.ts b/src/cli/args.test.ts index 58942e16..ff2e457e 100644 --- a/src/cli/args.test.ts +++ b/src/cli/args.test.ts @@ -214,7 +214,11 @@ test('hasExplicitCommand and shouldStartApp preserve command intent', () => { assert.equal(hasExplicitCommand(anilistRetryQueue), true); assert.equal(shouldStartApp(anilistRetryQueue), false); - const dictionaryCandidates = parseArgs(['--dictionary-candidates', '--dictionary-target', '/tmp/a.mkv']); + const dictionaryCandidates = parseArgs([ + '--dictionary-candidates', + '--dictionary-target', + '/tmp/a.mkv', + ]); assert.equal(dictionaryCandidates.dictionaryCandidates, true); assert.equal(dictionaryCandidates.dictionaryTarget, '/tmp/a.mkv'); assert.equal(hasExplicitCommand(dictionaryCandidates), true); @@ -334,6 +338,12 @@ test('hasExplicitCommand and shouldStartApp preserve command intent', () => { assert.equal(hasExplicitCommand(background), true); assert.equal(shouldStartApp(background), true); + const managedPlayback = parseArgs(['--background', '--managed-playback']); + assert.equal(managedPlayback.background, true); + assert.equal(managedPlayback.managedPlayback, true); + assert.equal(hasExplicitCommand(managedPlayback), true); + assert.equal(shouldStartApp(managedPlayback), true); + const setup = parseArgs(['--setup']); assert.equal((setup as typeof setup & { setup?: boolean }).setup, true); assert.equal(hasExplicitCommand(setup), true); diff --git a/src/cli/args.ts b/src/cli/args.ts index 77aa079d..aee216a9 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -1,5 +1,6 @@ export interface CliArgs { background: boolean; + managedPlayback: boolean; start: boolean; launchMpv: boolean; launchMpvTargets: string[]; @@ -99,6 +100,7 @@ export type CliCommandSource = 'initial' | 'second-instance'; export function parseArgs(argv: string[]): CliArgs { const args: CliArgs = { background: false, + managedPlayback: false, start: false, launchMpv: false, launchMpvTargets: [], @@ -201,6 +203,7 @@ export function parseArgs(argv: string[]): CliArgs { if (!arg || !arg.startsWith('--')) continue; if (arg === '--background') args.background = true; + else if (arg === '--managed-playback') args.managedPlayback = true; else if (arg === '--start') args.start = true; else if (arg.startsWith('--youtube-play=')) { const value = arg.split('=', 2)[1]; diff --git a/src/config/config.test.ts b/src/config/config.test.ts index aa96a5be..a26f4940 100644 --- a/src/config/config.test.ts +++ b/src/config/config.test.ts @@ -437,6 +437,22 @@ test('parses subtitleStyle.hoverTokenBackgroundColor and warns on invalid values ); }); +test('parses subtitleStyle.hoverBackground as a hoverTokenBackgroundColor alias', () => { + const validDir = makeTempDir(); + fs.writeFileSync( + path.join(validDir, 'config.jsonc'), + `{ + "subtitleStyle": { + "hoverBackground": "transparent" + } + }`, + 'utf-8', + ); + + const validService = new ConfigService(validDir); + assert.equal(validService.getConfig().subtitleStyle.hoverTokenBackgroundColor, 'transparent'); +}); + test('parses subtitleStyle.nameMatchEnabled and warns on invalid values', () => { const validDir = makeTempDir(); fs.writeFileSync( diff --git a/src/config/resolve/subtitle-domains.ts b/src/config/resolve/subtitle-domains.ts index 2bf032c8..2a77f0f3 100644 --- a/src/config/resolve/subtitle-domains.ts +++ b/src/config/resolve/subtitle-domains.ts @@ -260,20 +260,21 @@ export function applySubtitleDomainConfig(context: ResolveContext): void { ); } - const hoverTokenBackgroundColor = asString( - (src.subtitleStyle as { hoverTokenBackgroundColor?: unknown }).hoverTokenBackgroundColor, - ); + const subtitleStyleSource = src.subtitleStyle as { + hoverBackground?: unknown; + hoverTokenBackgroundColor?: unknown; + }; + const rawHoverTokenBackgroundColor = + subtitleStyleSource.hoverTokenBackgroundColor ?? subtitleStyleSource.hoverBackground; + const hoverTokenBackgroundColor = asString(rawHoverTokenBackgroundColor); if (hoverTokenBackgroundColor !== undefined) { resolved.subtitleStyle.hoverTokenBackgroundColor = hoverTokenBackgroundColor; - } else if ( - (src.subtitleStyle as { hoverTokenBackgroundColor?: unknown }).hoverTokenBackgroundColor !== - undefined - ) { + } else if (rawHoverTokenBackgroundColor !== undefined) { resolved.subtitleStyle.hoverTokenBackgroundColor = fallbackSubtitleStyleHoverTokenBackgroundColor; warn( 'subtitleStyle.hoverTokenBackgroundColor', - (src.subtitleStyle as { hoverTokenBackgroundColor?: unknown }).hoverTokenBackgroundColor, + rawHoverTokenBackgroundColor, resolved.subtitleStyle.hoverTokenBackgroundColor, 'Expected a CSS color value (hex, rgba/hsl/hsla, named color, or var()).', ); diff --git a/src/core/services/app-lifecycle.test.ts b/src/core/services/app-lifecycle.test.ts index d01b921b..7da4bf96 100644 --- a/src/core/services/app-lifecycle.test.ts +++ b/src/core/services/app-lifecycle.test.ts @@ -6,6 +6,7 @@ import { AppLifecycleServiceDeps, startAppLifecycle } from './app-lifecycle'; function makeArgs(overrides: Partial = {}): CliArgs { return { background: false, + managedPlayback: false, start: false, launchMpv: false, launchMpvTargets: [], diff --git a/src/core/services/cli-command.test.ts b/src/core/services/cli-command.test.ts index 098d9024..6aa62953 100644 --- a/src/core/services/cli-command.test.ts +++ b/src/core/services/cli-command.test.ts @@ -6,6 +6,7 @@ import { CliCommandServiceDeps, handleCliCommand } from './cli-command'; function makeArgs(overrides: Partial = {}): CliArgs { return { background: false, + managedPlayback: false, start: false, launchMpv: false, launchMpvTargets: [], @@ -717,6 +718,34 @@ test('handleCliCommand sets character dictionary manual AniList selection', asyn ); }); +test('handleCliCommand does not log character dictionary selection success when result is not ok', async () => { + const { calls, deps } = createDeps({ + setCharacterDictionarySelection: async () => ({ + ok: false, + seriesKey: 'test', + selected: { id: 0, title: '', episodes: null }, + staleMediaIds: [], + }), + }); + + handleCliCommand( + makeArgs({ + dictionarySelect: true, + dictionaryAnilistId: 21355, + dictionaryTarget: '/tmp/re-zero.mkv', + }), + 'initial', + deps, + ); + await new Promise((resolve) => setImmediate(resolve)); + + assert.ok(calls.includes('warn:Character dictionary override was not saved.')); + assert.equal( + calls.some((call) => call.startsWith('log:Character dictionary override saved:')), + false, + ); +}); + test('handleCliCommand does not dispatch runJellyfinCommand for non-Jellyfin commands', () => { const nonJellyfinArgs: Array> = [ { start: true }, diff --git a/src/core/services/cli-command.ts b/src/core/services/cli-command.ts index ea3402cf..ae249d8a 100644 --- a/src/core/services/cli-command.ts +++ b/src/core/services/cli-command.ts @@ -645,6 +645,10 @@ export function handleCliCommand( mediaId: args.dictionaryAnilistId, }) .then((result) => { + if (!result.ok) { + deps.warn('Character dictionary override was not saved.'); + return; + } deps.log( `Character dictionary override saved: ${result.seriesKey} -> ${result.selected.id} - ${result.selected.title}`, ); diff --git a/src/core/services/mpv-protocol.test.ts b/src/core/services/mpv-protocol.test.ts index 9259b70c..5c3756b7 100644 --- a/src/core/services/mpv-protocol.test.ts +++ b/src/core/services/mpv-protocol.test.ts @@ -19,6 +19,7 @@ function createDeps(overrides: Partial = {}): { commands: unknown[]; mediaPath: string; restored: number; + quitRequested: number; }; } { const state = { @@ -28,6 +29,7 @@ function createDeps(overrides: Partial = {}): { commands: [] as unknown[], mediaPath: '', restored: 0, + quitRequested: 0, }; const metrics: MpvSubtitleRenderMetrics = { subPos: 100, @@ -102,6 +104,10 @@ function createDeps(overrides: Partial = {}): { restorePreviousSecondarySubVisibility: () => { state.restored += 1; }, + shouldQuitOnMpvShutdown: () => false, + requestAppQuit: () => { + state.quitRequested += 1; + }, setPreviousSecondarySubVisibility: () => { // intentionally not tracked in this unit test }, @@ -223,6 +229,18 @@ test('dispatchMpvProtocolMessage restores secondary visibility on shutdown', asy await dispatchMpvProtocolMessage({ event: 'shutdown' }, deps); assert.equal(state.restored, 1); + assert.equal(state.quitRequested, 0); +}); + +test('dispatchMpvProtocolMessage quits app on managed playback shutdown', async () => { + const { deps, state } = createDeps({ + shouldQuitOnMpvShutdown: () => true, + }); + + await dispatchMpvProtocolMessage({ event: 'shutdown' }, deps); + + assert.equal(state.restored, 1); + assert.equal(state.quitRequested, 1); }); test('dispatchMpvProtocolMessage pauses on sub-end when pendingPauseAtSubEnd is set', async () => { diff --git a/src/core/services/mpv-protocol.ts b/src/core/services/mpv-protocol.ts index 6a8ac979..b79dbbd9 100644 --- a/src/core/services/mpv-protocol.ts +++ b/src/core/services/mpv-protocol.ts @@ -91,6 +91,8 @@ export interface MpvProtocolHandleMessageDeps { ) => void; sendCommand: (payload: { command: unknown[]; request_id?: number }) => boolean; restorePreviousSecondarySubVisibility: () => void; + shouldQuitOnMpvShutdown: () => boolean; + requestAppQuit: () => void; } type SubtitleTrackCandidate = { @@ -360,6 +362,9 @@ export async function dispatchMpvProtocolMessage( } } else if (msg.event === 'shutdown') { deps.restorePreviousSecondarySubVisibility(); + if (deps.shouldQuitOnMpvShutdown()) { + deps.requestAppQuit(); + } } else if (msg.request_id) { if (deps.resolvePendingRequest(msg.request_id, msg)) { return; diff --git a/src/core/services/mpv.ts b/src/core/services/mpv.ts index 54a76677..3d55e198 100644 --- a/src/core/services/mpv.ts +++ b/src/core/services/mpv.ts @@ -105,6 +105,8 @@ export interface MpvIpcClientProtocolDeps { isVisibleOverlayVisible: () => boolean; getReconnectTimer: () => ReturnType | null; setReconnectTimer: (timer: ReturnType | null) => void; + shouldQuitOnMpvShutdown?: () => boolean; + requestAppQuit?: () => void; } export interface MpvIpcClientDeps extends MpvIpcClientProtocolDeps {} @@ -399,6 +401,8 @@ export class MpvIpcClient implements MpvClient { restorePreviousSecondarySubVisibility: () => { this.restorePreviousSecondarySubVisibility(); }, + shouldQuitOnMpvShutdown: () => this.deps.shouldQuitOnMpvShutdown?.() ?? false, + requestAppQuit: () => this.deps.requestAppQuit?.(), }; } diff --git a/src/core/services/startup-bootstrap.test.ts b/src/core/services/startup-bootstrap.test.ts index 639aa338..ccf88848 100644 --- a/src/core/services/startup-bootstrap.test.ts +++ b/src/core/services/startup-bootstrap.test.ts @@ -6,6 +6,7 @@ import { CliArgs } from '../../cli/args'; function makeArgs(overrides: Partial = {}): CliArgs { return { background: false, + managedPlayback: false, start: false, launchMpv: false, launchMpvTargets: [], diff --git a/src/core/services/tokenizer.test.ts b/src/core/services/tokenizer.test.ts index 74b52020..8a42f858 100644 --- a/src/core/services/tokenizer.test.ts +++ b/src/core/services/tokenizer.test.ts @@ -4091,8 +4091,7 @@ test('tokenizeSubtitle clears annotations for ことに while preserving lexical text === '違う' ? 900 : text === '事' ? 81 : text === '気付く' ? 1500 : null, getJlptLevel: (text) => text === '違う' ? 'N4' : text === '事' ? 'N4' : text === '気付く' ? 'N3' : null, - isKnownWord: (text) => - ['さっき', 'の', '俺', 'と', '気付く', 'かい', '？'].includes(text), + isKnownWord: (text) => ['さっき', 'の', '俺', 'と', '気付く', 'かい', '？'].includes(text), getMinSentenceWordsForNPlusOne: () => 1, tokenizeWithMecab: async () => [ { diff --git a/src/main.ts b/src/main.ts index 582c5c4a..29089c19 100644 --- a/src/main.ts +++ b/src/main.ts @@ -3823,6 +3823,8 @@ const { setReconnectTimer: (timer: ReturnType | null) => { appState.reconnectTimer = timer; }, + shouldQuitOnMpvShutdown: () => appState.initialArgs?.managedPlayback === true, + requestAppQuit: () => requestAppQuit(), }, updateMpvSubtitleRenderMetricsMainDeps: { getCurrentMetrics: () => appState.mpvSubtitleRenderMetrics, diff --git a/src/main/character-dictionary-runtime.ts b/src/main/character-dictionary-runtime.ts index 3d663ca3..74cee0bb 100644 --- a/src/main/character-dictionary-runtime.ts +++ b/src/main/character-dictionary-runtime.ts @@ -175,7 +175,9 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar }; }; - const resolveGuessInput = (targetPath?: string): { mediaPath: string | null; mediaTitle: string | null } => { + const resolveGuessInput = ( + targetPath?: string, + ): { mediaPath: string | null; mediaTitle: string | null } => { const dictionaryTarget = targetPath?.trim() || ''; return dictionaryTarget.length > 0 ? resolveDictionaryGuessInputs(dictionaryTarget) @@ -324,13 +326,13 @@ export function createCharacterDictionaryRuntimeService(deps: CharacterDictionar `[dictionary] stored snapshot for AniList ${mediaId}: ${snapshot.entryCount} terms`, ); - return { - mediaId: snapshot.mediaId, - mediaTitle: snapshot.mediaTitle, - entryCount: snapshot.entryCount, - fromCache: false, - updatedAt: snapshot.updatedAt, - }; + return { + mediaId: snapshot.mediaId, + mediaTitle: snapshot.mediaTitle, + entryCount: snapshot.entryCount, + fromCache: false, + updatedAt: snapshot.updatedAt, + }; }; return { diff --git a/src/main/character-dictionary-runtime/fetch.test.ts b/src/main/character-dictionary-runtime/fetch.test.ts index 3bb1fab4..09bb7718 100644 --- a/src/main/character-dictionary-runtime/fetch.test.ts +++ b/src/main/character-dictionary-runtime/fetch.test.ts @@ -4,9 +4,10 @@ import test from 'node:test'; import { searchAniListMediaCandidates } from './fetch'; test('searchAniListMediaCandidates trims fallback candidate titles', async () => { - const previousFetch = globalThis.fetch; + const previousFetchDescriptor = Object.getOwnPropertyDescriptor(globalThis, 'fetch'); Object.defineProperty(globalThis, 'fetch', { configurable: true, + writable: true, value: async () => new Response( JSON.stringify({ @@ -24,6 +25,10 @@ test('searchAniListMediaCandidates trims fallback candidate titles', async () => assert.equal(candidates[0]?.title, 'Re:ZERO'); } finally { - Object.defineProperty(globalThis, 'fetch', { configurable: true, value: previousFetch }); + if (previousFetchDescriptor) { + Object.defineProperty(globalThis, 'fetch', previousFetchDescriptor); + } else { + Reflect.deleteProperty(globalThis, 'fetch'); + } } }); diff --git a/src/main/character-dictionary-runtime/manual-selection.ts b/src/main/character-dictionary-runtime/manual-selection.ts index 63345f6d..c2d29587 100644 --- a/src/main/character-dictionary-runtime/manual-selection.ts +++ b/src/main/character-dictionary-runtime/manual-selection.ts @@ -98,11 +98,7 @@ export function buildCharacterDictionarySeriesKey(input: { } export function createCharacterDictionaryManualSelectionStore(deps: { userDataPath: string }) { - const filePath = path.join( - deps.userDataPath, - 'character-dictionaries', - 'anilist-overrides.json', - ); + const filePath = path.join(deps.userDataPath, 'character-dictionaries', 'anilist-overrides.json'); return { getOverride: async (seriesKey: string): Promise => { diff --git a/src/main/runtime/character-dictionary-auto-sync.test.ts b/src/main/runtime/character-dictionary-auto-sync.test.ts index 529eb0b1..e6722568 100644 --- a/src/main/runtime/character-dictionary-auto-sync.test.ts +++ b/src/main/runtime/character-dictionary-auto-sync.test.ts @@ -467,10 +467,7 @@ test('auto sync removes stale manual-selection media ids when applying corrected path.join(dictionariesDir, 'auto-sync-state.json'), JSON.stringify( { - activeMediaIds: [ - '10607 - Rerere no Tensai Bakabon', - '130298 - The Eminence in Shadow', - ], + activeMediaIds: ['10607 - Rerere no Tensai Bakabon', '130298 - The Eminence in Shadow'], mergedRevision: 'old', mergedDictionaryTitle: 'SubMiner Character Dictionary', }, diff --git a/src/main/runtime/first-run-setup-service.test.ts b/src/main/runtime/first-run-setup-service.test.ts index 204225de..0edc8b9a 100644 --- a/src/main/runtime/first-run-setup-service.test.ts +++ b/src/main/runtime/first-run-setup-service.test.ts @@ -20,6 +20,7 @@ function withTempDir(fn: (dir: string) => Promise | void): Promise | function makeArgs(overrides: Partial = {}): CliArgs { return { background: false, + managedPlayback: false, start: false, launchMpv: false, launchMpvTargets: [], diff --git a/src/main/runtime/mpv-client-runtime-service-main-deps.ts b/src/main/runtime/mpv-client-runtime-service-main-deps.ts index c3c4fb20..b6169ae4 100644 --- a/src/main/runtime/mpv-client-runtime-service-main-deps.ts +++ b/src/main/runtime/mpv-client-runtime-service-main-deps.ts @@ -11,6 +11,8 @@ export function createBuildMpvClientRuntimeServiceFactoryDepsHandler< isVisibleOverlayVisible: () => boolean; getReconnectTimer: () => ReturnType | null; setReconnectTimer: (timer: ReturnType | null) => void; + shouldQuitOnMpvShutdown?: () => boolean; + requestAppQuit?: () => void; bindEventHandlers: (client: TClient) => void; }) { return () => ({ @@ -24,6 +26,8 @@ export function createBuildMpvClientRuntimeServiceFactoryDepsHandler< getReconnectTimer: () => deps.getReconnectTimer(), setReconnectTimer: (timer: ReturnType | null) => deps.setReconnectTimer(timer), + shouldQuitOnMpvShutdown: () => deps.shouldQuitOnMpvShutdown?.() ?? false, + requestAppQuit: () => deps.requestAppQuit?.(), }, bindEventHandlers: (client: TClient) => deps.bindEventHandlers(client), }); diff --git a/src/main/runtime/mpv-client-runtime-service.ts b/src/main/runtime/mpv-client-runtime-service.ts index 325d665d..2fd0290b 100644 --- a/src/main/runtime/mpv-client-runtime-service.ts +++ b/src/main/runtime/mpv-client-runtime-service.ts @@ -7,6 +7,8 @@ export type MpvClientRuntimeServiceOptions = { isVisibleOverlayVisible: () => boolean; getReconnectTimer: () => ReturnType | null; setReconnectTimer: (timer: ReturnType | null) => void; + shouldQuitOnMpvShutdown?: () => boolean; + requestAppQuit?: () => void; }; type MpvClientLike = { diff --git a/src/renderer/handlers/keyboard.ts b/src/renderer/handlers/keyboard.ts index 66833f42..4558a0d7 100644 --- a/src/renderer/handlers/keyboard.ts +++ b/src/renderer/handlers/keyboard.ts @@ -362,14 +362,7 @@ export function createKeyboardHandlers( } function isPrimarySubtitleVisibilityToggle(e: KeyboardEvent): boolean { - return ( - e.code === 'KeyV' && - !e.ctrlKey && - !e.altKey && - !e.metaKey && - !e.shiftKey && - !e.repeat - ); + return e.code === 'KeyV' && !e.ctrlKey && !e.altKey && !e.metaKey && !e.shiftKey && !e.repeat; } function togglePrimarySubtitleBarVisibility(): void { diff --git a/src/renderer/modals/character-dictionary.test.ts b/src/renderer/modals/character-dictionary.test.ts index ac8afbd4..3d9bd678 100644 --- a/src/renderer/modals/character-dictionary.test.ts +++ b/src/renderer/modals/character-dictionary.test.ts @@ -153,3 +153,63 @@ test('character dictionary modal loads candidates and applies selected override' Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument }); } }); + +test('character dictionary modal shows refresh errors without rejecting open', async () => { + const previousWindow = globalThis.window; + const overlay = createNodeStub(); + const modalNode = createNodeStub(true); + const status = createNodeStub(); + const state = createRendererState(); + + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + getCharacterDictionarySelection: async () => { + throw new Error('candidate lookup failed'); + }, + setCharacterDictionarySelection: async () => ({ + ok: false, + seriesKey: 'test', + selected: { id: 0, title: '', episodes: null }, + staleMediaIds: [], + }), + notifyOverlayModalClosed: () => {}, + } satisfies Pick< + ElectronAPI, + | 'getCharacterDictionarySelection' + | 'setCharacterDictionarySelection' + | 'notifyOverlayModalClosed' + >, + }, + }); + + try { + const modal = createCharacterDictionaryModal( + { + state, + dom: { + overlay, + characterDictionaryModal: modalNode, + characterDictionaryClose: createNodeStub(), + characterDictionarySummary: createNodeStub(), + characterDictionaryCurrent: createNodeStub(), + characterDictionaryCandidates: createNodeStub(), + characterDictionaryStatus: status, + }, + } as never, + { + modalStateReader: { isAnyModalOpen: () => false }, + syncSettingsModalSubtitleSuppression: () => {}, + }, + ); + + await modal.openCharacterDictionaryModal(); + + assert.equal(state.characterDictionaryModalOpen, true); + assert.equal(status.textContent, 'candidate lookup failed'); + assert.equal(status.classList.contains('error'), true); + } finally { + Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow }); + } +}); diff --git a/src/renderer/modals/character-dictionary.ts b/src/renderer/modals/character-dictionary.ts index 0837491f..b39dae48 100644 --- a/src/renderer/modals/character-dictionary.ts +++ b/src/renderer/modals/character-dictionary.ts @@ -162,7 +162,11 @@ export function createCharacterDictionaryModal( } else { setStatus('Refreshing AniList candidates...'); } - await refreshSelection(); + try { + await refreshSelection(); + } catch (error) { + setStatus(error instanceof Error ? error.message : String(error), true); + } } function closeCharacterDictionaryModal(): void { diff --git a/src/renderer/subtitle-render.test.ts b/src/renderer/subtitle-render.test.ts index c12c8ff2..55171518 100644 --- a/src/renderer/subtitle-render.test.ts +++ b/src/renderer/subtitle-render.test.ts @@ -658,6 +658,36 @@ test('sanitizeSubtitleHoverTokenColor keeps non-black color values', () => { assert.equal(sanitizeSubtitleHoverTokenColor(undefined), '#f4dbd6'); }); +test('applySubtitleStyle keeps transparent hover token background', () => { + const restoreDocument = installFakeDocument(); + try { + const subtitleRoot = new FakeElement('div'); + const subtitleContainer = new FakeElement('div'); + const secondarySubRoot = new FakeElement('div'); + const secondarySubContainer = new FakeElement('div'); + const ctx = { + state: createRendererState(), + dom: { + subtitleRoot, + subtitleContainer, + secondarySubRoot, + secondarySubContainer, + }, + } as never; + + const renderer = createSubtitleRenderer(ctx); + renderer.applySubtitleStyle({ + hoverTokenBackgroundColor: 'transparent', + } as never); + + const rootStyleValues = (subtitleRoot.style as unknown as { values?: Map }) + .values; + assert.equal(rootStyleValues?.get('--subtitle-hover-token-background-color'), 'transparent'); + } finally { + restoreDocument(); + } +}); + test('alignTokensToSourceText preserves newline separators between adjacent token surfaces', () => { const tokens = [ createToken({ surface: 'キリキリと', reading: 'きりきりと', headword: 'キリキリと' }),