mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-03-25 00:11:26 -07:00
fix: address latest CodeRabbit review items
This commit is contained in:
@@ -12,6 +12,7 @@ test('isYoutubeMediaPath detects youtube watch and short urls', () => {
|
||||
test('isYoutubeMediaPath ignores local files and non-youtube urls', () => {
|
||||
assert.equal(isYoutubeMediaPath('/tmp/video.mkv'), false);
|
||||
assert.equal(isYoutubeMediaPath('https://example.com/watch?v=abc123'), false);
|
||||
assert.equal(isYoutubeMediaPath('https://notyoutube.com/watch?v=abc123'), false);
|
||||
assert.equal(isYoutubeMediaPath(' '), false);
|
||||
assert.equal(isYoutubeMediaPath(null), false);
|
||||
});
|
||||
|
||||
@@ -6,6 +6,10 @@ function trimToNull(value: string | null | undefined): string | null {
|
||||
return trimmed.length > 0 ? trimmed : null;
|
||||
}
|
||||
|
||||
function matchesYoutubeHost(hostname: string, expectedHost: string): boolean {
|
||||
return hostname === expectedHost || hostname.endsWith(`.${expectedHost}`);
|
||||
}
|
||||
|
||||
export function isYoutubeMediaPath(mediaPath: string | null | undefined): boolean {
|
||||
const normalized = trimToNull(mediaPath);
|
||||
if (!normalized) {
|
||||
@@ -21,10 +25,9 @@ export function isYoutubeMediaPath(mediaPath: string | null | undefined): boolea
|
||||
|
||||
const host = parsed.hostname.toLowerCase();
|
||||
return (
|
||||
host === 'youtu.be' ||
|
||||
host.endsWith('.youtu.be') ||
|
||||
host.endsWith('youtube.com') ||
|
||||
host.endsWith('youtube-nocookie.com')
|
||||
matchesYoutubeHost(host, 'youtu.be') ||
|
||||
matchesYoutubeHost(host, 'youtube.com') ||
|
||||
matchesYoutubeHost(host, 'youtube-nocookie.com')
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@@ -349,6 +349,126 @@ test('youtube track picker surfaces rejected resolve calls as modal status', asy
|
||||
}
|
||||
});
|
||||
|
||||
test('youtube track picker ignores duplicate resolve submissions while request is in flight', async () => {
|
||||
const resolveCalls: Array<{
|
||||
sessionId: string;
|
||||
action: string;
|
||||
primaryTrackId: string | null;
|
||||
secondaryTrackId: string | null;
|
||||
}> = [];
|
||||
const originalWindow = globalThis.window;
|
||||
const originalDocument = globalThis.document;
|
||||
let releaseResolve: (() => void) | null = null;
|
||||
|
||||
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);
|
||||
await new Promise<void>((resolve) => {
|
||||
releaseResolve = resolve;
|
||||
});
|
||||
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',
|
||||
mode: 'download',
|
||||
tracks: [
|
||||
{
|
||||
id: 'auto:ja-orig',
|
||||
language: 'ja',
|
||||
sourceLanguage: 'ja-orig',
|
||||
kind: 'auto',
|
||||
label: 'Japanese (auto)',
|
||||
},
|
||||
],
|
||||
defaultPrimaryTrackId: 'auto:ja-orig',
|
||||
defaultSecondaryTrackId: null,
|
||||
hasTracks: true,
|
||||
});
|
||||
modal.wireDomEvents();
|
||||
|
||||
const listeners = dom.youtubePickerContinueButton.listeners.get('click') ?? [];
|
||||
const first = listeners[0]?.();
|
||||
const second = listeners[0]?.();
|
||||
await Promise.resolve();
|
||||
|
||||
assert.equal(resolveCalls.length, 1);
|
||||
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.ok(releaseResolve);
|
||||
const release = releaseResolve as () => void;
|
||||
release();
|
||||
await Promise.all([first, second]);
|
||||
|
||||
assert.equal(dom.youtubePickerPrimarySelect.disabled, false);
|
||||
assert.equal(dom.youtubePickerSecondarySelect.disabled, false);
|
||||
assert.equal(dom.youtubePickerContinueButton.disabled, false);
|
||||
assert.equal(dom.youtubePickerCloseButton.disabled, false);
|
||||
} 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 () => {
|
||||
const originalWindow = globalThis.window;
|
||||
const originalDocument = globalThis.document;
|
||||
|
||||
@@ -17,6 +17,8 @@ export function createYoutubeTrackPickerModal(
|
||||
syncSettingsModalSubtitleSuppression: () => void;
|
||||
},
|
||||
) {
|
||||
let resolveSelectionInFlight = false;
|
||||
|
||||
function setStatus(message: string, isError = false): void {
|
||||
ctx.state.youtubePickerStatus = message;
|
||||
ctx.dom.youtubePickerStatus.textContent = message;
|
||||
@@ -34,7 +36,12 @@ export function createYoutubeTrackPickerModal(
|
||||
const payload = ctx.state.youtubePickerPayload;
|
||||
if (!payload || payload.tracks.length === 0) {
|
||||
const li = document.createElement('li');
|
||||
li.innerHTML = '<span>No subtitle tracks found</span><span class="youtube-picker-track-meta">Continue without subtitles</span>';
|
||||
const left = document.createElement('span');
|
||||
left.textContent = 'No subtitle tracks found';
|
||||
const right = document.createElement('span');
|
||||
right.className = 'youtube-picker-track-meta';
|
||||
right.textContent = 'Continue without subtitles';
|
||||
li.append(left, right);
|
||||
ctx.dom.youtubePickerTracks.appendChild(li);
|
||||
return;
|
||||
}
|
||||
@@ -51,6 +58,13 @@ export function createYoutubeTrackPickerModal(
|
||||
}
|
||||
}
|
||||
|
||||
function setResolveControlsDisabled(disabled: boolean): void {
|
||||
ctx.dom.youtubePickerPrimarySelect.disabled = disabled;
|
||||
ctx.dom.youtubePickerSecondarySelect.disabled = disabled;
|
||||
ctx.dom.youtubePickerContinueButton.disabled = disabled;
|
||||
ctx.dom.youtubePickerCloseButton.disabled = disabled;
|
||||
}
|
||||
|
||||
function syncSecondaryOptions(): void {
|
||||
const payload = ctx.state.youtubePickerPayload;
|
||||
const primaryTrackId = ctx.dom.youtubePickerPrimarySelect.value || null;
|
||||
@@ -107,6 +121,9 @@ export function createYoutubeTrackPickerModal(
|
||||
}
|
||||
|
||||
async function resolveSelection(action: 'use-selected' | 'continue-without-subtitles'): Promise<void> {
|
||||
if (resolveSelectionInFlight) {
|
||||
return;
|
||||
}
|
||||
const payload = ctx.state.youtubePickerPayload;
|
||||
if (!payload) return;
|
||||
if (action === 'use-selected' && payload.hasTracks && !ctx.dom.youtubePickerPrimarySelect.value) {
|
||||
@@ -114,9 +131,10 @@ export function createYoutubeTrackPickerModal(
|
||||
return;
|
||||
}
|
||||
|
||||
let response;
|
||||
resolveSelectionInFlight = true;
|
||||
setResolveControlsDisabled(true);
|
||||
try {
|
||||
response =
|
||||
const response =
|
||||
action === 'use-selected'
|
||||
? await window.electronAPI.youtubePickerResolve({
|
||||
sessionId: payload.sessionId,
|
||||
@@ -130,15 +148,17 @@ export function createYoutubeTrackPickerModal(
|
||||
primaryTrackId: null,
|
||||
secondaryTrackId: null,
|
||||
});
|
||||
if (!response.ok) {
|
||||
setStatus(response.message, true);
|
||||
return;
|
||||
}
|
||||
closeYoutubePickerModal();
|
||||
} catch (error) {
|
||||
setStatus(error instanceof Error ? error.message : String(error), true);
|
||||
return;
|
||||
} finally {
|
||||
resolveSelectionInFlight = false;
|
||||
setResolveControlsDisabled(false);
|
||||
}
|
||||
if (!response.ok) {
|
||||
setStatus(response.message, true);
|
||||
return;
|
||||
}
|
||||
closeYoutubePickerModal();
|
||||
}
|
||||
|
||||
function openYoutubePickerModal(payload: YoutubePickerOpenPayload): void {
|
||||
|
||||
@@ -5,6 +5,7 @@ import { getRelatedCollectionLabel } from './MediaDetailView';
|
||||
test('getRelatedCollectionLabel returns View Channel for youtube-backed media', () => {
|
||||
assert.equal(
|
||||
getRelatedCollectionLabel({
|
||||
videoId: 1,
|
||||
animeId: 1,
|
||||
canonicalTitle: 'Video',
|
||||
totalSessions: 1,
|
||||
@@ -24,6 +25,7 @@ test('getRelatedCollectionLabel returns View Channel for youtube-backed media',
|
||||
test('getRelatedCollectionLabel returns View Anime for non-youtube media', () => {
|
||||
assert.equal(
|
||||
getRelatedCollectionLabel({
|
||||
videoId: 2,
|
||||
animeId: 1,
|
||||
canonicalTitle: 'Episode 5',
|
||||
totalSessions: 1,
|
||||
|
||||
Reference in New Issue
Block a user