mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-24 12:11:29 -07:00
fix: address PR #31 follow-up review comments
This commit is contained in:
49
src/main.ts
49
src/main.ts
@@ -947,30 +947,29 @@ async function runYoutubePlaybackFlowMain(request: {
|
|||||||
source: CliCommandSource;
|
source: CliCommandSource;
|
||||||
}): Promise<void> {
|
}): Promise<void> {
|
||||||
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(true);
|
youtubePrimarySubtitleNotificationRuntime.setAppOwnedFlowInFlight(true);
|
||||||
if (process.platform === 'win32' && !appState.mpvClient?.connected) {
|
|
||||||
const launchResult = launchWindowsMpv(
|
|
||||||
[request.url],
|
|
||||||
createWindowsMpvLaunchDeps({
|
|
||||||
showError: (title, content) => dialog.showErrorBox(title, content),
|
|
||||||
}),
|
|
||||||
[
|
|
||||||
'--pause=yes',
|
|
||||||
'--sub-auto=no',
|
|
||||||
'--sid=no',
|
|
||||||
'--secondary-sid=no',
|
|
||||||
'--script-opts=subminer-auto_start_pause_until_ready=no',
|
|
||||||
`--input-ipc-server=${appState.mpvSocketPath}`,
|
|
||||||
],
|
|
||||||
);
|
|
||||||
if (!launchResult.ok) {
|
|
||||||
logger.warn('Unable to bootstrap Windows mpv for YouTube playback.');
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if (!appState.mpvClient?.connected) {
|
|
||||||
appState.mpvClient?.connect();
|
|
||||||
}
|
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
if (process.platform === 'win32' && !appState.mpvClient?.connected) {
|
||||||
|
const launchResult = launchWindowsMpv(
|
||||||
|
[request.url],
|
||||||
|
createWindowsMpvLaunchDeps({
|
||||||
|
showError: (title, content) => dialog.showErrorBox(title, content),
|
||||||
|
}),
|
||||||
|
[
|
||||||
|
'--pause=yes',
|
||||||
|
'--sub-auto=no',
|
||||||
|
'--sid=no',
|
||||||
|
'--secondary-sid=no',
|
||||||
|
'--script-opts=subminer-auto_start_pause_until_ready=no',
|
||||||
|
`--input-ipc-server=${appState.mpvSocketPath}`,
|
||||||
|
],
|
||||||
|
);
|
||||||
|
if (!launchResult.ok) {
|
||||||
|
logger.warn('Unable to bootstrap Windows mpv for YouTube playback.');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (!appState.mpvClient?.connected) {
|
||||||
|
appState.mpvClient?.connect();
|
||||||
|
}
|
||||||
await youtubeFlowRuntime.runYoutubePlaybackFlow({
|
await youtubeFlowRuntime.runYoutubePlaybackFlow({
|
||||||
url: request.url,
|
url: request.url,
|
||||||
mode: request.mode,
|
mode: request.mode,
|
||||||
@@ -1263,6 +1262,10 @@ function reportYoutubeSubtitleFailure(message: string): void {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async function openYoutubeTrackPickerFromPlayback(): Promise<void> {
|
async function openYoutubeTrackPickerFromPlayback(): Promise<void> {
|
||||||
|
if (youtubeFlowRuntime.hasActiveSession()) {
|
||||||
|
showMpvOsd('YouTube subtitle flow already in progress.');
|
||||||
|
return;
|
||||||
|
}
|
||||||
const currentMediaPath =
|
const currentMediaPath =
|
||||||
appState.currentMediaPath?.trim() || appState.mpvClient?.currentVideoPath?.trim() || '';
|
appState.currentMediaPath?.trim() || appState.mpvClient?.currentVideoPath?.trim() || '';
|
||||||
if (!isYoutubePlaybackActiveNow() || !currentMediaPath) {
|
if (!isYoutubePlaybackActiveNow() || !currentMediaPath) {
|
||||||
|
|||||||
@@ -82,6 +82,28 @@ test('notifier suppresses failure when preferred primary subtitle is selected',
|
|||||||
assert.deepEqual(notifications, []);
|
assert.deepEqual(notifications, []);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('notifier suppresses failure when selected track is marked active before sid arrives', () => {
|
||||||
|
const notifications: string[] = [];
|
||||||
|
const timers = createTimerHarness();
|
||||||
|
const runtime = createYoutubePrimarySubtitleNotificationRuntime({
|
||||||
|
getPrimarySubtitleLanguages: () => ['ja', 'jpn'],
|
||||||
|
notifyFailure: (message) => {
|
||||||
|
notifications.push(message);
|
||||||
|
},
|
||||||
|
schedule: (fn) => timers.schedule(fn),
|
||||||
|
clearSchedule: (timer) => timers.clear(timer),
|
||||||
|
});
|
||||||
|
|
||||||
|
runtime.handleMediaPathChange('https://www.youtube.com/watch?v=abc');
|
||||||
|
runtime.handleSubtitleTrackChange(null);
|
||||||
|
runtime.handleSubtitleTrackListChange([
|
||||||
|
{ type: 'sub', id: 5, lang: 'ja', title: 'Japanese', external: false, selected: true },
|
||||||
|
]);
|
||||||
|
timers.runAll();
|
||||||
|
|
||||||
|
assert.deepEqual(notifications, []);
|
||||||
|
});
|
||||||
|
|
||||||
test('notifier suppresses failure when any external subtitle track is selected', () => {
|
test('notifier suppresses failure when any external subtitle track is selected', () => {
|
||||||
const notifications: string[] = [];
|
const notifications: string[] = [];
|
||||||
const timers = createTimerHarness();
|
const timers = createTimerHarness();
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ type SubtitleTrackEntry = {
|
|||||||
type: string;
|
type: string;
|
||||||
lang: string;
|
lang: string;
|
||||||
external: boolean;
|
external: boolean;
|
||||||
|
selected: boolean;
|
||||||
};
|
};
|
||||||
|
|
||||||
function parseTrackId(value: unknown): number | null {
|
function parseTrackId(value: unknown): number | null {
|
||||||
@@ -31,6 +32,7 @@ function normalizeTrack(entry: unknown): SubtitleTrackEntry | null {
|
|||||||
type: String(track.type || '').trim(),
|
type: String(track.type || '').trim(),
|
||||||
lang: String(track.lang || '').trim(),
|
lang: String(track.lang || '').trim(),
|
||||||
external: track.external === true,
|
external: track.external === true,
|
||||||
|
selected: track.selected === true,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -74,12 +76,15 @@ function hasSelectedPrimarySubtitle(
|
|||||||
trackList: unknown[] | null,
|
trackList: unknown[] | null,
|
||||||
preferredLanguages: Set<string>,
|
preferredLanguages: Set<string>,
|
||||||
): boolean {
|
): boolean {
|
||||||
if (sid === null || !Array.isArray(trackList)) {
|
if (!Array.isArray(trackList)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const tracks = trackList.map(normalizeTrack);
|
||||||
const activeTrack =
|
const activeTrack =
|
||||||
trackList.map(normalizeTrack).find((track) => track?.type === 'sub' && track.id === sid) ?? null;
|
(sid === null ? null : tracks.find((track) => track?.type === 'sub' && track.id === sid) ?? null) ??
|
||||||
|
tracks.find((track) => track?.type === 'sub' && track.selected) ??
|
||||||
|
null;
|
||||||
if (!activeTrack) {
|
if (!activeTrack) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -35,6 +35,9 @@ function createFakeElement() {
|
|||||||
append(...children: any[]) {
|
append(...children: any[]) {
|
||||||
this.children.push(...children);
|
this.children.push(...children);
|
||||||
},
|
},
|
||||||
|
replaceChildren(...children: any[]) {
|
||||||
|
this.children = [...children];
|
||||||
|
},
|
||||||
addEventListener(type: string, listener: (event?: any) => void) {
|
addEventListener(type: string, listener: (event?: any) => void) {
|
||||||
const existing = this.listeners.get(type) ?? [];
|
const existing = this.listeners.get(type) ?? [];
|
||||||
existing.push(listener);
|
existing.push(listener);
|
||||||
@@ -763,3 +766,109 @@ test('youtube track picker ignores immediate Enter after open before allowing ke
|
|||||||
Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
|
Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('youtube track picker uses track list as the source of truth for available selections', async () => {
|
||||||
|
const resolveCalls: Array<{
|
||||||
|
sessionId: string;
|
||||||
|
action: string;
|
||||||
|
primaryTrackId: string | null;
|
||||||
|
secondaryTrackId: string | null;
|
||||||
|
}> = [];
|
||||||
|
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 (payload: {
|
||||||
|
sessionId: string;
|
||||||
|
action: string;
|
||||||
|
primaryTrackId: string | null;
|
||||||
|
secondaryTrackId: string | null;
|
||||||
|
}) => {
|
||||||
|
resolveCalls.push(payload);
|
||||||
|
return { ok: true, message: '' };
|
||||||
|
},
|
||||||
|
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: [
|
||||||
|
{
|
||||||
|
id: 'manual:ja',
|
||||||
|
language: 'ja',
|
||||||
|
sourceLanguage: 'ja',
|
||||||
|
kind: 'manual',
|
||||||
|
label: 'Japanese',
|
||||||
|
},
|
||||||
|
],
|
||||||
|
defaultPrimaryTrackId: 'manual:ja',
|
||||||
|
defaultSecondaryTrackId: null,
|
||||||
|
hasTracks: false,
|
||||||
|
});
|
||||||
|
modal.wireDomEvents();
|
||||||
|
|
||||||
|
const listeners = dom.youtubePickerContinueButton.listeners.get('click') ?? [];
|
||||||
|
await Promise.all(listeners.map((listener) => listener()));
|
||||||
|
|
||||||
|
assert.deepEqual(resolveCalls, [
|
||||||
|
{
|
||||||
|
sessionId: 'yt-1',
|
||||||
|
action: 'use-selected',
|
||||||
|
primaryTrackId: 'manual:ja',
|
||||||
|
secondaryTrackId: null,
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
} finally {
|
||||||
|
Object.defineProperty(globalThis, 'window', { configurable: true, value: originalWindow });
|
||||||
|
Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -9,6 +9,10 @@ function createOption(value: string, label: string): HTMLOptionElement {
|
|||||||
return option;
|
return option;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function payloadHasTracks(payload: YoutubePickerOpenPayload | null): boolean {
|
||||||
|
return (payload?.tracks.length ?? 0) > 0;
|
||||||
|
}
|
||||||
|
|
||||||
export function createYoutubeTrackPickerModal(
|
export function createYoutubeTrackPickerModal(
|
||||||
ctx: RendererContext,
|
ctx: RendererContext,
|
||||||
options: {
|
options: {
|
||||||
@@ -34,9 +38,9 @@ export function createYoutubeTrackPickerModal(
|
|||||||
}
|
}
|
||||||
|
|
||||||
function renderTrackList(): void {
|
function renderTrackList(): void {
|
||||||
ctx.dom.youtubePickerTracks.innerHTML = '';
|
ctx.dom.youtubePickerTracks.replaceChildren();
|
||||||
const payload = ctx.state.youtubePickerPayload;
|
const payload = ctx.state.youtubePickerPayload;
|
||||||
if (!payload || payload.tracks.length === 0) {
|
if (!payload || !payloadHasTracks(payload)) {
|
||||||
const li = document.createElement('li');
|
const li = document.createElement('li');
|
||||||
const left = document.createElement('span');
|
const left = document.createElement('span');
|
||||||
left.textContent = 'No subtitle tracks found';
|
left.textContent = 'No subtitle tracks found';
|
||||||
@@ -70,7 +74,7 @@ export function createYoutubeTrackPickerModal(
|
|||||||
function syncSecondaryOptions(): void {
|
function syncSecondaryOptions(): void {
|
||||||
const payload = ctx.state.youtubePickerPayload;
|
const payload = ctx.state.youtubePickerPayload;
|
||||||
const primaryTrackId = ctx.dom.youtubePickerPrimarySelect.value || null;
|
const primaryTrackId = ctx.dom.youtubePickerPrimarySelect.value || null;
|
||||||
ctx.dom.youtubePickerSecondarySelect.innerHTML = '';
|
ctx.dom.youtubePickerSecondarySelect.replaceChildren();
|
||||||
ctx.dom.youtubePickerSecondarySelect.appendChild(createOption('', 'None'));
|
ctx.dom.youtubePickerSecondarySelect.appendChild(createOption('', 'None'));
|
||||||
if (!payload) return;
|
if (!payload) return;
|
||||||
|
|
||||||
@@ -97,10 +101,10 @@ export function createYoutubeTrackPickerModal(
|
|||||||
function applyPayload(payload: YoutubePickerOpenPayload): void {
|
function applyPayload(payload: YoutubePickerOpenPayload): void {
|
||||||
ctx.state.youtubePickerPayload = payload;
|
ctx.state.youtubePickerPayload = payload;
|
||||||
ctx.dom.youtubePickerTitle.textContent = `Select YouTube subtitles for ${payload.url}`;
|
ctx.dom.youtubePickerTitle.textContent = `Select YouTube subtitles for ${payload.url}`;
|
||||||
ctx.dom.youtubePickerPrimarySelect.innerHTML = '';
|
ctx.dom.youtubePickerPrimarySelect.replaceChildren();
|
||||||
ctx.dom.youtubePickerSecondarySelect.innerHTML = '';
|
ctx.dom.youtubePickerSecondarySelect.replaceChildren();
|
||||||
|
|
||||||
if (payload.tracks.length === 0) {
|
if (!payloadHasTracks(payload)) {
|
||||||
ctx.dom.youtubePickerPrimarySelect.appendChild(createOption('', 'No tracks available'));
|
ctx.dom.youtubePickerPrimarySelect.appendChild(createOption('', 'No tracks available'));
|
||||||
ctx.dom.youtubePickerPrimarySelect.disabled = true;
|
ctx.dom.youtubePickerPrimarySelect.disabled = true;
|
||||||
ctx.dom.youtubePickerSecondarySelect.disabled = true;
|
ctx.dom.youtubePickerSecondarySelect.disabled = true;
|
||||||
@@ -128,7 +132,8 @@ export function createYoutubeTrackPickerModal(
|
|||||||
}
|
}
|
||||||
const payload = ctx.state.youtubePickerPayload;
|
const payload = ctx.state.youtubePickerPayload;
|
||||||
if (!payload) return;
|
if (!payload) return;
|
||||||
if (action === 'use-selected' && payload.hasTracks && !ctx.dom.youtubePickerPrimarySelect.value) {
|
const hasTracks = payloadHasTracks(payload);
|
||||||
|
if (action === 'use-selected' && hasTracks && !ctx.dom.youtubePickerPrimarySelect.value) {
|
||||||
setStatus('Primary subtitle selection is required.', true);
|
setStatus('Primary subtitle selection is required.', true);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -160,7 +165,7 @@ export function createYoutubeTrackPickerModal(
|
|||||||
} finally {
|
} finally {
|
||||||
resolveSelectionInFlight = false;
|
resolveSelectionInFlight = false;
|
||||||
const shouldKeepDisabled =
|
const shouldKeepDisabled =
|
||||||
ctx.state.youtubePickerModalOpen && !(ctx.state.youtubePickerPayload?.hasTracks ?? false);
|
ctx.state.youtubePickerModalOpen && !payloadHasTracks(ctx.state.youtubePickerPayload);
|
||||||
setResolveControlsDisabled(shouldKeepDisabled);
|
setResolveControlsDisabled(shouldKeepDisabled);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -233,7 +238,7 @@ export function createYoutubeTrackPickerModal(
|
|||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
void resolveSelection(
|
void resolveSelection(
|
||||||
ctx.state.youtubePickerPayload?.hasTracks ? 'use-selected' : 'continue-without-subtitles',
|
payloadHasTracks(ctx.state.youtubePickerPayload) ? 'use-selected' : 'continue-without-subtitles',
|
||||||
);
|
);
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
@@ -264,7 +269,7 @@ export function createYoutubeTrackPickerModal(
|
|||||||
|
|
||||||
ctx.dom.youtubePickerContinueButton.addEventListener('click', () => {
|
ctx.dom.youtubePickerContinueButton.addEventListener('click', () => {
|
||||||
void resolveSelection(
|
void resolveSelection(
|
||||||
ctx.state.youtubePickerPayload?.hasTracks ? 'use-selected' : 'continue-without-subtitles',
|
payloadHasTracks(ctx.state.youtubePickerPayload) ? 'use-selected' : 'continue-without-subtitles',
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user