mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-25 00:11:26 -07:00
fix: address PR #31 latest review follow-ups
This commit is contained in:
@@ -5,7 +5,9 @@ area: launcher
|
|||||||
- Kept startup playback gating tied only to primary subtitle load and tokenization readiness; secondary failures no longer block resume.
|
- Kept startup playback gating tied only to primary subtitle load and tokenization readiness; secondary failures no longer block resume.
|
||||||
- Added a default `Ctrl+Alt+C` keybinding that opens the YouTube subtitle picker manually during active YouTube playback.
|
- Added a default `Ctrl+Alt+C` keybinding that opens the YouTube subtitle picker manually during active YouTube playback.
|
||||||
- Routed primary YouTube subtitle auto-load failures through the configured notification output path so failed startup still resumes cleanly.
|
- Routed primary YouTube subtitle auto-load failures through the configured notification output path so failed startup still resumes cleanly.
|
||||||
|
- Suppressed false primary-subtitle failure notifications while app-owned YouTube subtitle probing and downloads are still in flight.
|
||||||
- Kept `youtubeSubgen.whisper*` settings as legacy compatibility configuration only; normal startup now resolves tracks through YouTube source download/selection.
|
- Kept `youtubeSubgen.whisper*` settings as legacy compatibility configuration only; normal startup now resolves tracks through YouTube source download/selection.
|
||||||
- Fixed startup-launched YouTube playback so primary subtitle overlay updates continue after auto-load completes.
|
- Fixed startup-launched YouTube playback so primary subtitle overlay updates continue after auto-load completes.
|
||||||
- Fixed auto-loaded YouTube primary subtitles so parsed cues appear in the subtitle sidebar without needing a manual picker retry.
|
- Fixed auto-loaded YouTube primary subtitles so parsed cues appear in the subtitle sidebar without needing a manual picker retry.
|
||||||
|
- Kept the YouTube picker controls disabled when no subtitle tracks are available, even after a failed continue attempt.
|
||||||
- Prefer existing authoritative YouTube subtitle tracks when available, fall back to downloaded tracks only for missing sides, and keep native mpv secondary subtitle rendering hidden so the overlay remains the sole secondary display.
|
- Prefer existing authoritative YouTube subtitle tracks when available, fall back to downloaded tracks only for missing sides, and keep native mpv secondary subtitle rendering hidden so the overlay remains the sole secondary display.
|
||||||
|
|||||||
@@ -85,6 +85,12 @@ function createContext(): LauncherCommandContext {
|
|||||||
test('youtube playback launches overlay with app-owned youtube flow args', async () => {
|
test('youtube playback launches overlay with app-owned youtube flow args', async () => {
|
||||||
const calls: string[] = [];
|
const calls: string[] = [];
|
||||||
const context = createContext();
|
const context = createContext();
|
||||||
|
context.pluginRuntimeConfig = {
|
||||||
|
...context.pluginRuntimeConfig,
|
||||||
|
autoStart: false,
|
||||||
|
autoStartVisibleOverlay: false,
|
||||||
|
autoStartPauseUntilReady: false,
|
||||||
|
};
|
||||||
let receivedStartMpvOptions: Record<string, unknown> | null = null;
|
let receivedStartMpvOptions: Record<string, unknown> | null = null;
|
||||||
|
|
||||||
await runPlaybackCommandWithDeps(context, {
|
await runPlaybackCommandWithDeps(context, {
|
||||||
|
|||||||
@@ -246,6 +246,24 @@ test('handleCliCommand defaults youtube mode to download when omitted', () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('handleCliCommand reports youtube playback flow failures to logs and OSD', async () => {
|
||||||
|
const { deps, calls, osd } = createDeps({
|
||||||
|
runYoutubePlaybackFlow: async () => {
|
||||||
|
throw new Error('yt failed');
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
handleCliCommand(
|
||||||
|
makeArgs({ youtubePlay: 'https://youtube.com/watch?v=abc', youtubeMode: 'download' }),
|
||||||
|
'initial',
|
||||||
|
deps,
|
||||||
|
);
|
||||||
|
await new Promise((resolve) => setImmediate(resolve));
|
||||||
|
|
||||||
|
assert.ok(calls.some((value) => value.startsWith('error:runYoutubePlaybackFlow failed:')));
|
||||||
|
assert.ok(osd.includes('YouTube playback failed: yt failed'));
|
||||||
|
});
|
||||||
|
|
||||||
test('handleCliCommand reconnects MPV for second-instance --start when overlay runtime is already initialized', () => {
|
test('handleCliCommand reconnects MPV for second-instance --start when overlay runtime is already initialized', () => {
|
||||||
const { deps, calls } = createDeps({
|
const { deps, calls } = createDeps({
|
||||||
isOverlayRuntimeInitialized: () => true,
|
isOverlayRuntimeInitialized: () => true,
|
||||||
|
|||||||
@@ -404,11 +404,18 @@ export function handleCliCommand(
|
|||||||
deps.openJellyfinSetup();
|
deps.openJellyfinSetup();
|
||||||
deps.log('Opened Jellyfin setup flow.');
|
deps.log('Opened Jellyfin setup flow.');
|
||||||
} else if (args.youtubePlay) {
|
} else if (args.youtubePlay) {
|
||||||
void deps.runYoutubePlaybackFlow({
|
const youtubeUrl = args.youtubePlay;
|
||||||
url: args.youtubePlay,
|
runAsyncWithOsd(
|
||||||
mode: args.youtubeMode ?? 'download',
|
() =>
|
||||||
source,
|
deps.runYoutubePlaybackFlow({
|
||||||
});
|
url: youtubeUrl,
|
||||||
|
mode: args.youtubeMode ?? 'download',
|
||||||
|
source,
|
||||||
|
}),
|
||||||
|
deps,
|
||||||
|
'runYoutubePlaybackFlow',
|
||||||
|
'YouTube playback failed',
|
||||||
|
);
|
||||||
} else if (args.dictionary) {
|
} else if (args.dictionary) {
|
||||||
const shouldStopAfterRun = source === 'initial' && !deps.hasMainWindow();
|
const shouldStopAfterRun = source === 'initial' && !deps.hasMainWindow();
|
||||||
deps.log('Generating character dictionary for current anime...');
|
deps.log('Generating character dictionary for current anime...');
|
||||||
|
|||||||
30
src/main.ts
30
src/main.ts
@@ -946,6 +946,7 @@ async function runYoutubePlaybackFlowMain(request: {
|
|||||||
mode: NonNullable<CliArgs['youtubeMode']>;
|
mode: NonNullable<CliArgs['youtubeMode']>;
|
||||||
source: CliCommandSource;
|
source: CliCommandSource;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
|
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(true);
|
||||||
if (process.platform === 'win32' && !appState.mpvClient?.connected) {
|
if (process.platform === 'win32' && !appState.mpvClient?.connected) {
|
||||||
const launchResult = launchWindowsMpv(
|
const launchResult = launchWindowsMpv(
|
||||||
[request.url],
|
[request.url],
|
||||||
@@ -969,11 +970,15 @@ async function runYoutubePlaybackFlowMain(request: {
|
|||||||
appState.mpvClient?.connect();
|
appState.mpvClient?.connect();
|
||||||
}
|
}
|
||||||
|
|
||||||
await youtubeFlowRuntime.runYoutubePlaybackFlow({
|
try {
|
||||||
url: request.url,
|
await youtubeFlowRuntime.runYoutubePlaybackFlow({
|
||||||
mode: request.mode,
|
url: request.url,
|
||||||
});
|
mode: request.mode,
|
||||||
logger.info(`YouTube playback flow completed from ${request.source}.`);
|
});
|
||||||
|
logger.info(`YouTube playback flow completed from ${request.source}.`);
|
||||||
|
} finally {
|
||||||
|
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(false);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
let firstRunSetupMessage: string | null = null;
|
let firstRunSetupMessage: string | null = null;
|
||||||
@@ -3965,21 +3970,6 @@ function destroyTray(): void {
|
|||||||
|
|
||||||
function initializeOverlayRuntime(): void {
|
function initializeOverlayRuntime(): void {
|
||||||
initializeOverlayRuntimeHandler();
|
initializeOverlayRuntimeHandler();
|
||||||
initializeOverlayAnkiIntegrationCore({
|
|
||||||
getResolvedConfig: () => getResolvedConfig(),
|
|
||||||
getSubtitleTimingTracker: () => appState.subtitleTimingTracker,
|
|
||||||
getMpvClient: () => appState.mpvClient,
|
|
||||||
getRuntimeOptionsManager: () => appState.runtimeOptionsManager,
|
|
||||||
getAnkiIntegration: () => appState.ankiIntegration,
|
|
||||||
setAnkiIntegration: (integration) => {
|
|
||||||
appState.ankiIntegration = integration as AnkiIntegration | null;
|
|
||||||
},
|
|
||||||
showDesktopNotification,
|
|
||||||
createFieldGroupingCallback: () => createFieldGroupingCallback(),
|
|
||||||
getKnownWordCacheStatePath: () => path.join(USER_DATA_PATH, 'known-words-cache.json'),
|
|
||||||
shouldStartAnkiIntegration: () =>
|
|
||||||
!(appState.initialArgs && isHeadlessInitialCommand(appState.initialArgs)),
|
|
||||||
});
|
|
||||||
appState.ankiIntegration?.setRecordCardsMinedCallback(recordTrackedCardsMined);
|
appState.ankiIntegration?.setRecordCardsMinedCallback(recordTrackedCardsMined);
|
||||||
syncOverlayMpvSubtitleSuppression();
|
syncOverlayMpvSubtitleSuppression();
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -147,3 +147,29 @@ test('notifier ignores empty and null media paths and waits for track list befor
|
|||||||
timers.runAll();
|
timers.runAll();
|
||||||
assert.deepEqual(notifications, []);
|
assert.deepEqual(notifications, []);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('notifier suppresses timer while app-owned youtube flow is still settling', () => {
|
||||||
|
const notifications: string[] = [];
|
||||||
|
const timers = createTimerHarness();
|
||||||
|
const runtime = createYoutubePrimarySubtitleNotificationRuntime({
|
||||||
|
getPrimarySubtitleLanguages: () => ['ja'],
|
||||||
|
notifyFailure: (message) => {
|
||||||
|
notifications.push(message);
|
||||||
|
},
|
||||||
|
schedule: (fn) => timers.schedule(fn),
|
||||||
|
clearSchedule: (timer) => timers.clear(timer),
|
||||||
|
});
|
||||||
|
|
||||||
|
runtime.setAppOwnedFlowInFlight(true);
|
||||||
|
runtime.handleMediaPathChange('https://www.youtube.com/watch?v=abc');
|
||||||
|
|
||||||
|
assert.equal(timers.size(), 0);
|
||||||
|
|
||||||
|
runtime.setAppOwnedFlowInFlight(false);
|
||||||
|
assert.equal(timers.size(), 1);
|
||||||
|
|
||||||
|
timers.runAll();
|
||||||
|
assert.deepEqual(notifications, [
|
||||||
|
'Primary subtitle failed to download or load. Try again from the subtitle modal.',
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
|||||||
@@ -102,6 +102,7 @@ export function createYoutubePrimarySubtitleNotificationRuntime(deps: {
|
|||||||
let currentTrackList: unknown[] | null = null;
|
let currentTrackList: unknown[] | null = null;
|
||||||
let pendingTimer: YoutubePrimarySubtitleNotificationTimer | null = null;
|
let pendingTimer: YoutubePrimarySubtitleNotificationTimer | null = null;
|
||||||
let lastReportedMediaPath: string | null = null;
|
let lastReportedMediaPath: string | null = null;
|
||||||
|
let appOwnedFlowInFlight = false;
|
||||||
|
|
||||||
const clearPendingTimer = (): void => {
|
const clearPendingTimer = (): void => {
|
||||||
deps.clearSchedule(pendingTimer);
|
deps.clearSchedule(pendingTimer);
|
||||||
@@ -129,6 +130,9 @@ export function createYoutubePrimarySubtitleNotificationRuntime(deps: {
|
|||||||
|
|
||||||
const schedulePendingCheck = (): void => {
|
const schedulePendingCheck = (): void => {
|
||||||
clearPendingTimer();
|
clearPendingTimer();
|
||||||
|
if (appOwnedFlowInFlight) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
const mediaPath = currentMediaPath?.trim() || '';
|
const mediaPath = currentMediaPath?.trim() || '';
|
||||||
if (!mediaPath || !isYoutubeMediaPath(mediaPath)) {
|
if (!mediaPath || !isYoutubeMediaPath(mediaPath)) {
|
||||||
return;
|
return;
|
||||||
@@ -164,5 +168,13 @@ export function createYoutubePrimarySubtitleNotificationRuntime(deps: {
|
|||||||
clearPendingTimer();
|
clearPendingTimer();
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
|
setAppOwnedFlowInFlight: (inFlight: boolean): void => {
|
||||||
|
appOwnedFlowInFlight = inFlight;
|
||||||
|
if (inFlight) {
|
||||||
|
clearPendingTimer();
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
schedulePendingCheck();
|
||||||
|
},
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -469,6 +469,87 @@ test('youtube track picker ignores duplicate resolve submissions while request i
|
|||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('youtube track picker keeps no-track controls disabled after a rejected continue request', async () => {
|
||||||
|
const originalWindow = globalThis.window;
|
||||||
|
const originalDocument = globalThis.document;
|
||||||
|
|
||||||
|
Object.defineProperty(globalThis, 'document', {
|
||||||
|
configurable: true,
|
||||||
|
value: {
|
||||||
|
createElement: () => createFakeElement(),
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
Object.defineProperty(globalThis, 'window', {
|
||||||
|
configurable: true,
|
||||||
|
value: {
|
||||||
|
dispatchEvent: () => true,
|
||||||
|
focus: () => {},
|
||||||
|
electronAPI: {
|
||||||
|
notifyOverlayModalOpened: () => {},
|
||||||
|
notifyOverlayModalClosed: () => {},
|
||||||
|
youtubePickerResolve: async () => ({ ok: false, message: 'still no tracks' }),
|
||||||
|
setIgnoreMouseEvents: () => {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
const state = createRendererState();
|
||||||
|
const dom = {
|
||||||
|
overlay: {
|
||||||
|
classList: createClassList(),
|
||||||
|
focus: () => {},
|
||||||
|
},
|
||||||
|
youtubePickerModal: createFakeElement(),
|
||||||
|
youtubePickerTitle: createFakeElement(),
|
||||||
|
youtubePickerPrimarySelect: createFakeElement(),
|
||||||
|
youtubePickerSecondarySelect: createFakeElement(),
|
||||||
|
youtubePickerTracks: createFakeElement(),
|
||||||
|
youtubePickerStatus: createFakeElement(),
|
||||||
|
youtubePickerContinueButton: createFakeElement(),
|
||||||
|
youtubePickerCloseButton: createFakeElement(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const modal = createYoutubeTrackPickerModal(
|
||||||
|
{
|
||||||
|
state,
|
||||||
|
dom,
|
||||||
|
platform: {
|
||||||
|
shouldToggleMouseIgnore: false,
|
||||||
|
},
|
||||||
|
} as never,
|
||||||
|
{
|
||||||
|
modalStateReader: { isAnyModalOpen: () => true },
|
||||||
|
restorePointerInteractionState: () => {},
|
||||||
|
syncSettingsModalSubtitleSuppression: () => {},
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
modal.openYoutubePickerModal({
|
||||||
|
sessionId: 'yt-1',
|
||||||
|
url: 'https://example.com',
|
||||||
|
tracks: [],
|
||||||
|
defaultPrimaryTrackId: null,
|
||||||
|
defaultSecondaryTrackId: null,
|
||||||
|
hasTracks: false,
|
||||||
|
});
|
||||||
|
modal.wireDomEvents();
|
||||||
|
|
||||||
|
const listeners = dom.youtubePickerContinueButton.listeners.get('click') ?? [];
|
||||||
|
await Promise.all(listeners.map((listener) => listener()));
|
||||||
|
|
||||||
|
assert.equal(dom.youtubePickerPrimarySelect.disabled, true);
|
||||||
|
assert.equal(dom.youtubePickerSecondarySelect.disabled, true);
|
||||||
|
assert.equal(dom.youtubePickerContinueButton.disabled, true);
|
||||||
|
assert.equal(dom.youtubePickerCloseButton.disabled, true);
|
||||||
|
assert.equal(dom.youtubePickerStatus.textContent, 'still no tracks');
|
||||||
|
} finally {
|
||||||
|
Object.defineProperty(globalThis, 'window', { configurable: true, value: originalWindow });
|
||||||
|
Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
test('youtube track picker only consumes handled keys', async () => {
|
test('youtube track picker only consumes handled keys', async () => {
|
||||||
const originalWindow = globalThis.window;
|
const originalWindow = globalThis.window;
|
||||||
const originalDocument = globalThis.document;
|
const originalDocument = globalThis.document;
|
||||||
|
|||||||
@@ -159,7 +159,9 @@ export function createYoutubeTrackPickerModal(
|
|||||||
setStatus(error instanceof Error ? error.message : String(error), true);
|
setStatus(error instanceof Error ? error.message : String(error), true);
|
||||||
} finally {
|
} finally {
|
||||||
resolveSelectionInFlight = false;
|
resolveSelectionInFlight = false;
|
||||||
setResolveControlsDisabled(false);
|
const shouldKeepDisabled =
|
||||||
|
ctx.state.youtubePickerModalOpen && !(ctx.state.youtubePickerPayload?.hasTracks ?? false);
|
||||||
|
setResolveControlsDisabled(shouldKeepDisabled);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user