mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-05-25 12:55:18 -07:00
fix: clear stale CSS properties and subtitle state on style/media update
- Remove CSS properties absent from subsequent subtitle style updates - Broadcast subtitle:set clear when media path changes - Preserve launcher lifecycle ownership for already-managed overlay apps - Clamp negative autoplay current time to zero - Reject blank subminerBinaryPath values via parseNonEmptyString - Log and rethrow legacy config migration errors instead of swallowing - Normalize modifier aliases (e.g. CommandOrControl) in keybinding display
This commit is contained in:
@@ -106,6 +106,16 @@ test('parseLauncherMpvConfig reads launch mode preference', () => {
|
||||
assert.equal(parsed.aniskipButtonKey, 'F8');
|
||||
});
|
||||
|
||||
test('parseLauncherMpvConfig ignores blank subminer binary paths', () => {
|
||||
const parsed = parseLauncherMpvConfig({
|
||||
mpv: {
|
||||
subminerBinaryPath: ' ',
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(parsed.subminerBinaryPath, undefined);
|
||||
});
|
||||
|
||||
test('parseLauncherMpvConfig ignores invalid launch mode values', () => {
|
||||
const parsed = parseLauncherMpvConfig({
|
||||
mpv: {
|
||||
|
||||
@@ -37,8 +37,7 @@ export function parseLauncherMpvConfig(root: Record<string, unknown>): LauncherM
|
||||
typeof mpv.autoStartSubMiner === 'boolean' ? mpv.autoStartSubMiner : undefined,
|
||||
pauseUntilOverlayReady:
|
||||
typeof mpv.pauseUntilOverlayReady === 'boolean' ? mpv.pauseUntilOverlayReady : undefined,
|
||||
subminerBinaryPath:
|
||||
typeof mpv.subminerBinaryPath === 'string' ? mpv.subminerBinaryPath.trim() : undefined,
|
||||
subminerBinaryPath: parseNonEmptyString(mpv.subminerBinaryPath),
|
||||
aniskipEnabled: typeof mpv.aniskipEnabled === 'boolean' ? mpv.aniskipEnabled : undefined,
|
||||
aniskipButtonKey: parseNonEmptyString(mpv.aniskipButtonKey),
|
||||
};
|
||||
|
||||
@@ -697,6 +697,47 @@ test('startOverlay borrows an already-running background app instead of owning i
|
||||
}
|
||||
});
|
||||
|
||||
test('startOverlay keeps lifecycle ownership for its already-managed app', async () => {
|
||||
const { dir, socketPath } = createTempSocketPath();
|
||||
const appPath = path.join(dir, 'fake-subminer.sh');
|
||||
const appInvocationsPath = path.join(dir, 'app-invocations.log');
|
||||
fs.writeFileSync(
|
||||
appPath,
|
||||
[
|
||||
'#!/bin/sh',
|
||||
`printf '%s\\n' "$@" >> ${JSON.stringify(appInvocationsPath)}`,
|
||||
'if [ "$1" = "--app-ping" ]; then exit 0; fi',
|
||||
'exit 0',
|
||||
'',
|
||||
].join('\n'),
|
||||
);
|
||||
fs.chmodSync(appPath, 0o755);
|
||||
fs.writeFileSync(socketPath, '');
|
||||
const originalCreateConnection = net.createConnection;
|
||||
try {
|
||||
state.appPath = appPath;
|
||||
state.overlayManagedByLauncher = true;
|
||||
net.createConnection = (() => {
|
||||
const socket = new EventEmitter() as net.Socket;
|
||||
socket.destroy = (() => socket) as net.Socket['destroy'];
|
||||
socket.setTimeout = (() => socket) as net.Socket['setTimeout'];
|
||||
setTimeout(() => socket.emit('connect'), 10);
|
||||
return socket;
|
||||
}) as typeof net.createConnection;
|
||||
|
||||
await startOverlay(appPath, makeArgs(), socketPath);
|
||||
|
||||
assert.equal(state.overlayManagedByLauncher, true);
|
||||
assert.equal(state.appPath, appPath);
|
||||
} finally {
|
||||
net.createConnection = originalCreateConnection;
|
||||
state.overlayProc = null;
|
||||
state.overlayManagedByLauncher = false;
|
||||
state.appPath = '';
|
||||
fs.rmSync(dir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test('cleanupPlaybackSession stops launcher-managed overlay app and mpv-owned children', async () => {
|
||||
const { dir } = createTempSocketPath();
|
||||
const appPath = path.join(dir, 'fake-subminer.sh');
|
||||
|
||||
+1
-1
@@ -1016,7 +1016,7 @@ export async function startOverlay(
|
||||
env: buildAppEnv(process.env, target.env),
|
||||
});
|
||||
attachAppProcessLogging(state.overlayProc);
|
||||
if (appAlreadyRunning) {
|
||||
if (appAlreadyRunning && !(state.overlayManagedByLauncher && state.appPath === appPath)) {
|
||||
log(
|
||||
'debug',
|
||||
args.logLevel,
|
||||
|
||||
@@ -2096,10 +2096,19 @@ test('migrates legacy ankiConnect n+1 color value to subtitleStyle', () => {
|
||||
subtitleStyle: { nPlusOneColor?: string; knownWordColor?: string };
|
||||
};
|
||||
assert.equal(parsed.subtitleStyle.nPlusOneColor, '#c6a0f6');
|
||||
assert.equal(config.subtitleStyle.knownWordColor, '#a6da95');
|
||||
assert.equal(Object.hasOwn(parsed.ankiConnect.nPlusOne ?? {}, 'nPlusOne'), false);
|
||||
});
|
||||
|
||||
test('legacy migration failures are logged and rethrown', () => {
|
||||
const source = fs.readFileSync(path.join(process.cwd(), 'src/config/service.ts'), 'utf-8');
|
||||
const catchBlock = source.match(/catch\s*\(error\)\s*\{(?<body>[\s\S]*?)\n \}/)?.groups?.body;
|
||||
|
||||
assert.ok(catchBlock);
|
||||
assert.match(catchBlock, /legacy config migration failed/);
|
||||
assert.match(catchBlock, /console\.error/);
|
||||
assert.match(catchBlock, /throw error;/);
|
||||
});
|
||||
|
||||
test('migrates legacy ankiConnect nPlusOne known-word settings to knownWords', () => {
|
||||
const dir = makeTempDir();
|
||||
const configPath = path.join(dir, 'config.jsonc');
|
||||
|
||||
@@ -132,7 +132,7 @@ test('n+1 annotation color has one public config path', () => {
|
||||
const leaves = collectConfigLeafPaths(DEFAULT_CONFIG);
|
||||
|
||||
assert.ok(leaves.includes('subtitleStyle.nPlusOneColor'));
|
||||
assert.ok(!leaves.includes('ankiConnect.nPlusOne.nPlusOne'));
|
||||
assert.ok(!leaves.includes('ankiConnect.nPlusOne.color'));
|
||||
});
|
||||
|
||||
test('every DEFAULT_CONFIG leaf is in CONFIG_OPTION_REGISTRY or UNDOCUMENTED_LEAVES', () => {
|
||||
|
||||
@@ -151,8 +151,9 @@ export class ConfigService {
|
||||
}
|
||||
fs.writeFileSync(configPath, content, 'utf-8');
|
||||
return rawConfig;
|
||||
} catch {
|
||||
return config;
|
||||
} catch (error) {
|
||||
console.error(`[ConfigService] legacy config migration failed for ${configPath}:`, error);
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4251,6 +4251,7 @@ const {
|
||||
appState.currentSubText = '';
|
||||
appState.currentSubAssText = '';
|
||||
appState.currentSubtitleData = null;
|
||||
broadcastToOverlayWindows('subtitle:set', { text: '', tokens: null });
|
||||
}
|
||||
autoplayReadyGate.invalidatePendingAutoplayReadyFallbacks();
|
||||
currentMediaTokenizationGate.updateCurrentMediaPath(path);
|
||||
|
||||
@@ -21,6 +21,23 @@ test('manual watched session action starts immersion tracker before marking watc
|
||||
);
|
||||
});
|
||||
|
||||
test('media path changes clear rendered subtitle state', () => {
|
||||
const source = readMainSource();
|
||||
const actionBlock = source.match(
|
||||
/updateCurrentMediaPath:\s*\(path\)\s*=>\s*\{(?<body>[\s\S]*?)autoplayReadyGate\.invalidatePendingAutoplayReadyFallbacks\(\);/,
|
||||
)?.groups?.body;
|
||||
|
||||
assert.ok(actionBlock);
|
||||
assert.match(actionBlock, /appState\.currentSubText = '';/);
|
||||
assert.match(actionBlock, /appState\.currentSubAssText = '';/);
|
||||
assert.match(actionBlock, /appState\.currentSubtitleData = null;/);
|
||||
assert.match(actionBlock, /broadcastToOverlayWindows\('subtitle:set',/);
|
||||
assert.ok(
|
||||
actionBlock.indexOf('appState.currentSubtitleData = null;') <
|
||||
actionBlock.indexOf("broadcastToOverlayWindows('subtitle:set'"),
|
||||
);
|
||||
});
|
||||
|
||||
test('main process uses one shared mpv plugin runtime config helper', () => {
|
||||
const source = readMainSource();
|
||||
assert.match(source, /function getMpvPluginRuntimeConfig\(\)/);
|
||||
|
||||
@@ -30,6 +30,13 @@ test('selectAutoplayStartupCue returns the next imminent cue before playback sta
|
||||
);
|
||||
});
|
||||
|
||||
test('selectAutoplayStartupCue clamps negative current time to startup', () => {
|
||||
assert.deepEqual(
|
||||
selectAutoplayStartupCue([{ startTime: 0, endTime: 1, text: 'startup' }], -0.5, 0),
|
||||
{ startTime: 0, endTime: 1, text: 'startup' },
|
||||
);
|
||||
});
|
||||
|
||||
test('selectAutoplayStartupCue does not reveal far future subtitle text', () => {
|
||||
assert.equal(
|
||||
selectAutoplayStartupCue([{ startTime: 12, endTime: 15, text: 'later' }], 0, 2),
|
||||
|
||||
@@ -5,7 +5,7 @@ export function selectAutoplayStartupCue(
|
||||
currentTimeSeconds: number,
|
||||
lookaheadSeconds: number,
|
||||
): SubtitleCue | null {
|
||||
const currentTime = Number.isFinite(currentTimeSeconds) ? currentTimeSeconds : 0;
|
||||
const currentTime = Math.max(0, Number.isFinite(currentTimeSeconds) ? currentTimeSeconds : 0);
|
||||
const lookahead = Math.max(0, Number.isFinite(lookaheadSeconds) ? lookaheadSeconds : 0);
|
||||
const latestStartTime = currentTime + lookahead;
|
||||
|
||||
|
||||
@@ -68,10 +68,13 @@ function normalizeKeyToken(token: string): string {
|
||||
}
|
||||
|
||||
function formatKeybinding(rawBinding: string): string {
|
||||
const parts = rawBinding.split('+');
|
||||
const parts = rawBinding
|
||||
.split('+')
|
||||
.map((part) => part.trim())
|
||||
.filter(Boolean);
|
||||
const key = parts.pop();
|
||||
if (!key) return rawBinding;
|
||||
const normalized = [...parts, normalizeKeyToken(key)];
|
||||
const normalized = [...parts.map(normalizeKeyToken), normalizeKeyToken(key)];
|
||||
return normalized.join(' + ');
|
||||
}
|
||||
|
||||
|
||||
@@ -33,6 +33,10 @@ test('session help formats bracket keybindings as physical keys', () => {
|
||||
assert.equal(formatSessionHelpKeybinding('Shift+BracketLeft'), 'Shift + [');
|
||||
});
|
||||
|
||||
test('session help normalizes configured modifier aliases', () => {
|
||||
assert.equal(formatSessionHelpKeybinding('CommandOrControl+KeyS'), 'Cmd/Ctrl + S');
|
||||
});
|
||||
|
||||
test('session help imports browser-safe special command constants', () => {
|
||||
const source = fs.readFileSync(
|
||||
path.join(process.cwd(), 'src', 'renderer', 'modals', 'session-help-sections.ts'),
|
||||
|
||||
@@ -45,6 +45,12 @@ class FakeStyleDeclaration {
|
||||
setProperty(name: string, value: string) {
|
||||
this.values.set(name, value);
|
||||
}
|
||||
|
||||
removeProperty(name: string) {
|
||||
const previous = this.values.get(name) ?? '';
|
||||
this.values.delete(name);
|
||||
return previous;
|
||||
}
|
||||
}
|
||||
|
||||
class FakeElement {
|
||||
@@ -475,6 +481,57 @@ test('applySubtitleStyle applies primary and secondary css declaration objects',
|
||||
}
|
||||
});
|
||||
|
||||
test('applySubtitleStyle removes css declarations missing from later updates', () => {
|
||||
const restoreDocument = installFakeDocument();
|
||||
try {
|
||||
const subtitleRoot = new FakeElement('div');
|
||||
const subtitleContainer = new FakeElement('div');
|
||||
const secondarySubRoot = new FakeElement('div');
|
||||
const secondarySubContainer = new FakeElement('div');
|
||||
const ctx = {
|
||||
state: createRendererState(),
|
||||
dom: {
|
||||
subtitleRoot,
|
||||
subtitleContainer,
|
||||
secondarySubRoot,
|
||||
secondarySubContainer,
|
||||
},
|
||||
} as never;
|
||||
|
||||
const renderer = createSubtitleRenderer(ctx);
|
||||
renderer.applySubtitleStyle({
|
||||
css: {
|
||||
'font-size': '42px',
|
||||
'text-wrap': 'balance',
|
||||
},
|
||||
secondary: {
|
||||
css: {
|
||||
'text-transform': 'uppercase',
|
||||
},
|
||||
},
|
||||
} as never);
|
||||
renderer.applySubtitleStyle({
|
||||
css: {
|
||||
'font-size': '44px',
|
||||
},
|
||||
secondary: {
|
||||
css: {},
|
||||
},
|
||||
} as never);
|
||||
|
||||
const primaryValues = (subtitleRoot.style as unknown as { values?: Map<string, string> })
|
||||
.values;
|
||||
const secondaryValues = (secondarySubRoot.style as unknown as { values?: Map<string, string> })
|
||||
.values;
|
||||
|
||||
assert.equal(primaryValues?.get('font-size'), '44px');
|
||||
assert.equal(primaryValues?.has('text-wrap'), false);
|
||||
assert.equal(secondaryValues?.has('text-transform'), false);
|
||||
} finally {
|
||||
restoreDocument();
|
||||
}
|
||||
});
|
||||
|
||||
test('annotated subtitle tokens inherit configured base subtitle typography', () => {
|
||||
const restoreDocument = installFakeDocument();
|
||||
try {
|
||||
|
||||
@@ -158,6 +158,49 @@ function applyInlineStyleDeclarations(
|
||||
}
|
||||
}
|
||||
|
||||
const appliedCssKeys = new WeakMap<HTMLElement, Set<string>>();
|
||||
|
||||
function inlineStyleDeclarationKeys(
|
||||
declarations: Record<string, unknown>,
|
||||
excludedKeys: ReadonlySet<string>,
|
||||
): Set<string> {
|
||||
const keys = new Set<string>();
|
||||
for (const [key, value] of Object.entries(declarations)) {
|
||||
if (excludedKeys.has(key)) continue;
|
||||
if (value === null || value === undefined || typeof value === 'object') continue;
|
||||
keys.add(key);
|
||||
}
|
||||
return keys;
|
||||
}
|
||||
|
||||
function clearInlineStyleDeclaration(target: HTMLElement, key: string): void {
|
||||
if (key.includes('-')) {
|
||||
target.style.removeProperty(key);
|
||||
if (key === '--webkit-text-stroke') {
|
||||
target.style.removeProperty('-webkit-text-stroke');
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
(target.style as unknown as Record<string, string>)[key] = '';
|
||||
}
|
||||
|
||||
function replaceInlineStyleDeclarations(
|
||||
target: HTMLElement,
|
||||
declarations: Record<string, unknown>,
|
||||
excludedKeys: ReadonlySet<string> = new Set<string>(),
|
||||
): void {
|
||||
const nextKeys = inlineStyleDeclarationKeys(declarations, excludedKeys);
|
||||
const previousKeys = appliedCssKeys.get(target) ?? new Set<string>();
|
||||
for (const key of previousKeys) {
|
||||
if (!nextKeys.has(key)) {
|
||||
clearInlineStyleDeclaration(target, key);
|
||||
}
|
||||
}
|
||||
applyInlineStyleDeclarations(target, declarations, excludedKeys);
|
||||
appliedCssKeys.set(target, nextKeys);
|
||||
}
|
||||
|
||||
function normalizeCssDeclarationObject(value: unknown): Record<string, string> {
|
||||
if (!value || typeof value !== 'object' || Array.isArray(value)) {
|
||||
return {};
|
||||
@@ -177,8 +220,8 @@ function applySubtitleCssDeclarations(
|
||||
container: HTMLElement,
|
||||
declarations: Record<string, string>,
|
||||
): void {
|
||||
applyInlineStyleDeclarations(root, declarations, CONTAINER_STYLE_KEYS);
|
||||
applyInlineStyleDeclarations(
|
||||
replaceInlineStyleDeclarations(root, declarations, CONTAINER_STYLE_KEYS);
|
||||
replaceInlineStyleDeclarations(
|
||||
container,
|
||||
pickInlineStyleDeclarations(declarations, CONTAINER_STYLE_KEYS),
|
||||
);
|
||||
|
||||
Reference in New Issue
Block a user