From d8a7ae77b0c9242ccbd9d17931efecd28e9e1df3 Mon Sep 17 00:00:00 2001 From: sudacode Date: Sun, 22 Mar 2026 20:09:16 -0700 Subject: [PATCH] fix: address latest review feedback --- .../immersion-tracker-service.test.ts | 53 +++++++++--- .../immersion-tracker/storage-session.test.ts | 2 + .../services/youtube/metadata-probe.test.ts | 49 +++++++++++ src/core/services/youtube/metadata-probe.ts | 7 +- src/core/services/youtube/timedtext.test.ts | 40 +++++++++ src/core/services/youtube/timedtext.ts | 21 ++++- .../services/youtube/track-download.test.ts | 27 ++++++ src/core/services/youtube/track-download.ts | 8 +- src/core/services/youtube/track-probe.test.ts | 14 +++- src/core/services/youtube/track-probe.ts | 18 +++- src/main/runtime/youtube-flow.test.ts | 66 +++++++++++++++ src/main/runtime/youtube-flow.ts | 5 ++ src/renderer/handlers/mouse.test.ts | 84 +++++++++++++++++++ src/renderer/handlers/mouse.ts | 7 +- stats/src/components/library/LibraryTab.tsx | 9 +- stats/src/components/library/MediaCard.tsx | 4 +- stats/src/lib/media-library-grouping.test.tsx | 26 +++++- stats/src/lib/media-library-grouping.ts | 32 +++++-- 18 files changed, 428 insertions(+), 44 deletions(-) create mode 100644 src/core/services/youtube/metadata-probe.test.ts create mode 100644 src/core/services/youtube/timedtext.test.ts diff --git a/src/core/services/immersion-tracker-service.test.ts b/src/core/services/immersion-tracker-service.test.ts index 6f3a5d6..c4fe2de 100644 --- a/src/core/services/immersion-tracker-service.test.ts +++ b/src/core/services/immersion-tracker-service.test.ts @@ -37,6 +37,21 @@ async function waitForPendingAnimeMetadata(tracker: ImmersionTrackerService): Pr await privateApi.pendingAnimeMetadataUpdates?.get(videoId); } +async function waitForCondition( + predicate: () => boolean, + timeoutMs = 1_000, + intervalMs = 10, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (predicate()) { + return; + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + assert.equal(predicate(), true); +} + function makeMergedToken(overrides: Partial): MergedToken { return { surface: '', @@ -2305,13 +2320,24 @@ test('handleMediaChange stores youtube metadata for new youtube sessions', async try { const fakeBinDir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-yt-dlp-bin-')); - const scriptPath = path.join(fakeBinDir, 'yt-dlp'); - fs.writeFileSync( - scriptPath, - `#!/bin/sh -printf '%s\n' '{"id":"abc123","title":"Video Name","webpage_url":"https://www.youtube.com/watch?v=abc123","thumbnail":"https://i.ytimg.com/vi/abc123/hqdefault.jpg","channel_id":"UCcreator123","channel":"Creator Name","channel_url":"https://www.youtube.com/channel/UCcreator123","uploader_id":"@creator","uploader_url":"https://www.youtube.com/@creator","description":"Video description","channel_follower_count":12345,"thumbnails":[{"url":"https://i.ytimg.com/vi/abc123/hqdefault.jpg"},{"url":"https://yt3.googleusercontent.com/channel-avatar=s88"}]}'\n`, - { mode: 0o755 }, - ); + const ytDlpOutput = + '{"id":"abc123","title":"Video Name","webpage_url":"https://www.youtube.com/watch?v=abc123","thumbnail":"https://i.ytimg.com/vi/abc123/hqdefault.jpg","channel_id":"UCcreator123","channel":"Creator Name","channel_url":"https://www.youtube.com/channel/UCcreator123","uploader_id":"@creator","uploader_url":"https://www.youtube.com/@creator","description":"Video description","channel_follower_count":12345,"thumbnails":[{"url":"https://i.ytimg.com/vi/abc123/hqdefault.jpg"},{"url":"https://yt3.googleusercontent.com/channel-avatar=s88"}]}'; + if (process.platform === 'win32') { + fs.writeFileSync( + path.join(fakeBinDir, 'yt-dlp.cmd'), + `@echo off\r\necho ${ytDlpOutput.replace(/"/g, '\\"')}\r\n`, + 'utf8', + ); + } else { + const scriptPath = path.join(fakeBinDir, 'yt-dlp'); + fs.writeFileSync( + scriptPath, + `#!/bin/sh +printf '%s\n' '${ytDlpOutput}' +`, + { mode: 0o755 }, + ); + } process.env.PATH = `${fakeBinDir}${path.delimiter}${originalPath ?? ''}`; globalThis.fetch = async (input) => { @@ -2333,11 +2359,16 @@ printf '%s\n' '{"id":"abc123","title":"Video Name","webpage_url":"https://www.yo const Ctor = await loadTrackerCtor(); tracker = new Ctor({ dbPath }); tracker.handleMediaChange('https://www.youtube.com/watch?v=abc123', 'Player Title'); - - await waitForPendingAnimeMetadata(tracker); - await new Promise((resolve) => setTimeout(resolve, 25)); - const privateApi = tracker as unknown as { db: DatabaseSync }; + await waitForCondition( + () => { + const stored = privateApi.db + .prepare("SELECT 1 AS ready FROM imm_youtube_videos WHERE youtube_video_id = 'abc123'") + .get() as { ready: number } | null; + return stored?.ready === 1; + }, + 5_000, + ); const row = privateApi.db .prepare( ` diff --git a/src/core/services/immersion-tracker/storage-session.test.ts b/src/core/services/immersion-tracker/storage-session.test.ts index ddbc9e1..150e431 100644 --- a/src/core/services/immersion-tracker/storage-session.test.ts +++ b/src/core/services/immersion-tracker/storage-session.test.ts @@ -278,6 +278,8 @@ test('ensureSchema creates large-history performance indexes', () => { assert.ok(indexNames.has('idx_kanji_frequency')); assert.ok(indexNames.has('idx_media_art_anilist_id')); assert.ok(indexNames.has('idx_media_art_cover_url')); + assert.ok(indexNames.has('idx_youtube_videos_channel_id')); + assert.ok(indexNames.has('idx_youtube_videos_youtube_video_id')); } finally { db.close(); cleanupDbPath(dbPath); diff --git a/src/core/services/youtube/metadata-probe.test.ts b/src/core/services/youtube/metadata-probe.test.ts new file mode 100644 index 0000000..562a2b3 --- /dev/null +++ b/src/core/services/youtube/metadata-probe.test.ts @@ -0,0 +1,49 @@ +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import test from 'node:test'; +import { probeYoutubeVideoMetadata } from './metadata-probe'; + +async function withTempDir(fn: (dir: string) => Promise): Promise { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'subminer-youtube-metadata-probe-')); + try { + return await fn(dir); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } +} + +function makeFakeYtDlpScript(dir: string, payload: string): void { + const scriptPath = path.join(dir, 'yt-dlp'); + const script = `#!/usr/bin/env node +process.stdout.write(${JSON.stringify(payload)}); +`; + fs.writeFileSync(scriptPath, script, 'utf8'); + if (process.platform !== 'win32') { + fs.chmodSync(scriptPath, 0o755); + } + fs.writeFileSync(scriptPath + '.cmd', `@echo off\r\nnode "${scriptPath}"\r\n`, 'utf8'); +} + +async function withFakeYtDlp(payload: string, fn: () => Promise): Promise { + return await withTempDir(async (root) => { + const binDir = path.join(root, 'bin'); + fs.mkdirSync(binDir, { recursive: true }); + makeFakeYtDlpScript(binDir, payload); + const originalPath = process.env.PATH ?? ''; + process.env.PATH = `${binDir}${path.delimiter}${originalPath}`; + try { + return await fn(); + } finally { + process.env.PATH = originalPath; + } + }); +} + +test('probeYoutubeVideoMetadata returns null on malformed yt-dlp JSON', async () => { + await withFakeYtDlp('not-json', async () => { + const result = await probeYoutubeVideoMetadata('https://www.youtube.com/watch?v=abc123'); + assert.equal(result, null); + }); +}); diff --git a/src/core/services/youtube/metadata-probe.ts b/src/core/services/youtube/metadata-probe.ts index c61736b..1344ef5 100644 --- a/src/core/services/youtube/metadata-probe.ts +++ b/src/core/services/youtube/metadata-probe.ts @@ -79,7 +79,12 @@ export async function probeYoutubeVideoMetadata( '--skip-download', targetUrl, ]); - const info = JSON.parse(stdout) as YtDlpYoutubeMetadata; + let info: YtDlpYoutubeMetadata; + try { + info = JSON.parse(stdout) as YtDlpYoutubeMetadata; + } catch { + return null; + } const youtubeVideoId = info.id?.trim(); const videoUrl = info.webpage_url?.trim() || targetUrl.trim(); if (!youtubeVideoId || !videoUrl) { diff --git a/src/core/services/youtube/timedtext.test.ts b/src/core/services/youtube/timedtext.test.ts new file mode 100644 index 0000000..062220e --- /dev/null +++ b/src/core/services/youtube/timedtext.test.ts @@ -0,0 +1,40 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { convertYoutubeTimedTextToVtt } from './timedtext'; + +test('convertYoutubeTimedTextToVtt leaves malformed numeric entities literal', () => { + const result = convertYoutubeTimedTextToVtt( + '

� � A

', + ); + + assert.equal( + result, + ['WEBVTT', '', '00:00:00.000 --> 00:00:01.000', '� � A', ''].join('\n'), + ); +}); + +test('convertYoutubeTimedTextToVtt does not swallow text after zero-length overlap rows', () => { + const result = convertYoutubeTimedTextToVtt( + [ + '', + '

今日は

', + '

今日はいい天気ですね

', + '

今日はいい天気ですね

', + '
', + ].join(''), + ); + + assert.equal( + result, + [ + 'WEBVTT', + '', + '00:00:00.000 --> 00:00:00.999', + '今日は', + '', + '00:00:01.000 --> 00:00:03.000', + 'いい天気ですね', + '', + ].join('\n'), + ); +}); diff --git a/src/core/services/youtube/timedtext.ts b/src/core/services/youtube/timedtext.ts index 905d449..6580c57 100644 --- a/src/core/services/youtube/timedtext.ts +++ b/src/core/services/youtube/timedtext.ts @@ -6,6 +6,18 @@ interface YoutubeTimedTextRow { const YOUTUBE_TIMEDTEXT_EXTENSIONS = new Set(['srv1', 'srv2', 'srv3', 'ytsrv3']); +function decodeNumericEntity(match: string, codePoint: number): string { + if ( + !Number.isInteger(codePoint) || + codePoint < 0 || + codePoint > 0x10ffff || + (codePoint >= 0xd800 && codePoint <= 0xdfff) + ) { + return match; + } + return String.fromCodePoint(codePoint); +} + function decodeHtmlEntities(value: string): string { return value .replace(/&/g, '&') @@ -13,9 +25,11 @@ function decodeHtmlEntities(value: string): string { .replace(/>/g, '>') .replace(/"/g, '"') .replace(/'/g, "'") - .replace(/&#(\d+);/g, (_match, codePoint) => String.fromCodePoint(Number(codePoint))) - .replace(/&#x([0-9a-f]+);/gi, (_match, codePoint) => - String.fromCodePoint(Number.parseInt(codePoint, 16)), + .replace(/&#(\d+);/g, (match, codePoint) => + decodeNumericEntity(match, Number(codePoint)), + ) + .replace(/&#x([0-9a-f]+);/gi, (match, codePoint) => + decodeNumericEntity(match, Number.parseInt(codePoint, 16)), ); } @@ -85,7 +99,6 @@ export function convertYoutubeTimedTextToVtt(xml: string): string { ? Math.max(row.startMs, nextRow.startMs - 1) : unclampedEnd; if (clampedEnd <= row.startMs) { - previousText = row.text; continue; } diff --git a/src/core/services/youtube/track-download.test.ts b/src/core/services/youtube/track-download.test.ts index 3c317fe..f6c4d17 100644 --- a/src/core/services/youtube/track-download.test.ts +++ b/src/core/services/youtube/track-download.test.ts @@ -303,6 +303,33 @@ test('downloadYoutubeSubtitleTrack prefers direct download URL when available', }); }); +test('downloadYoutubeSubtitleTrack sanitizes metadata source language in filenames', async () => { + await withTempDir(async (root) => { + await withStubFetch( + async () => new Response('WEBVTT\n', { status: 200 }), + async () => { + const result = await downloadYoutubeSubtitleTrack({ + targetUrl: 'https://www.youtube.com/watch?v=abc123', + outputDir: path.join(root, 'out'), + track: { + id: 'auto:../../ja-orig', + language: 'ja', + sourceLanguage: '../ja-orig/../../evil', + kind: 'auto', + label: 'Japanese (auto)', + downloadUrl: 'https://example.com/subs/ja.vtt', + fileExtension: 'vtt', + }, + mode: 'download', + }); + + assert.equal(path.dirname(result.path), path.join(root, 'out')); + assert.equal(path.basename(result.path), 'auto-ja-orig.ja-orig-evil.vtt'); + }, + ); + }); +}); + test('downloadYoutubeSubtitleTrack converts srv3 auto subtitles into regular vtt', async () => { await withTempDir(async (root) => { await withStubFetch( diff --git a/src/core/services/youtube/track-download.ts b/src/core/services/youtube/track-download.ts index 90e24ca..7245cc2 100644 --- a/src/core/services/youtube/track-download.ts +++ b/src/core/services/youtube/track-download.ts @@ -9,6 +9,11 @@ const YOUTUBE_SUBTITLE_EXTENSIONS = new Set(['.srt', '.vtt', '.ass']); const YOUTUBE_BATCH_PREFIX = 'youtube-batch'; const YOUTUBE_DOWNLOAD_TIMEOUT_MS = 15_000; +function sanitizeFilenameSegment(value: string): string { + const sanitized = value.trim().replace(/[^a-z0-9_-]+/gi, '-').replace(/-+/g, '-'); + return sanitized.replace(/^-+|-+$/g, '') || 'unknown'; +} + function createFetchTimeoutSignal(timeoutMs: number): AbortSignal | undefined { if (typeof AbortSignal !== 'undefined' && typeof AbortSignal.timeout === 'function') { return AbortSignal.timeout(timeoutMs); @@ -154,9 +159,10 @@ async function downloadSubtitleFromUrl(input: { : YOUTUBE_SUBTITLE_EXTENSIONS.has(`.${ext}`) ? ext : 'vtt'; + const safeSourceLanguage = sanitizeFilenameSegment(input.track.sourceLanguage); const targetPath = path.join( input.outputDir, - `${input.prefix}.${input.track.sourceLanguage}.${safeExt}`, + `${input.prefix}.${safeSourceLanguage}.${safeExt}`, ); const response = await fetch(input.track.downloadUrl, { signal: createFetchTimeoutSignal(YOUTUBE_DOWNLOAD_TIMEOUT_MS), diff --git a/src/core/services/youtube/track-probe.test.ts b/src/core/services/youtube/track-probe.test.ts index 0c5eda8..998aeaa 100644 --- a/src/core/services/youtube/track-probe.test.ts +++ b/src/core/services/youtube/track-probe.test.ts @@ -14,10 +14,12 @@ async function withTempDir(fn: (dir: string) => Promise): Promise { } } -function makeFakeYtDlpScript(dir: string, payload: unknown): void { +function makeFakeYtDlpScript(dir: string, payload: unknown, rawScript = false): void { const scriptPath = path.join(dir, 'yt-dlp'); const stdoutBody = typeof payload === 'string' ? payload : JSON.stringify(payload); - const script = `#!/usr/bin/env node + const script = rawScript + ? stdoutBody + : `#!/usr/bin/env node process.stdout.write(${JSON.stringify(stdoutBody)}); `; fs.writeFileSync(scriptPath, script, 'utf8'); @@ -27,11 +29,15 @@ process.stdout.write(${JSON.stringify(stdoutBody)}); fs.writeFileSync(scriptPath + '.cmd', `@echo off\r\nnode "${scriptPath}"\r\n`, 'utf8'); } -async function withFakeYtDlp(payload: unknown, fn: () => Promise): Promise { +async function withFakeYtDlp( + payload: unknown, + fn: () => Promise, + options: { rawScript?: boolean } = {}, +): Promise { return await withTempDir(async (root) => { const binDir = path.join(root, 'bin'); fs.mkdirSync(binDir, { recursive: true }); - makeFakeYtDlpScript(binDir, payload); + makeFakeYtDlpScript(binDir, payload, options.rawScript === true); const originalPath = process.env.PATH ?? ''; process.env.PATH = `${binDir}${path.delimiter}${originalPath}`; try { diff --git a/src/core/services/youtube/track-probe.ts b/src/core/services/youtube/track-probe.ts index c1a695a..16d4304 100644 --- a/src/core/services/youtube/track-probe.ts +++ b/src/core/services/youtube/track-probe.ts @@ -2,6 +2,8 @@ import { spawn } from 'node:child_process'; import type { YoutubeTrackOption } from '../../../types'; import { formatYoutubeTrackLabel, normalizeYoutubeLangCode, type YoutubeTrackKind } from './labels'; +const YOUTUBE_TRACK_PROBE_TIMEOUT_MS = 15_000; + export type YoutubeTrackProbeResult = { videoId: string; title: string; @@ -17,11 +19,19 @@ type YtDlpInfo = { automatic_captions?: Record; }; -function runCapture(command: string, args: string[]): Promise<{ stdout: string; stderr: string }> { +function runCapture( + command: string, + args: string[], + timeoutMs = YOUTUBE_TRACK_PROBE_TIMEOUT_MS, +): Promise<{ stdout: string; stderr: string }> { return new Promise((resolve, reject) => { const proc = spawn(command, args, { stdio: ['ignore', 'pipe', 'pipe'] }); let stdout = ''; let stderr = ''; + const timer = setTimeout(() => { + proc.kill(); + reject(new Error(`yt-dlp timed out after ${timeoutMs}ms`)); + }, timeoutMs); proc.stdout.setEncoding('utf8'); proc.stderr.setEncoding('utf8'); proc.stdout.on('data', (chunk) => { @@ -30,8 +40,12 @@ function runCapture(command: string, args: string[]): Promise<{ stdout: string; proc.stderr.on('data', (chunk) => { stderr += String(chunk); }); - proc.once('error', reject); + proc.once('error', (error) => { + clearTimeout(timer); + reject(error); + }); proc.once('close', (code) => { + clearTimeout(timer); if (code === 0) { resolve({ stdout, stderr }); return; diff --git a/src/main/runtime/youtube-flow.test.ts b/src/main/runtime/youtube-flow.test.ts index cedf13a..15b2d63 100644 --- a/src/main/runtime/youtube-flow.test.ts +++ b/src/main/runtime/youtube-flow.test.ts @@ -506,3 +506,69 @@ test('youtube flow cleans up paused picker state when opening the picker throws' assert.equal(warns.some((message) => message.includes('picker boom')), true); assert.equal(runtime.hasActiveSession(), false); }); + +test('youtube flow reports failure when the primary subtitle never binds', async () => { + const commands: Array> = []; + const osdMessages: string[] = []; + const warns: string[] = []; + + const runtime = createYoutubeFlowRuntime({ + probeYoutubeTracks: async () => ({ + videoId: 'video123', + title: 'Video 123', + tracks: [primaryTrack], + }), + acquireYoutubeSubtitleTracks: async () => new Map(), + acquireYoutubeSubtitleTrack: async () => ({ path: '/tmp/auto-ja-orig.vtt' }), + retimeYoutubePrimaryTrack: async ({ primaryPath }) => primaryPath, + startTokenizationWarmups: async () => {}, + waitForTokenizationReady: async () => {}, + waitForAnkiReady: async () => {}, + waitForPlaybackWindowReady: async () => {}, + waitForOverlayGeometryReady: async () => {}, + focusOverlayWindow: () => {}, + openPicker: async (payload) => { + queueMicrotask(() => { + void runtime.resolveActivePicker({ + sessionId: payload.sessionId, + action: 'use-selected', + primaryTrackId: primaryTrack.id, + secondaryTrackId: null, + }); + }); + return true; + }, + pauseMpv: () => {}, + resumeMpv: () => {}, + sendMpvCommand: (command) => { + commands.push(command); + }, + requestMpvProperty: async (name) => { + if (name === 'track-list') { + return []; + } + throw new Error(`unexpected property request: ${name}`); + }, + refreshCurrentSubtitle: () => { + throw new Error('should not refresh subtitle text on bind failure'); + }, + wait: async () => {}, + showMpvOsd: (text) => { + osdMessages.push(text); + }, + warn: (message) => { + warns.push(message); + }, + log: () => {}, + getYoutubeOutputDir: () => '/tmp', + }); + + await runtime.runYoutubePlaybackFlow({ url: 'https://example.com', mode: 'download' }); + + assert.equal( + commands.some((command) => command[0] === 'set_property' && command[1] === 'sid' && command[2] !== 'no'), + false, + ); + assert.deepEqual(osdMessages.slice(-1), ['Primary subtitles failed to load.']); + assert.equal(warns.some((message) => message.includes('Unable to bind downloaded primary subtitle track')), true); +}); diff --git a/src/main/runtime/youtube-flow.ts b/src/main/runtime/youtube-flow.ts index fbe5c1a..9f26060 100644 --- a/src/main/runtime/youtube-flow.ts +++ b/src/main/runtime/youtube-flow.ts @@ -258,6 +258,11 @@ async function injectDownloadedSubtitles( } } + if (primaryTrackId === null) { + deps.showMpvOsd('Primary subtitles failed to load.'); + return false; + } + const currentSubText = await deps.requestMpvProperty('sub-text'); if (typeof currentSubText === 'string' && currentSubText.trim().length > 0) { deps.refreshCurrentSubtitle(currentSubText); diff --git a/src/renderer/handlers/mouse.test.ts b/src/renderer/handlers/mouse.test.ts index 94ae796..f297200 100644 --- a/src/renderer/handlers/mouse.test.ts +++ b/src/renderer/handlers/mouse.test.ts @@ -21,6 +21,22 @@ function createClassList() { classes.delete(token); } }, + toggle: (token: string, force?: boolean) => { + if (force === undefined) { + if (classes.has(token)) { + classes.delete(token); + return false; + } + classes.add(token); + return true; + } + if (force) { + classes.add(token); + return true; + } + classes.delete(token); + return false; + }, contains: (token: string) => classes.has(token), }; } @@ -317,6 +333,74 @@ test('subtitle leave restores passthrough while embedded sidebar is open but not } }); +test('restorePointerInteractionState reapplies the secondary hover class from pointer location', async () => { + const ctx = createMouseTestContext(); + ctx.platform.shouldToggleMouseIgnore = true; + + const documentListeners = new Map void>>(); + const originalDocument = (globalThis as { document?: unknown }).document; + const originalWindow = (globalThis as { window?: unknown }).window; + + const secondarySubContainer = ctx.dom.secondarySubContainer as unknown as object; + const overlay = ctx.dom.overlay as unknown as { classList: ReturnType }; + + Object.defineProperty(globalThis, 'document', { + configurable: true, + value: { + addEventListener: (type: string, listener: (event: MouseEvent | PointerEvent) => void) => { + const listeners = documentListeners.get(type) ?? []; + listeners.push(listener); + documentListeners.set(type, listeners); + }, + elementFromPoint: () => secondarySubContainer, + }, + }); + Object.defineProperty(globalThis, 'window', { + configurable: true, + value: { + electronAPI: { + setIgnoreMouseEvents: () => {}, + }, + innerHeight: 1000, + getSelection: () => ({ rangeCount: 0, isCollapsed: true }), + }, + }); + + try { + const handlers = createMouseHandlers(ctx as never, { + modalStateReader: { + isAnySettingsModalOpen: () => false, + isAnyModalOpen: () => false, + }, + applyYPercent: () => {}, + getCurrentYPercent: () => 10, + persistSubtitlePositionPatch: () => {}, + getSubtitleHoverAutoPauseEnabled: () => false, + getYomitanPopupAutoPauseEnabled: () => false, + getPlaybackPaused: async () => false, + sendMpvCommand: () => {}, + }); + + handlers.setupPointerTracking(); + await handlers.handleSecondaryMouseEnter({ + clientX: 10, + clientY: 20, + } as unknown as MouseEvent); + handlers.restorePointerInteractionState(); + + overlay.classList.add('interactive'); + const mousemove = documentListeners.get('mousemove')?.[0]; + assert.ok(mousemove); + mousemove?.({ clientX: 10, clientY: 20 } as MouseEvent); + + assert.equal(ctx.state.isOverSubtitle, true); + assert.equal(ctx.dom.secondarySubContainer.classList.contains('secondary-sub-hover-active'), true); + } finally { + Object.defineProperty(globalThis, 'document', { configurable: true, value: originalDocument }); + Object.defineProperty(globalThis, 'window', { configurable: true, value: originalWindow }); + } +}); + test('pending hover pause check is ignored when mouse leaves before pause state resolves', async () => { const ctx = createMouseTestContext(); const mpvCommands: Array<(string | number)[]> = []; diff --git a/src/renderer/handlers/mouse.ts b/src/renderer/handlers/mouse.ts index 56cda6f..b26ec33 100644 --- a/src/renderer/handlers/mouse.ts +++ b/src/renderer/handlers/mouse.ts @@ -57,9 +57,10 @@ export function createMouseHandlers( ); ctx.state.isOverSubtitle = overPrimarySubtitle || overSecondarySubtitle; - if (!overSecondarySubtitle) { - ctx.dom.secondarySubContainer.classList.remove('secondary-sub-hover-active'); - } + ctx.dom.secondarySubContainer.classList.toggle( + 'secondary-sub-hover-active', + overSecondarySubtitle, + ); return ctx.state.isOverSubtitle; } diff --git a/stats/src/components/library/LibraryTab.tsx b/stats/src/components/library/LibraryTab.tsx index 2242e50..2595b46 100644 --- a/stats/src/components/library/LibraryTab.tsx +++ b/stats/src/components/library/LibraryTab.tsx @@ -1,7 +1,7 @@ import { useState, useMemo } from 'react'; import { useMediaLibrary } from '../../hooks/useMediaLibrary'; import { formatDuration, formatNumber } from '../../lib/formatters'; -import { groupMediaLibraryItems } from '../../lib/media-library-grouping'; +import { groupMediaLibraryItems, summarizeMediaLibraryGroups } from '../../lib/media-library-grouping'; import { CoverImage } from './CoverImage'; import { MediaCard } from './MediaCard'; import { MediaDetailView } from './MediaDetailView'; @@ -30,8 +30,7 @@ export function LibraryTab({ onNavigateToSession }: LibraryTabProps) { }); }, [media, search]); const grouped = useMemo(() => groupMediaLibraryItems(filtered), [filtered]); - - const totalMs = media.reduce((sum, m) => sum + m.totalActiveMs, 0); + const summary = useMemo(() => summarizeMediaLibraryGroups(grouped), [grouped]); if (selectedVideoId !== null) { return setSelectedVideoId(null)} />; @@ -51,8 +50,8 @@ export function LibraryTab({ onNavigateToSession }: LibraryTabProps) { className="flex-1 bg-ctp-surface0 border border-ctp-surface1 rounded-lg px-3 py-2 text-sm text-ctp-text placeholder:text-ctp-overlay2 focus:outline-none focus:border-ctp-blue" />
- {grouped.length} channel{grouped.length !== 1 ? 's' : ''} · {filtered.length} video - {filtered.length !== 1 ? 's' : ''} · {formatDuration(totalMs)} + {grouped.length} group{grouped.length !== 1 ? 's' : ''} · {summary.totalVideos} video + {summary.totalVideos !== 1 ? 's' : ''} · {formatDuration(summary.totalMs)}
diff --git a/stats/src/components/library/MediaCard.tsx b/stats/src/components/library/MediaCard.tsx index 45e63af..c2ddd0d 100644 --- a/stats/src/components/library/MediaCard.tsx +++ b/stats/src/components/library/MediaCard.tsx @@ -1,6 +1,5 @@ -import { CoverImage } from './CoverImage'; import { formatDuration, formatNumber } from '../../lib/formatters'; -import { resolveMediaArtworkUrl } from '../../lib/media-library-grouping'; +import { CoverImage } from './CoverImage'; import type { MediaLibraryItem } from '../../types/stats'; interface MediaCardProps { @@ -18,7 +17,6 @@ export function MediaCard({ item, onClick }: MediaCardProps) {
diff --git a/stats/src/lib/media-library-grouping.test.tsx b/stats/src/lib/media-library-grouping.test.tsx index 48ef6db..bc3c238 100644 --- a/stats/src/lib/media-library-grouping.test.tsx +++ b/stats/src/lib/media-library-grouping.test.tsx @@ -2,8 +2,13 @@ import assert from 'node:assert/strict'; import test from 'node:test'; import { renderToStaticMarkup } from 'react-dom/server'; import type { MediaLibraryItem } from '../types/stats'; -import { groupMediaLibraryItems, resolveMediaArtworkUrl } from './media-library-grouping'; +import { + groupMediaLibraryItems, + resolveMediaArtworkUrl, + summarizeMediaLibraryGroups, +} from './media-library-grouping'; import { CoverImage } from '../components/library/CoverImage'; +import { MediaCard } from '../components/library/MediaCard'; const youtubeEpisodeA: MediaLibraryItem = { videoId: 1, @@ -85,6 +90,16 @@ test('resolveMediaArtworkUrl prefers youtube thumbnails for video and channel im assert.equal(resolveMediaArtworkUrl(localVideo, 'channel'), null); }); +test('summarizeMediaLibraryGroups stays aligned with rendered group buckets', () => { + const groups = groupMediaLibraryItems([youtubeEpisodeA, localVideo, youtubeEpisodeB]); + const summary = summarizeMediaLibraryGroups(groups); + + assert.deepEqual(summary, { + totalMs: 29_000, + totalVideos: 3, + }); +}); + test('CoverImage renders explicit remote artwork when src is provided', () => { const markup = renderToStaticMarkup( { assert.match(markup, /src="https:\/\/i\.ytimg\.com\/vi\/yt-1\/hqdefault\.jpg"/); }); + +test('MediaCard uses the proxied cover endpoint instead of metadata artwork urls', () => { + const markup = renderToStaticMarkup( + {}} />, + ); + + assert.match(markup, /src="http:\/\/127\.0\.0\.1:6969\/api\/stats\/media\/1\/cover"/); + assert.doesNotMatch(markup, /https:\/\/i\.ytimg\.com\/vi\/yt-1\/hqdefault\.jpg/); +}); diff --git a/stats/src/lib/media-library-grouping.ts b/stats/src/lib/media-library-grouping.ts index 7d57203..61cad69 100644 --- a/stats/src/lib/media-library-grouping.ts +++ b/stats/src/lib/media-library-grouping.ts @@ -27,21 +27,35 @@ export function resolveMediaCoverApiUrl(videoId: number): string { return `${BASE_URL}/api/stats/media/${videoId}/cover`; } +export function summarizeMediaLibraryGroups(groups: MediaLibraryGroup[]): { + totalMs: number; + totalVideos: number; +} { + return groups.reduce( + (summary, group) => ({ + totalMs: summary.totalMs + group.totalActiveMs, + totalVideos: summary.totalVideos + group.items.length, + }), + { totalMs: 0, totalVideos: 0 }, + ); +} + export function groupMediaLibraryItems(items: MediaLibraryItem[]): MediaLibraryGroup[] { const groups = new Map(); for (const item of items) { - const key = item.channelId?.trim() || `video:${item.videoId}`; + const channelId = item.channelId?.trim() || null; + const channelName = item.channelName?.trim() || null; + const uploaderId = item.uploaderId?.trim() || null; + const videoTitle = item.videoTitle?.trim() || null; + const key = channelId || `video:${item.videoId}`; const title = - item.channelName?.trim() || - item.uploaderId?.trim() || - item.videoTitle?.trim() || - item.canonicalTitle; + channelName || uploaderId || videoTitle || item.canonicalTitle; const subtitle = - item.channelId?.trim() != null && item.channelId?.trim() !== '' - ? `${item.channelId}` - : item.videoTitle?.trim() && item.videoTitle !== item.canonicalTitle - ? item.videoTitle + channelId + ? channelId + : videoTitle && videoTitle !== item.canonicalTitle + ? videoTitle : null; const existing = groups.get(key); if (existing) {