fix: address latest CodeRabbit review items

This commit is contained in:
2026-03-23 01:01:53 -07:00
parent c6e6aeebbe
commit 207151dba3
5 changed files with 159 additions and 13 deletions

View File

@@ -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);
});

View File

@@ -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')
);
}

View File

@@ -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;

View File

@@ -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 {

View File

@@ -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,