From 06d0e3ed18a307c12b3b36bc85fbc8db4ce5c6be Mon Sep 17 00:00:00 2001 From: sudacode Date: Mon, 18 May 2026 16:03:16 -0700 Subject: [PATCH] fix: disable macOS mpv menu shortcuts, buffer latest subtitle IPC state - Pass --macos-menu-shortcuts=no on Darwin so SubMiner bindings reach mpv - Replace queued IPC listener with latest-value variant for subtitle channels - Skip JSONC line/block comments in duplicate-key offset helpers - Preserve configured Anki note model name in selectPreferredNoteFieldModelName - Guard known-words deck rename against collision; add chooseKnownWordsDeckRenameValue - Apply asCssColor on hover token CSS compat reads --- changes/fix-macos-mpv-shortcuts.md | 4 ++ docs-site/shortcuts.md | 24 ++++++----- launcher/mpv.test.ts | 9 ++++ launcher/mpv.ts | 4 ++ src/config/resolve/subtitle-domains.ts | 6 ++- src/config/resolve/subtitle-style.test.ts | 16 +++++++ src/config/settings/jsonc-edit.test.ts | 31 ++++++++++++++ src/config/settings/jsonc-edit.ts | 47 +++++++++++++++++++-- src/preload-settings.test.ts | 4 +- src/preload.ts | 41 +++++++++++++++--- src/settings/settings-anki-controls.test.ts | 26 ++++++++---- src/settings/settings-anki-controls.ts | 31 +++++++++++++- 12 files changed, 208 insertions(+), 35 deletions(-) create mode 100644 changes/fix-macos-mpv-shortcuts.md diff --git a/changes/fix-macos-mpv-shortcuts.md b/changes/fix-macos-mpv-shortcuts.md new file mode 100644 index 00000000..47fac5df --- /dev/null +++ b/changes/fix-macos-mpv-shortcuts.md @@ -0,0 +1,4 @@ +type: fixed +area: shortcuts + +- Disabled native mpv menu shortcuts during managed macOS playback so configured SubMiner shortcuts also work while mpv has focus. diff --git a/docs-site/shortcuts.md b/docs-site/shortcuts.md index 75a0bebb..00eaf2d5 100644 --- a/docs-site/shortcuts.md +++ b/docs-site/shortcuts.md @@ -61,6 +61,8 @@ These control playback and subtitle display. They require overlay window focus. These keybindings can be overridden or disabled via the `keybindings` config array. The playlist browser opens a split overlay modal with sibling video files on the left and the live mpv playlist on the right. +On macOS managed playback, SubMiner disables mpv's menu-bar shortcuts so configured SubMiner shortcuts like `Cmd+Shift+O` reach the mpv plugin instead of opening native mpv menu actions. + Mouse-hover playback behavior is configured separately from shortcuts: `subtitleStyle.autoPauseVideoOnHover` defaults to `true` (pause on subtitle hover, resume on leave). ## Subtitle & Feature Shortcuts @@ -68,7 +70,7 @@ Mouse-hover playback behavior is configured separately from shortcuts: `subtitle | Shortcut | Action | Config key | | ------------------ | -------------------------------------------------------- | ----------------------------------- | | `Ctrl/Cmd+Shift+V` | Cycle secondary subtitle mode (hidden → visible → hover) | `shortcuts.toggleSecondarySub` | -| `Ctrl/Cmd+Alt+A` | Open character dictionary AniList selector | `shortcuts.openCharacterDictionary` | +| `Ctrl/Cmd+Alt+A` | Open character dictionary AniList selector | `shortcuts.openCharacterDictionary` | | `Ctrl/Cmd+Shift+O` | Open runtime options palette | `shortcuts.openRuntimeOptions` | | `Ctrl/Cmd+/` | Open session help modal | `shortcuts.openSessionHelp` | | `Ctrl+Shift+J` | Open Jimaku subtitle search modal | `shortcuts.openJimaku` | @@ -96,17 +98,17 @@ Controller input only drives the overlay while keyboard-only mode is enabled. Th When the mpv plugin is installed, all commands use a `y` chord prefix — press `y`, then the second key within 1 second. -| Chord | Action | -| ----- | ------------------------ | -| `y-y` | Open SubMiner menu (OSD) | -| `y-s` | Start overlay | -| `y-S` | Stop overlay | -| `y-t` | Toggle visible overlay | +| Chord | Action | +| ----- | -------------------------------------- | +| `y-y` | Open SubMiner menu (OSD) | +| `y-s` | Start overlay | +| `y-S` | Stop overlay | +| `y-t` | Toggle visible overlay | | `v` | Toggle primary subtitle bar visibility | -| `y-o` | Open Yomitan settings | -| `y-r` | Restart overlay | -| `y-c` | Check overlay status | -| `y-h` | Open session help | +| `y-o` | Open Yomitan settings | +| `y-r` | Restart overlay | +| `y-c` | Check overlay status | +| `y-h` | Open session help | The bare `v` plugin binding intentionally overrides mpv's native primary subtitle visibility toggle so the SubMiner primary subtitle bar is hidden or restored instead. diff --git a/launcher/mpv.test.ts b/launcher/mpv.test.ts index 1ee1892d..615ee638 100644 --- a/launcher/mpv.test.ts +++ b/launcher/mpv.test.ts @@ -294,6 +294,15 @@ test('buildConfiguredMpvDefaultArgs appends maximized launch mode to configured }); }); +test('buildConfiguredMpvDefaultArgs disables macOS menu shortcuts so SubMiner bindings reach mpv', () => { + withPlatform('darwin', () => { + assert.equal( + buildConfiguredMpvDefaultArgs(makeArgs()).includes('--macos-menu-shortcuts=no'), + true, + ); + }); +}); + test('resolveLauncherRuntimePluginPath finds bundled plugin from explicit environment path', () => { const pluginDir = '/opt/SubMiner/plugin/subminer'; assert.equal( diff --git a/launcher/mpv.ts b/launcher/mpv.ts index b146d59b..f68a0fb1 100644 --- a/launcher/mpv.ts +++ b/launcher/mpv.ts @@ -1248,6 +1248,10 @@ export function buildConfiguredMpvDefaultArgs( const mpvArgs: string[] = []; if (args.profile) mpvArgs.push(`--profile=${args.profile}`); mpvArgs.push(...DEFAULT_MPV_SUBMINER_ARGS); + if (process.platform === 'darwin') { + // macOS menu accelerators do not reach mpv script bindings unless disabled. + mpvArgs.push('--macos-menu-shortcuts=no'); + } mpvArgs.push(...buildMpvBackendArgs(args, baseEnv)); mpvArgs.push(...buildMpvLaunchModeArgs(args.launchMode)); return mpvArgs; diff --git a/src/config/resolve/subtitle-domains.ts b/src/config/resolve/subtitle-domains.ts index 1ca8eda1..16213734 100644 --- a/src/config/resolve/subtitle-domains.ts +++ b/src/config/resolve/subtitle-domains.ts @@ -31,12 +31,14 @@ const SUBTITLE_HOVER_TOKEN_BACKGROUND_CSS_PROPERTY = '--subtitle-hover-token-bac function applySubtitleHoverTokenCssCompatibility( subtitleStyle: ResolvedConfig['subtitleStyle'], ): void { - const hoverTokenColor = subtitleStyle.css[SUBTITLE_HOVER_TOKEN_COLOR_CSS_PROPERTY]; + const hoverTokenColor = asCssColor(subtitleStyle.css[SUBTITLE_HOVER_TOKEN_COLOR_CSS_PROPERTY]); if (hoverTokenColor !== undefined) { subtitleStyle.hoverTokenColor = hoverTokenColor; } - const hoverTokenBackgroundColor = subtitleStyle.css[SUBTITLE_HOVER_TOKEN_BACKGROUND_CSS_PROPERTY]; + const hoverTokenBackgroundColor = asCssColor( + subtitleStyle.css[SUBTITLE_HOVER_TOKEN_BACKGROUND_CSS_PROPERTY], + ); if (hoverTokenBackgroundColor !== undefined) { subtitleStyle.hoverTokenBackgroundColor = hoverTokenBackgroundColor; } diff --git a/src/config/resolve/subtitle-style.test.ts b/src/config/resolve/subtitle-style.test.ts index 8561c5cf..ee1583fa 100644 --- a/src/config/resolve/subtitle-style.test.ts +++ b/src/config/resolve/subtitle-style.test.ts @@ -74,6 +74,22 @@ test('subtitleStyle css declarations accept string declaration maps and warn on assert.ok(invalid.warnings.some((warning) => warning.path === 'subtitleStyle.secondary.css')); }); +test('subtitleStyle hover css compatibility ignores invalid color declarations', () => { + const { context } = createResolveContext({ + subtitleStyle: { + css: { + '--subtitle-hover-token-color': 'purple', + '--subtitle-hover-token-background-color': '#363a4fd6', + }, + }, + }); + + applySubtitleDomainConfig(context); + + assert.equal(context.resolved.subtitleStyle.hoverTokenColor, '#f4dbd6'); + assert.equal(context.resolved.subtitleStyle.hoverTokenBackgroundColor, '#363a4fd6'); +}); + test('subtitleStyle autoPauseVideoOnHover falls back on invalid value', () => { const { context, warnings } = createResolveContext({ subtitleStyle: { diff --git a/src/config/settings/jsonc-edit.test.ts b/src/config/settings/jsonc-edit.test.ts index 12d8ffaa..8c23bd0e 100644 --- a/src/config/settings/jsonc-edit.test.ts +++ b/src/config/settings/jsonc-edit.test.ts @@ -65,6 +65,37 @@ test('applyConfigSettingsPatchToContent updates effective duplicate object path' assert.equal(parsed.ankiConnect.nPlusOne.minSentenceWords, 3); }); +test('applyConfigSettingsPatchToContent removes duplicate properties across JSONC trivia', () => { + const input = `{ + "ankiConnect": { + "nPlusOne": { + "enabled": false + } /* old value */ , + // effective value follows + "nPlusOne": { + "minSentenceWords": 3 + } + } +}`; + + const result = applyConfigSettingsPatchToContent({ + content: input, + operations: [ + { + op: 'set', + path: 'ankiConnect.nPlusOne.enabled', + value: true, + }, + ], + previousWarnings: [], + }); + + assert.equal(result.ok, true); + const parsed = parse(result.content); + assert.equal(parsed.ankiConnect.nPlusOne.enabled, true); + assert.equal(parsed.ankiConnect.nPlusOne.minSentenceWords, 3); +}); + test('applyConfigSettingsPatchToContent reset removes explicit path', () => { const input = `{ "subtitleStyle": { diff --git a/src/config/settings/jsonc-edit.ts b/src/config/settings/jsonc-edit.ts index 08be7fd7..7a9fe5aa 100644 --- a/src/config/settings/jsonc-edit.ts +++ b/src/config/settings/jsonc-edit.ts @@ -125,16 +125,55 @@ function isWhitespace(value: string | undefined): boolean { function nextNonWhitespaceOffset(content: string, offset: number): number { let index = offset; - while (index < content.length && isWhitespace(content[index])) { - index += 1; + while (index < content.length) { + if (isWhitespace(content[index])) { + index += 1; + continue; + } + if (content[index] === '/' && content[index + 1] === '/') { + index += 2; + while (index < content.length && content[index] !== '\n') index += 1; + continue; + } + if (content[index] === '/' && content[index + 1] === '*') { + index += 2; + while ( + index + 1 < content.length && + !(content[index] === '*' && content[index + 1] === '/') + ) { + index += 1; + } + index = Math.min(content.length, index + 2); + continue; + } + break; } return index; } function previousNonWhitespaceOffset(content: string, offset: number): number { let index = offset; - while (index >= 0 && isWhitespace(content[index])) { - index -= 1; + while (index >= 0) { + if (isWhitespace(content[index])) { + index -= 1; + continue; + } + const lineStart = content.lastIndexOf('\n', index) + 1; + const linePrefix = content.slice(lineStart, index + 1); + const lineCommentStart = linePrefix.lastIndexOf('//'); + if (lineCommentStart >= 0 && /^[ \t]*$/.test(linePrefix.slice(0, lineCommentStart))) { + index = lineStart - 1; + continue; + } + if (content[index] === '/' && content[index - 1] === '*') { + index -= 2; + while (index > 0 && !(content[index - 1] === '/' && content[index] === '*')) { + index -= 1; + } + index -= 2; + continue; + } + break; } return index; } diff --git a/src/preload-settings.test.ts b/src/preload-settings.test.ts index a24aa178..4fd8b40b 100644 --- a/src/preload-settings.test.ts +++ b/src/preload-settings.test.ts @@ -23,12 +23,12 @@ test('settings preload exposes Anki lookup helpers', () => { } }); -test('overlay preload queues subtitle updates until renderer listener registration', () => { +test('overlay preload buffers only latest subtitle state until renderer listener registration', () => { const source = fs.readFileSync(path.join(process.cwd(), 'src', 'preload.ts'), 'utf8'); assert.match( source, - /const onSubtitleSetEvent =\s*createQueuedIpcListenerWithPayload\(\s*IPC_CHANNELS\.event\.subtitleSet,/, + /const onSubtitleSetEvent =\s*createLatestValueIpcListenerWithPayload\(\s*IPC_CHANNELS\.event\.subtitleSet,/, ); assert.match(source, /onSubtitle:\s*\(callback:[\s\S]+?onSubtitleSetEvent\(callback\);/); }); diff --git a/src/preload.ts b/src/preload.ts index 0d48f451..dce58d64 100644 --- a/src/preload.ts +++ b/src/preload.ts @@ -122,6 +122,37 @@ function createQueuedIpcListenerWithPayload( }; } +function createLatestValueIpcListenerWithPayload( + channel: string, + normalize: (payload: unknown) => T, +): (listener: PayloadedListener) => void { + let pending: T | undefined; + const listeners: PayloadedListener[] = []; + + const dispatch = (payload: T): void => { + if (listeners.length === 0) { + pending = payload; + return; + } + for (const listener of listeners) { + listener(payload); + } + }; + + ipcRenderer.on(channel, (_event: IpcRendererEvent, payloadArg: unknown) => { + dispatch(normalize(payloadArg)); + }); + + return (listener: PayloadedListener): void => { + listeners.push(listener); + if (pending !== undefined) { + const payload = pending; + pending = undefined; + listener(payload); + } + }; +} + const onOpenRuntimeOptionsEvent = createQueuedIpcListener(IPC_CHANNELS.event.runtimeOptionsOpen); const onOpenSessionHelpEvent = createQueuedIpcListener(IPC_CHANNELS.event.sessionHelpOpen); const onOpenCharacterDictionaryEvent = createQueuedIpcListener( @@ -161,23 +192,23 @@ const onKikuFieldGroupingRequestEvent = IPC_CHANNELS.event.kikuFieldGroupingRequest, (payload) => payload as KikuFieldGroupingRequestData, ); -const onSubtitleSetEvent = createQueuedIpcListenerWithPayload( +const onSubtitleSetEvent = createLatestValueIpcListenerWithPayload( IPC_CHANNELS.event.subtitleSet, (payload) => payload as SubtitleData, ); -const onSubtitleVisibilityEvent = createQueuedIpcListenerWithPayload( +const onSubtitleVisibilityEvent = createLatestValueIpcListenerWithPayload( IPC_CHANNELS.event.subtitleVisibility, (payload) => payload === true, ); -const onSubtitlePositionSetEvent = createQueuedIpcListenerWithPayload( +const onSubtitlePositionSetEvent = createLatestValueIpcListenerWithPayload( IPC_CHANNELS.event.subtitlePositionSet, (payload) => payload as SubtitlePosition | null, ); -const onSecondarySubtitleSetEvent = createQueuedIpcListenerWithPayload( +const onSecondarySubtitleSetEvent = createLatestValueIpcListenerWithPayload( IPC_CHANNELS.event.secondarySubtitleSet, (payload) => (typeof payload === 'string' ? payload : ''), ); -const onSecondarySubtitleModeEvent = createQueuedIpcListenerWithPayload( +const onSecondarySubtitleModeEvent = createLatestValueIpcListenerWithPayload( IPC_CHANNELS.event.secondarySubtitleMode, (payload) => payload as SecondarySubMode, ); diff --git a/src/settings/settings-anki-controls.test.ts b/src/settings/settings-anki-controls.test.ts index 765d0473..568c8f65 100644 --- a/src/settings/settings-anki-controls.test.ts +++ b/src/settings/settings-anki-controls.test.ts @@ -2,17 +2,17 @@ import test from 'node:test'; import assert from 'node:assert/strict'; import * as ankiControls from './settings-anki-controls'; -test('note field model preference ignores configured sentence-card model before Kiku fallback', () => { +test('note field model preference keeps configured sentence-card model before Kiku fallback', () => { assert.equal( ankiControls.selectPreferredNoteFieldModelName(['Lapis Morph', 'Kiku'], 'Lapis Morph'), - 'Kiku', + 'Lapis Morph', ); }); -test('note field model preference ignores configured sentence-card model case-insensitively', () => { +test('note field model preference keeps configured sentence-card model case-insensitively', () => { assert.equal( ankiControls.selectPreferredNoteFieldModelName(['Lapis Morph', 'Kiku'], 'lapis morph'), - 'Kiku', + 'Lapis Morph', ); }); @@ -29,15 +29,23 @@ test('note field model preference does not treat partial Kiku matches as Kiku', }); test('note field model preference does not treat partial Lapis matches as Lapis', () => { - assert.equal( - ankiControls.selectPreferredNoteFieldModelName(['Mining', 'Lapis Morph'], ''), - '', - ); + assert.equal(ankiControls.selectPreferredNoteFieldModelName(['Mining', 'Lapis Morph'], ''), ''); }); -test('note field model preference stays blank when no Kiku or Lapis note type exists', () => { +test('note field model preference stays blank when no current Kiku or Lapis note type exists', () => { assert.equal( ankiControls.selectPreferredNoteFieldModelName(['Basic', 'Mining'], 'Lapis Morph'), '', ); }); + +test('known word deck rename selection keeps current deck on collision', () => { + assert.equal( + ankiControls.chooseKnownWordsDeckRenameValue( + { Mining: ['Word'], Core: ['Expression'] }, + 'Core', + 'Mining', + ), + 'Core', + ); +}); diff --git a/src/settings/settings-anki-controls.ts b/src/settings/settings-anki-controls.ts index 38e4e18f..eb4635d8 100644 --- a/src/settings/settings-anki-controls.ts +++ b/src/settings/settings-anki-controls.ts @@ -55,7 +55,13 @@ export function selectPreferredNoteFieldModelName( modelNames: readonly string[], currentModelName = '', ): string { - void currentModelName; + const normalizedCurrent = currentModelName.trim().toLowerCase(); + if (normalizedCurrent) { + const current = modelNames.find((name) => name.trim().toLowerCase() === normalizedCurrent); + if (current) { + return current; + } + } const exactKiku = modelNames.find((name) => name.trim().toLowerCase() === 'kiku'); if (exactKiku) { @@ -70,6 +76,20 @@ export function selectPreferredNoteFieldModelName( return ''; } +export function chooseKnownWordsDeckRenameValue( + decks: Record, + currentDeckName: string, + nextDeckName: string, +): string { + if ( + nextDeckName !== currentDeckName && + Object.prototype.hasOwnProperty.call(decks, nextDeckName) + ) { + return currentDeckName; + } + return nextDeckName; +} + function normalizeStringArray(value: unknown): string[] { return Array.isArray(value) ? value.filter((entry): entry is string => typeof entry === 'string' && entry.length > 0) @@ -491,16 +511,23 @@ export function renderKnownWordsDecksInput( void loadAnkiDeckFieldNames(deckName, draftUrl); } const row = createElement('div', 'deck-field-row'); + const usedDeckNames = new Set(Object.keys(currentDecks)); const deckSelect = createElement('select', 'config-input') as HTMLSelectElement; for (const candidateDeck of uniqueSorted([...deckNames, deckName])) { + if (candidateDeck !== deckName && usedDeckNames.has(candidateDeck)) continue; addOption(deckSelect, candidateDeck); } deckSelect.value = deckName; deckSelect.addEventListener('change', () => { const nextDecks = normalizeKnownWordsDecks(context.valueForField(field)); + const nextDeckName = chooseKnownWordsDeckRenameValue(nextDecks, deckName, deckSelect.value); + if (nextDeckName !== deckSelect.value) { + deckSelect.value = nextDeckName; + return; + } const fields = nextDecks[deckName] ?? []; delete nextDecks[deckName]; - nextDecks[deckSelect.value] = fields; + nextDecks[nextDeckName] = fields; setKnownWordsDecks(context, field.configPath, nextDecks); requestRender(); });