mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-04-09 04:19:27 -07:00
Compare commits
3 Commits
6ae3888b53
...
90772f994c
| Author | SHA1 | Date | |
|---|---|---|---|
|
90772f994c
|
|||
|
c471bdf554
|
|||
|
f901433eea
|
@@ -5,7 +5,7 @@ status: In Progress
|
||||
assignee:
|
||||
- codex
|
||||
created_date: '2026-03-30 05:46'
|
||||
updated_date: '2026-03-30 08:34'
|
||||
updated_date: '2026-03-30 09:22'
|
||||
labels:
|
||||
- feature
|
||||
- overlay
|
||||
@@ -40,6 +40,8 @@ Add an in-session overlay modal that opens from a keybinding during active playb
|
||||
5. Write failing renderer and keyboard tests for modal open/close, split-pane interaction, keyboard controls, and degraded states.
|
||||
6. Implement playlist-browser modal markup, DOM/state, renderer composition, keyboard routing, and session-help labeling.
|
||||
7. Run targeted test lanes first, then the maintained verification gate relevant to the touched surfaces; update task notes/criteria as checks pass.
|
||||
|
||||
2026-03-30 CodeRabbit follow-up: 1) add failing runtime coverage for unreadable playlist-browser file stat failures, 2) add failing renderer coverage for stale snapshot UI reset on refresh failure/close, 3) add failing renderer coverage to block playlist-browser open when another modal already owns the overlay, 4) implement minimal fixes, 5) rerun targeted tests plus typecheck for touched surfaces.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
@@ -58,4 +60,8 @@ Repo gate blockers outside this feature: `bun run test:fast` hits existing Bun `
|
||||
2026-03-30: Follow-up subtitle regression fix. Pre-jump `sid=auto` was ineffective because mpv resolved it against the current episode before `playlist-play-index`. Local playlist jumps now set `sub-auto=fuzzy`, switch episodes, then schedule a delayed rearm of `sid=auto` and `secondary-sid=auto` so selection happens against the new file's tracks. Added failing-first runtime coverage for delayed local rearm and remote no-op behavior.
|
||||
|
||||
2026-03-30: Cleaned up playlist-browser runtime local-play subtitle-rearm flow by extracting focused helpers without changing behavior. Added public docs/readme coverage for the default `Ctrl+Alt+P` playlist browser keybinding and modal, plus changelog fragment `changes/260-playlist-browser.md`. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts`, `bun run typecheck`, `bun run docs:test`, `bun run docs:build`, `bun run changelog:lint`, `bun run build`.
|
||||
|
||||
2026-03-30: Pulled unresolved CodeRabbit review threads for PR #37. Actionable set is three items: unreadable-file stat error handling in playlist-browser runtime, stale playlist-browser DOM after failed refresh/close, and missing modal-ownership guard before opening the playlist-browser overlay. Proceeding test-first for each.
|
||||
|
||||
2026-03-30: Addressed current CodeRabbit follow-up findings for PR #37. Fixed playlist-browser unreadable-file stat handling, stale playlist-browser DOM reset on refresh failure/close, modal-ownership guard before opening the playlist-browser overlay, async rejection surfacing for PLAYLIST_BROWSER_OPEN IPC commands, overlay bootstrap before playlist-browser open dispatch, texthooker option normalization in the mpv plugin, and superseded local subtitle-rearm suppression. Added targeted regressions plus new playlist-browser-open helper coverage. Verification: `bun test src/main/runtime/playlist-browser-runtime.test.ts src/main/runtime/playlist-browser-open.test.ts src/core/services/ipc-command.test.ts src/renderer/modals/playlist-browser.test.ts`, `lua scripts/test-plugin-start-gate.lua`, `bun run typecheck`, `bun run build`.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
---
|
||||
id: TASK-260
|
||||
title: >-
|
||||
Fix macOS overlay subtitle sidebar passthrough without requiring a subtitle
|
||||
hover cycle
|
||||
status: Done
|
||||
assignee:
|
||||
- '@codex'
|
||||
created_date: '2026-03-31 00:58'
|
||||
updated_date: '2026-03-31 01:01'
|
||||
labels:
|
||||
- bug
|
||||
- macos
|
||||
- overlay
|
||||
- subtitle-sidebar
|
||||
- passthrough
|
||||
dependencies: []
|
||||
references:
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/renderer/modals/subtitle-sidebar.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/renderer/overlay-mouse-ignore.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/renderer/handlers/mouse.ts
|
||||
- /Users/sudacode/projects/japanese/SubMiner/src/main/overlay-runtime.ts
|
||||
- >-
|
||||
/Users/sudacode/projects/japanese/SubMiner/src/core/services/overlay-visibility.ts
|
||||
documentation:
|
||||
- docs/workflow/verification.md
|
||||
priority: high
|
||||
---
|
||||
|
||||
## Description
|
||||
|
||||
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||
On macOS, opening the overlay-layout subtitle sidebar should allow click-through outside the sidebar immediately. Users should not need to first hover subtitle content before passthrough/click-through starts working, including when no subtitle line is currently visible.
|
||||
<!-- SECTION:DESCRIPTION:END -->
|
||||
|
||||
## Acceptance Criteria
|
||||
<!-- AC:BEGIN -->
|
||||
- [x] #1 With the overlay-layout subtitle sidebar open on macOS, areas outside the sidebar pass clicks through immediately after open without requiring a prior subtitle hover.
|
||||
- [x] #2 When no subtitle line is currently visible, opening the subtitle sidebar still leaves non-sidebar overlay regions click-through on macOS.
|
||||
- [x] #3 Regression coverage exercises the first-open/idle passthrough path so overlay interactivity does not depend on a later hover cycle.
|
||||
<!-- AC:END -->
|
||||
|
||||
## Implementation Plan
|
||||
|
||||
<!-- SECTION:PLAN:BEGIN -->
|
||||
1. Add/adjust focused overlay visibility regressions for the tracked macOS visible overlay so the default idle state stays click-through instead of forcing mouse interaction.
|
||||
2. Update main-process visible overlay visibility sync to keep the tracked macOS overlay passive by default and let renderer hover/sidebar state opt into interaction.
|
||||
3. Run focused verification for overlay visibility and any dependent runtime tests, then update task notes/criteria/final summary with the confirmed outcome.
|
||||
<!-- SECTION:PLAN:END -->
|
||||
|
||||
## Implementation Notes
|
||||
|
||||
<!-- SECTION:NOTES:BEGIN -->
|
||||
Investigation points to a main-process override on macOS: renderer sidebar open path already requests mouse passthrough outside the panel, but visible-overlay visibility sync still hard-sets the tracked overlay window interactive on macOS (`mouse-ignore:false`). Window-tracker focus/visibility resync can therefore undo renderer passthrough until a later hover cycle re-applies it.
|
||||
|
||||
Added a failing regression in `src/core/services/overlay-visibility.test.ts` showing the tracked macOS visible overlay was still forced interactive by main-process visibility sync (`mouse-ignore:false`) instead of staying forwarded click-through.
|
||||
|
||||
Updated `src/core/services/overlay-visibility.ts` so tracked macOS visible overlays now default to `setIgnoreMouseEvents(true, { forward: true })`, matching the renderer-side passthrough model and preventing window-tracker/focus resync from undoing idle sidebar clickthrough.
|
||||
<!-- SECTION:NOTES:END -->
|
||||
|
||||
## Final Summary
|
||||
|
||||
<!-- SECTION:FINAL_SUMMARY:BEGIN -->
|
||||
Fixed the macOS subtitle-sidebar passthrough regression by changing tracked visible-overlay startup/visibility sync to stay click-through by default in the main process. Previously `updateVisibleOverlayVisibility` forced the macOS overlay window interactive, which could override renderer sidebar passthrough until a later hover cycle repaired it. Added a regression in `src/core/services/overlay-visibility.test.ts` and verified with `bun test src/core/services/overlay-visibility.test.ts`, `bun test src/renderer/modals/subtitle-sidebar.test.ts src/renderer/handlers/mouse.test.ts`, and `bun run typecheck`.
|
||||
<!-- SECTION:FINAL_SUMMARY:END -->
|
||||
5
changes/261-macos-overlay-passthrough.md
Normal file
5
changes/261-macos-overlay-passthrough.md
Normal file
@@ -0,0 +1,5 @@
|
||||
type: fixed
|
||||
area: overlay
|
||||
|
||||
- Keep tracked macOS visible overlays click-through by default so subtitle sidebar passthrough works immediately without requiring a subtitle hover cycle first.
|
||||
- Add regression coverage for the macOS visible-overlay passthrough default.
|
||||
@@ -34,6 +34,17 @@ function M.create(ctx)
|
||||
return options_helper.coerce_bool(raw_pause_until_ready, false)
|
||||
end
|
||||
|
||||
local function resolve_texthooker_enabled(override_value)
|
||||
if override_value ~= nil then
|
||||
return options_helper.coerce_bool(override_value, false)
|
||||
end
|
||||
local raw_texthooker_enabled = opts.texthooker_enabled
|
||||
if raw_texthooker_enabled == nil then
|
||||
raw_texthooker_enabled = opts["texthooker-enabled"]
|
||||
end
|
||||
return options_helper.coerce_bool(raw_texthooker_enabled, false)
|
||||
end
|
||||
|
||||
local function resolve_pause_until_ready_timeout_seconds()
|
||||
local raw_timeout_seconds = opts.auto_start_pause_until_ready_timeout_seconds
|
||||
if raw_timeout_seconds == nil then
|
||||
@@ -192,10 +203,7 @@ function M.create(ctx)
|
||||
table.insert(args, "--hide-visible-overlay")
|
||||
end
|
||||
|
||||
local texthooker_enabled = overrides.texthooker_enabled
|
||||
if texthooker_enabled == nil then
|
||||
texthooker_enabled = opts.texthooker_enabled
|
||||
end
|
||||
local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled)
|
||||
if texthooker_enabled then
|
||||
table.insert(args, "--texthooker")
|
||||
end
|
||||
@@ -296,10 +304,7 @@ function M.create(ctx)
|
||||
return
|
||||
end
|
||||
|
||||
local texthooker_enabled = overrides.texthooker_enabled
|
||||
if texthooker_enabled == nil then
|
||||
texthooker_enabled = opts.texthooker_enabled
|
||||
end
|
||||
local texthooker_enabled = resolve_texthooker_enabled(overrides.texthooker_enabled)
|
||||
local socket_path = overrides.socket_path or opts.socket_path
|
||||
local should_pause_until_ready = (
|
||||
overrides.auto_start_trigger == true
|
||||
@@ -498,7 +503,7 @@ function M.create(ctx)
|
||||
end
|
||||
end)
|
||||
|
||||
if opts.texthooker_enabled then
|
||||
if resolve_texthooker_enabled(nil) then
|
||||
ensure_texthooker_running(function() end)
|
||||
end
|
||||
end)
|
||||
|
||||
@@ -531,6 +531,31 @@ do
|
||||
)
|
||||
end
|
||||
|
||||
do
|
||||
local recorded, err = run_plugin_scenario({
|
||||
process_list = "",
|
||||
option_overrides = {
|
||||
binary_path = binary_path,
|
||||
auto_start = "yes",
|
||||
auto_start_visible_overlay = "yes",
|
||||
auto_start_pause_until_ready = "no",
|
||||
socket_path = "/tmp/subminer-socket",
|
||||
texthooker_enabled = "no",
|
||||
},
|
||||
input_ipc_server = "/tmp/subminer-socket",
|
||||
media_title = "Random Movie",
|
||||
files = {
|
||||
[binary_path] = true,
|
||||
},
|
||||
})
|
||||
assert_true(recorded ~= nil, "plugin failed to load for disabled texthooker auto-start scenario: " .. tostring(err))
|
||||
fire_event(recorded, "file-loaded")
|
||||
local start_call = find_start_call(recorded.async_calls)
|
||||
assert_true(start_call ~= nil, "disabled texthooker auto-start should still issue --start command")
|
||||
assert_true(not call_has_arg(start_call, "--texthooker"), "disabled texthooker should not include --texthooker on --start")
|
||||
assert_true(find_control_call(recorded.async_calls, "--texthooker") == nil, "disabled texthooker should not issue a helper texthooker command")
|
||||
end
|
||||
|
||||
do
|
||||
local recorded, err = run_plugin_scenario({
|
||||
process_list = "",
|
||||
|
||||
@@ -114,14 +114,28 @@ test('handleMpvCommandFromIpc dispatches special youtube picker open command', (
|
||||
assert.deepEqual(osd, []);
|
||||
});
|
||||
|
||||
test('handleMpvCommandFromIpc dispatches special playlist browser open command', () => {
|
||||
test('handleMpvCommandFromIpc dispatches special playlist browser open command', async () => {
|
||||
const { options, calls, sentCommands, osd } = createOptions();
|
||||
handleMpvCommandFromIpc(['__playlist-browser-open'], options);
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
assert.deepEqual(calls, ['playlist-browser']);
|
||||
assert.deepEqual(sentCommands, []);
|
||||
assert.deepEqual(osd, []);
|
||||
});
|
||||
|
||||
test('handleMpvCommandFromIpc surfaces playlist browser open rejections via mpv osd', async () => {
|
||||
const { options, osd } = createOptions({
|
||||
openPlaylistBrowser: async () => {
|
||||
throw new Error('overlay failed');
|
||||
},
|
||||
});
|
||||
|
||||
handleMpvCommandFromIpc(['__playlist-browser-open'], options);
|
||||
await new Promise((resolve) => setImmediate(resolve));
|
||||
|
||||
assert.deepEqual(osd, ['Playlist browser failed: overlay failed']);
|
||||
});
|
||||
|
||||
test('handleMpvCommandFromIpc does not forward commands while disconnected', () => {
|
||||
const { options, sentCommands, osd } = createOptions({
|
||||
isMpvConnected: () => false,
|
||||
|
||||
@@ -100,7 +100,12 @@ export function handleMpvCommandFromIpc(
|
||||
}
|
||||
|
||||
if (first === options.specialCommands.PLAYLIST_BROWSER_OPEN) {
|
||||
void options.openPlaylistBrowser();
|
||||
Promise.resolve()
|
||||
.then(() => options.openPlaylistBrowser())
|
||||
.catch((error) => {
|
||||
const message = error instanceof Error ? error.message : String(error);
|
||||
options.showMpvOsd(`Playlist browser failed: ${message}`);
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -238,7 +238,7 @@ test('visible overlay stays hidden while a modal window is active', () => {
|
||||
assert.ok(!calls.includes('update-bounds'));
|
||||
});
|
||||
|
||||
test('macOS tracked visible overlay stays visible without passively stealing focus', () => {
|
||||
test('macOS tracked visible overlay stays click-through without passively stealing focus', () => {
|
||||
const { window, calls } = createMainWindowRecorder();
|
||||
const tracker: WindowTrackerStub = {
|
||||
isTracking: () => true,
|
||||
@@ -270,7 +270,7 @@ test('macOS tracked visible overlay stays visible without passively stealing foc
|
||||
isWindowsPlatform: false,
|
||||
} as never);
|
||||
|
||||
assert.ok(calls.includes('mouse-ignore:false:plain'));
|
||||
assert.ok(calls.includes('mouse-ignore:true:forward'));
|
||||
assert.ok(calls.includes('show'));
|
||||
assert.ok(!calls.includes('focus'));
|
||||
});
|
||||
|
||||
@@ -37,7 +37,7 @@ export function updateVisibleOverlayVisibility(args: {
|
||||
|
||||
const showPassiveVisibleOverlay = (): void => {
|
||||
const forceMousePassthrough = args.forceMousePassthrough === true;
|
||||
if (args.isWindowsPlatform || forceMousePassthrough) {
|
||||
if (args.isMacOSPlatform || args.isWindowsPlatform || forceMousePassthrough) {
|
||||
mainWindow.setIgnoreMouseEvents(true, { forward: true });
|
||||
} else {
|
||||
mainWindow.setIgnoreMouseEvents(false);
|
||||
|
||||
@@ -31,6 +31,7 @@ import {
|
||||
screen,
|
||||
} from 'electron';
|
||||
import { applyControllerConfigUpdate } from './main/controller-config-update.js';
|
||||
import { openPlaylistBrowser as openPlaylistBrowserRuntime } from './main/runtime/playlist-browser-open';
|
||||
import { createDiscordRpcClient } from './main/runtime/discord-rpc-client.js';
|
||||
import { mergeAiConfig } from './ai/config';
|
||||
|
||||
@@ -1941,8 +1942,12 @@ function openPlaylistBrowser(): void {
|
||||
showMpvOsd('Playlist browser requires active playback.');
|
||||
return;
|
||||
}
|
||||
const opened = sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, {
|
||||
restoreOnModalClose: 'playlist-browser',
|
||||
const opened = openPlaylistBrowserRuntime({
|
||||
ensureOverlayStartupPrereqs: () => ensureOverlayStartupPrereqs(),
|
||||
ensureOverlayWindowsReadyForVisibilityActions: () =>
|
||||
ensureOverlayWindowsReadyForVisibilityActions(),
|
||||
sendToActiveOverlayWindow: (channel, payload, runtimeOptions) =>
|
||||
sendToActiveOverlayWindow(channel, payload, runtimeOptions),
|
||||
});
|
||||
if (!opened) {
|
||||
showMpvOsd('Playlist browser overlay unavailable.');
|
||||
|
||||
28
src/main/runtime/playlist-browser-open.test.ts
Normal file
28
src/main/runtime/playlist-browser-open.test.ts
Normal file
@@ -0,0 +1,28 @@
|
||||
import assert from 'node:assert/strict';
|
||||
import test from 'node:test';
|
||||
import { IPC_CHANNELS } from '../../shared/ipc/contracts';
|
||||
import { openPlaylistBrowser } from './playlist-browser-open';
|
||||
|
||||
test('playlist browser open bootstraps overlay runtime before dispatching the modal event', () => {
|
||||
const calls: string[] = [];
|
||||
|
||||
const opened = openPlaylistBrowser({
|
||||
ensureOverlayStartupPrereqs: () => {
|
||||
calls.push('prereqs');
|
||||
},
|
||||
ensureOverlayWindowsReadyForVisibilityActions: () => {
|
||||
calls.push('windows');
|
||||
},
|
||||
sendToActiveOverlayWindow: (channel, payload, runtimeOptions) => {
|
||||
calls.push(`send:${channel}`);
|
||||
assert.equal(payload, undefined);
|
||||
assert.deepEqual(runtimeOptions, {
|
||||
restoreOnModalClose: 'playlist-browser',
|
||||
});
|
||||
return true;
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(opened, true);
|
||||
assert.deepEqual(calls, ['prereqs', 'windows', `send:${IPC_CHANNELS.event.playlistBrowserOpen}`]);
|
||||
});
|
||||
23
src/main/runtime/playlist-browser-open.ts
Normal file
23
src/main/runtime/playlist-browser-open.ts
Normal file
@@ -0,0 +1,23 @@
|
||||
import type { OverlayHostedModal } from '../../shared/ipc/contracts';
|
||||
import { IPC_CHANNELS } from '../../shared/ipc/contracts';
|
||||
|
||||
const PLAYLIST_BROWSER_MODAL: OverlayHostedModal = 'playlist-browser';
|
||||
|
||||
export function openPlaylistBrowser(deps: {
|
||||
ensureOverlayStartupPrereqs: () => void;
|
||||
ensureOverlayWindowsReadyForVisibilityActions: () => void;
|
||||
sendToActiveOverlayWindow: (
|
||||
channel: string,
|
||||
payload?: unknown,
|
||||
runtimeOptions?: {
|
||||
restoreOnModalClose?: OverlayHostedModal;
|
||||
preferModalWindow?: boolean;
|
||||
},
|
||||
) => boolean;
|
||||
}): boolean {
|
||||
deps.ensureOverlayStartupPrereqs();
|
||||
deps.ensureOverlayWindowsReadyForVisibilityActions();
|
||||
return deps.sendToActiveOverlayWindow(IPC_CHANNELS.event.playlistBrowserOpen, undefined, {
|
||||
restoreOnModalClose: PLAYLIST_BROWSER_MODAL,
|
||||
});
|
||||
}
|
||||
@@ -249,6 +249,41 @@ test('playlist-browser mutation runtimes mutate queue and return refreshed snaps
|
||||
);
|
||||
});
|
||||
|
||||
test('appendPlaylistBrowserFileRuntime returns an error result when statSync throws', async () => {
|
||||
const dir = createTempVideoDir();
|
||||
const episode1 = path.join(dir, 'Show - S01E01.mkv');
|
||||
fs.writeFileSync(episode1, '');
|
||||
|
||||
const mutableFs = fs as typeof fs & { statSync: typeof fs.statSync };
|
||||
const originalStatSync = mutableFs.statSync;
|
||||
mutableFs.statSync = ((targetPath: fs.PathLike) => {
|
||||
if (path.resolve(String(targetPath)) === episode1) {
|
||||
throw new Error('EACCES');
|
||||
}
|
||||
return originalStatSync(targetPath);
|
||||
}) as typeof fs.statSync;
|
||||
|
||||
try {
|
||||
const result = await appendPlaylistBrowserFileRuntime(
|
||||
{
|
||||
getMpvClient: () =>
|
||||
createFakeMpvClient({
|
||||
currentVideoPath: episode1,
|
||||
playlist: [{ filename: episode1, current: true }],
|
||||
}),
|
||||
},
|
||||
episode1,
|
||||
);
|
||||
|
||||
assert.deepEqual(result, {
|
||||
ok: false,
|
||||
message: 'Playlist browser file is not readable.',
|
||||
});
|
||||
} finally {
|
||||
mutableFs.statSync = originalStatSync;
|
||||
}
|
||||
});
|
||||
|
||||
test('movePlaylistBrowserIndexRuntime rejects top and bottom boundary moves', async () => {
|
||||
const dir = createTempVideoDir();
|
||||
const episode1 = path.join(dir, 'Show - S01E01.mkv');
|
||||
@@ -324,3 +359,52 @@ test('playPlaylistBrowserIndexRuntime skips local subtitle reset for remote play
|
||||
assert.deepEqual(mpvClient.getCommands().slice(-1), [['playlist-play-index', 1]]);
|
||||
assert.equal(scheduled.length, 0);
|
||||
});
|
||||
|
||||
test('playPlaylistBrowserIndexRuntime ignores superseded local subtitle rearm callbacks', async () => {
|
||||
const dir = createTempVideoDir();
|
||||
const episode1 = path.join(dir, 'Show - S01E01.mkv');
|
||||
const episode2 = path.join(dir, 'Show - S01E02.mkv');
|
||||
const episode3 = path.join(dir, 'Show - S01E03.mkv');
|
||||
fs.writeFileSync(episode1, '');
|
||||
fs.writeFileSync(episode2, '');
|
||||
fs.writeFileSync(episode3, '');
|
||||
|
||||
const scheduled: Array<() => void> = [];
|
||||
const mpvClient = createFakeMpvClient({
|
||||
currentVideoPath: episode1,
|
||||
playlist: [
|
||||
{ filename: episode1, current: true, title: 'Episode 1' },
|
||||
{ filename: episode2, title: 'Episode 2' },
|
||||
{ filename: episode3, title: 'Episode 3' },
|
||||
],
|
||||
});
|
||||
|
||||
const deps = {
|
||||
getMpvClient: () => mpvClient,
|
||||
schedule: (callback: () => void) => {
|
||||
scheduled.push(callback);
|
||||
},
|
||||
};
|
||||
|
||||
const firstPlay = await playPlaylistBrowserIndexRuntime(deps, 1);
|
||||
const secondPlay = await playPlaylistBrowserIndexRuntime(deps, 2);
|
||||
|
||||
assert.equal(firstPlay.ok, true);
|
||||
assert.equal(secondPlay.ok, true);
|
||||
assert.equal(scheduled.length, 2);
|
||||
|
||||
scheduled[0]?.();
|
||||
scheduled[1]?.();
|
||||
|
||||
assert.deepEqual(
|
||||
mpvClient.getCommands().slice(-6),
|
||||
[
|
||||
['set_property', 'sub-auto', 'fuzzy'],
|
||||
['playlist-play-index', 1],
|
||||
['set_property', 'sub-auto', 'fuzzy'],
|
||||
['playlist-play-index', 2],
|
||||
['set_property', 'sid', 'auto'],
|
||||
['set_property', 'secondary-sid', 'auto'],
|
||||
],
|
||||
);
|
||||
});
|
||||
|
||||
@@ -30,6 +30,8 @@ export type PlaylistBrowserRuntimeDeps = {
|
||||
schedule?: (callback: () => void, delayMs: number) => void;
|
||||
};
|
||||
|
||||
const pendingLocalSubtitleSelectionRearms = new WeakMap<MpvPlaylistBrowserClientLike, number>();
|
||||
|
||||
function trimToNull(value: unknown): string | null {
|
||||
if (typeof value !== 'string') return null;
|
||||
const trimmed = value.trim();
|
||||
@@ -219,15 +221,26 @@ function prepareLocalSubtitleAutoload(client: MpvPlaylistBrowserClientLike): voi
|
||||
client.send({ command: ['set_property', 'sub-auto', 'fuzzy'] });
|
||||
}
|
||||
|
||||
function isLocalPlaylistItem(item: PlaylistBrowserQueueItem | null | undefined): item is PlaylistBrowserQueueItem {
|
||||
function isLocalPlaylistItem(
|
||||
item: PlaylistBrowserQueueItem | null | undefined,
|
||||
): item is PlaylistBrowserQueueItem & { path: string } {
|
||||
return Boolean(item?.path && !isRemoteMediaPath(item.path));
|
||||
}
|
||||
|
||||
function scheduleLocalSubtitleSelectionRearm(
|
||||
deps: PlaylistBrowserRuntimeDeps,
|
||||
client: MpvPlaylistBrowserClientLike,
|
||||
expectedPath: string,
|
||||
): void {
|
||||
const nextToken = (pendingLocalSubtitleSelectionRearms.get(client) ?? 0) + 1;
|
||||
pendingLocalSubtitleSelectionRearms.set(client, nextToken);
|
||||
(deps.schedule ?? setTimeout)(() => {
|
||||
if (pendingLocalSubtitleSelectionRearms.get(client) !== nextToken) return;
|
||||
pendingLocalSubtitleSelectionRearms.delete(client);
|
||||
const currentPath = trimToNull(client.currentVideoPath);
|
||||
if (currentPath && path.resolve(currentPath) !== expectedPath) {
|
||||
return;
|
||||
}
|
||||
rearmLocalSubtitleSelection(client);
|
||||
}, 400);
|
||||
}
|
||||
@@ -241,7 +254,16 @@ export async function appendPlaylistBrowserFileRuntime(
|
||||
return client;
|
||||
}
|
||||
const resolvedPath = path.resolve(filePath);
|
||||
if (!fs.existsSync(resolvedPath) || !fs.statSync(resolvedPath).isFile()) {
|
||||
let stats: fs.Stats;
|
||||
try {
|
||||
stats = fs.statSync(resolvedPath);
|
||||
} catch {
|
||||
return {
|
||||
ok: false,
|
||||
message: 'Playlist browser file is not readable.',
|
||||
};
|
||||
}
|
||||
if (!stats.isFile()) {
|
||||
return {
|
||||
ok: false,
|
||||
message: 'Playlist browser file is not readable.',
|
||||
@@ -267,7 +289,7 @@ export async function playPlaylistBrowserIndexRuntime(
|
||||
}
|
||||
result.client.send({ command: ['playlist-play-index', index] });
|
||||
if (isLocalPlaylistItem(targetItem)) {
|
||||
scheduleLocalSubtitleSelectionRearm(deps, result.client);
|
||||
scheduleLocalSubtitleSelectionRearm(deps, result.client, path.resolve(targetItem.path));
|
||||
}
|
||||
return buildMutationResult(`Playing playlist item ${index + 1}`, deps);
|
||||
}
|
||||
|
||||
@@ -428,3 +428,241 @@ test('playlist browser keeps modal open when playing selected queue item fails',
|
||||
Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
|
||||
}
|
||||
});
|
||||
|
||||
test('playlist browser refresh failure clears stale rendered rows and reports the error', async () => {
|
||||
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
|
||||
const previousWindow = globals.window;
|
||||
const previousDocument = globals.document;
|
||||
const notifications: string[] = [];
|
||||
let refreshShouldFail = false;
|
||||
|
||||
Object.defineProperty(globalThis, 'window', {
|
||||
configurable: true,
|
||||
value: {
|
||||
electronAPI: {
|
||||
getPlaylistBrowserSnapshot: async () => {
|
||||
if (refreshShouldFail) {
|
||||
throw new Error('snapshot failed');
|
||||
}
|
||||
return createSnapshot();
|
||||
},
|
||||
notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
|
||||
notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
|
||||
focusMainWindow: async () => {},
|
||||
setIgnoreMouseEvents: () => {},
|
||||
appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
} as unknown as ElectronAPI,
|
||||
focus: () => {},
|
||||
},
|
||||
});
|
||||
Object.defineProperty(globalThis, 'document', {
|
||||
configurable: true,
|
||||
value: {
|
||||
createElement: () => createPlaylistRow(),
|
||||
},
|
||||
});
|
||||
|
||||
try {
|
||||
const state = createRendererState();
|
||||
const playlistBrowserTitle = createFakeElement();
|
||||
const playlistBrowserStatus = createFakeElement();
|
||||
const directoryList = createListStub();
|
||||
const playlistList = createListStub();
|
||||
const ctx = {
|
||||
state,
|
||||
platform: {
|
||||
shouldToggleMouseIgnore: false,
|
||||
},
|
||||
dom: {
|
||||
overlay: {
|
||||
classList: createClassList(),
|
||||
focus: () => {},
|
||||
},
|
||||
playlistBrowserModal: createFakeElement(),
|
||||
playlistBrowserTitle,
|
||||
playlistBrowserStatus,
|
||||
playlistBrowserDirectoryList: directoryList,
|
||||
playlistBrowserPlaylistList: playlistList,
|
||||
playlistBrowserClose: createFakeElement(),
|
||||
},
|
||||
};
|
||||
|
||||
const modal = createPlaylistBrowserModal(ctx as never, {
|
||||
modalStateReader: { isAnyModalOpen: () => false },
|
||||
syncSettingsModalSubtitleSuppression: () => {},
|
||||
});
|
||||
|
||||
await modal.openPlaylistBrowserModal();
|
||||
assert.equal(directoryList.children.length, 2);
|
||||
assert.equal(playlistList.children.length, 2);
|
||||
|
||||
refreshShouldFail = true;
|
||||
await modal.refreshSnapshot();
|
||||
|
||||
assert.equal(state.playlistBrowserSnapshot, null);
|
||||
assert.equal(directoryList.children.length, 0);
|
||||
assert.equal(playlistList.children.length, 0);
|
||||
assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser');
|
||||
assert.equal(playlistBrowserStatus.textContent, 'snapshot failed');
|
||||
assert.equal(playlistBrowserStatus.classList.contains('error'), true);
|
||||
assert.deepEqual(notifications, ['open:playlist-browser']);
|
||||
} finally {
|
||||
Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
|
||||
Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
|
||||
}
|
||||
});
|
||||
|
||||
test('playlist browser close clears rendered snapshot ui', async () => {
|
||||
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
|
||||
const previousWindow = globals.window;
|
||||
const previousDocument = globals.document;
|
||||
const notifications: string[] = [];
|
||||
|
||||
Object.defineProperty(globalThis, 'window', {
|
||||
configurable: true,
|
||||
value: {
|
||||
electronAPI: {
|
||||
getPlaylistBrowserSnapshot: async () => createSnapshot(),
|
||||
notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
|
||||
notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
|
||||
focusMainWindow: async () => {},
|
||||
setIgnoreMouseEvents: () => {},
|
||||
appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
} as unknown as ElectronAPI,
|
||||
focus: () => {},
|
||||
},
|
||||
});
|
||||
Object.defineProperty(globalThis, 'document', {
|
||||
configurable: true,
|
||||
value: {
|
||||
createElement: () => createPlaylistRow(),
|
||||
},
|
||||
});
|
||||
|
||||
try {
|
||||
const state = createRendererState();
|
||||
const playlistBrowserTitle = createFakeElement();
|
||||
const playlistBrowserStatus = createFakeElement();
|
||||
const directoryList = createListStub();
|
||||
const playlistList = createListStub();
|
||||
const ctx = {
|
||||
state,
|
||||
platform: {
|
||||
shouldToggleMouseIgnore: false,
|
||||
},
|
||||
dom: {
|
||||
overlay: {
|
||||
classList: createClassList(),
|
||||
focus: () => {},
|
||||
},
|
||||
playlistBrowserModal: createFakeElement(),
|
||||
playlistBrowserTitle,
|
||||
playlistBrowserStatus,
|
||||
playlistBrowserDirectoryList: directoryList,
|
||||
playlistBrowserPlaylistList: playlistList,
|
||||
playlistBrowserClose: createFakeElement(),
|
||||
},
|
||||
};
|
||||
|
||||
const modal = createPlaylistBrowserModal(ctx as never, {
|
||||
modalStateReader: { isAnyModalOpen: () => false },
|
||||
syncSettingsModalSubtitleSuppression: () => {},
|
||||
});
|
||||
|
||||
await modal.openPlaylistBrowserModal();
|
||||
assert.equal(directoryList.children.length, 2);
|
||||
assert.equal(playlistList.children.length, 2);
|
||||
|
||||
modal.closePlaylistBrowserModal();
|
||||
|
||||
assert.equal(state.playlistBrowserSnapshot, null);
|
||||
assert.equal(state.playlistBrowserStatus, '');
|
||||
assert.equal(directoryList.children.length, 0);
|
||||
assert.equal(playlistList.children.length, 0);
|
||||
assert.equal(playlistBrowserTitle.textContent, 'Playlist Browser');
|
||||
assert.equal(playlistBrowserStatus.textContent, '');
|
||||
assert.deepEqual(notifications, ['open:playlist-browser', 'close:playlist-browser']);
|
||||
} finally {
|
||||
Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
|
||||
Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
|
||||
}
|
||||
});
|
||||
|
||||
test('playlist browser open is ignored while another modal is already open', async () => {
|
||||
const globals = globalThis as typeof globalThis & { window?: unknown; document?: unknown };
|
||||
const previousWindow = globals.window;
|
||||
const previousDocument = globals.document;
|
||||
const notifications: string[] = [];
|
||||
let snapshotCalls = 0;
|
||||
|
||||
Object.defineProperty(globalThis, 'window', {
|
||||
configurable: true,
|
||||
value: {
|
||||
electronAPI: {
|
||||
getPlaylistBrowserSnapshot: async () => {
|
||||
snapshotCalls += 1;
|
||||
return createSnapshot();
|
||||
},
|
||||
notifyOverlayModalOpened: (modal: string) => notifications.push(`open:${modal}`),
|
||||
notifyOverlayModalClosed: (modal: string) => notifications.push(`close:${modal}`),
|
||||
focusMainWindow: async () => {},
|
||||
setIgnoreMouseEvents: () => {},
|
||||
appendPlaylistBrowserFile: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
playPlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
removePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
movePlaylistBrowserIndex: async () => ({ ok: true, message: 'ok', snapshot: createSnapshot() }),
|
||||
} as unknown as ElectronAPI,
|
||||
focus: () => {},
|
||||
},
|
||||
});
|
||||
Object.defineProperty(globalThis, 'document', {
|
||||
configurable: true,
|
||||
value: {
|
||||
createElement: () => createPlaylistRow(),
|
||||
},
|
||||
});
|
||||
|
||||
try {
|
||||
const state = createRendererState();
|
||||
const overlay = {
|
||||
classList: createClassList(),
|
||||
focus: () => {},
|
||||
};
|
||||
const ctx = {
|
||||
state,
|
||||
platform: {
|
||||
shouldToggleMouseIgnore: false,
|
||||
},
|
||||
dom: {
|
||||
overlay,
|
||||
playlistBrowserModal: createFakeElement(),
|
||||
playlistBrowserTitle: createFakeElement(),
|
||||
playlistBrowserStatus: createFakeElement(),
|
||||
playlistBrowserDirectoryList: createListStub(),
|
||||
playlistBrowserPlaylistList: createListStub(),
|
||||
playlistBrowserClose: createFakeElement(),
|
||||
},
|
||||
};
|
||||
|
||||
const modal = createPlaylistBrowserModal(ctx as never, {
|
||||
modalStateReader: { isAnyModalOpen: () => true },
|
||||
syncSettingsModalSubtitleSuppression: () => {},
|
||||
});
|
||||
|
||||
await modal.openPlaylistBrowserModal();
|
||||
|
||||
assert.equal(state.playlistBrowserModalOpen, false);
|
||||
assert.equal(snapshotCalls, 0);
|
||||
assert.equal(overlay.classList.contains('interactive'), false);
|
||||
assert.deepEqual(notifications, []);
|
||||
} finally {
|
||||
Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
|
||||
Object.defineProperty(globalThis, 'document', { configurable: true, value: previousDocument });
|
||||
}
|
||||
});
|
||||
|
||||
@@ -49,6 +49,18 @@ export function createPlaylistBrowserModal(
|
||||
return ctx.state.playlistBrowserSnapshot;
|
||||
}
|
||||
|
||||
function resetSnapshotUi(): void {
|
||||
ctx.state.playlistBrowserSnapshot = null;
|
||||
ctx.state.playlistBrowserStatus = '';
|
||||
ctx.state.playlistBrowserSelectedDirectoryIndex = 0;
|
||||
ctx.state.playlistBrowserSelectedPlaylistIndex = 0;
|
||||
ctx.dom.playlistBrowserTitle.textContent = 'Playlist Browser';
|
||||
ctx.dom.playlistBrowserDirectoryList.replaceChildren();
|
||||
ctx.dom.playlistBrowserPlaylistList.replaceChildren();
|
||||
ctx.dom.playlistBrowserStatus.textContent = '';
|
||||
ctx.dom.playlistBrowserStatus.classList.remove('error');
|
||||
}
|
||||
|
||||
function syncSelection(snapshot: PlaylistBrowserSnapshot): void {
|
||||
const directoryIndex = snapshot.directoryItems.findIndex((item) => item.isCurrentFile);
|
||||
const playlistIndex =
|
||||
@@ -194,6 +206,7 @@ export function createPlaylistBrowserModal(
|
||||
}
|
||||
|
||||
async function refreshSnapshot(): Promise<void> {
|
||||
try {
|
||||
const snapshot = await window.electronAPI.getPlaylistBrowserSnapshot();
|
||||
ctx.state.playlistBrowserStatus = '';
|
||||
applySnapshot(snapshot);
|
||||
@@ -201,6 +214,10 @@ export function createPlaylistBrowserModal(
|
||||
buildDefaultStatus(snapshot),
|
||||
!snapshot.directoryAvailable && snapshot.directoryStatus.length > 0,
|
||||
);
|
||||
} catch (error) {
|
||||
resetSnapshotUi();
|
||||
setStatus(error instanceof Error ? error.message : String(error), true);
|
||||
}
|
||||
}
|
||||
|
||||
async function handleMutation(
|
||||
@@ -249,6 +266,9 @@ export function createPlaylistBrowserModal(
|
||||
await refreshSnapshot();
|
||||
return;
|
||||
}
|
||||
if (options.modalStateReader.isAnyModalOpen()) {
|
||||
return;
|
||||
}
|
||||
|
||||
ctx.state.playlistBrowserModalOpen = true;
|
||||
ctx.state.playlistBrowserActivePane = 'playlist';
|
||||
@@ -257,19 +277,13 @@ export function createPlaylistBrowserModal(
|
||||
ctx.dom.playlistBrowserModal.classList.remove('hidden');
|
||||
ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'false');
|
||||
window.electronAPI.notifyOverlayModalOpened('playlist-browser');
|
||||
|
||||
try {
|
||||
await refreshSnapshot();
|
||||
} catch (error) {
|
||||
setStatus(error instanceof Error ? error.message : String(error), true);
|
||||
}
|
||||
}
|
||||
|
||||
function closePlaylistBrowserModal(): void {
|
||||
if (!ctx.state.playlistBrowserModalOpen) return;
|
||||
ctx.state.playlistBrowserModalOpen = false;
|
||||
ctx.state.playlistBrowserSnapshot = null;
|
||||
ctx.state.playlistBrowserStatus = '';
|
||||
resetSnapshotUi();
|
||||
ctx.dom.playlistBrowserModal.classList.add('hidden');
|
||||
ctx.dom.playlistBrowserModal.setAttribute('aria-hidden', 'true');
|
||||
window.electronAPI.notifyOverlayModalClosed('playlist-browser');
|
||||
|
||||
Reference in New Issue
Block a user