mirror of
https://github.com/ksyasuda/SubMiner.git
synced 2026-02-27 18:22:41 -08:00
Fix overlay toggle regression TASK-7
This commit is contained in:
@@ -0,0 +1,62 @@
|
|||||||
|
---
|
||||||
|
id: TASK-107
|
||||||
|
title: Fix post-rebase overlay toggle regression
|
||||||
|
status: In Progress
|
||||||
|
assignee:
|
||||||
|
- codex
|
||||||
|
created_date: '2026-02-21 23:34'
|
||||||
|
updated_date: '2026-02-21 23:45'
|
||||||
|
labels:
|
||||||
|
- bug
|
||||||
|
- overlay
|
||||||
|
- regression
|
||||||
|
dependencies: []
|
||||||
|
priority: high
|
||||||
|
---
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
<!-- SECTION:DESCRIPTION:BEGIN -->
|
||||||
|
After recent rebase, toggling visible or invisible overlay opens a transparent non-interactable window. Symptoms:
|
||||||
|
- invisible subtitle mode no longer renders/works,
|
||||||
|
- visible subtitles do not show,
|
||||||
|
- overlay keybinds do not work on the created window.
|
||||||
|
|
||||||
|
Need root-cause fix so both overlay modes render and interactive/keybind behavior returns to pre-rebase behavior.
|
||||||
|
<!-- SECTION:DESCRIPTION:END -->
|
||||||
|
|
||||||
|
## Action Steps
|
||||||
|
|
||||||
|
<!-- SECTION:PLAN:BEGIN -->
|
||||||
|
1. Reproduce regression in source tests around overlay toggle/window behavior.
|
||||||
|
2. Add failing regression test for transparent/non-interactable overlay window path.
|
||||||
|
3. Identify rebase-introduced break in overlay creation/runtime toggle wiring.
|
||||||
|
4. Implement minimal fix for renderer loading + window interaction state.
|
||||||
|
5. Verify with focused tests and smoke checks.
|
||||||
|
<!-- SECTION:PLAN:END -->
|
||||||
|
|
||||||
|
## Acceptance Criteria
|
||||||
|
<!-- AC:BEGIN -->
|
||||||
|
- [ ] #1 Toggling visible overlay shows subtitle content again.
|
||||||
|
- [ ] #2 Toggling invisible overlay restores interactive subtitle behavior.
|
||||||
|
- [ ] #3 Overlay keybinds work after overlay toggle in both modes.
|
||||||
|
- [ ] #4 Added regression coverage for broken toggle path.
|
||||||
|
<!-- AC:END -->
|
||||||
|
|
||||||
|
## Definition of Done
|
||||||
|
<!-- DOD:BEGIN -->
|
||||||
|
- [ ] #1 Focused overlay/runtime tests pass.
|
||||||
|
- [ ] #2 No new type/build regressions introduced by fix.
|
||||||
|
- [ ] #3 Task notes include root cause and validation commands.
|
||||||
|
<!-- DOD:END -->
|
||||||
|
|
||||||
|
## Implementation Notes
|
||||||
|
|
||||||
|
<!-- SECTION:NOTES:BEGIN -->
|
||||||
|
- Root-cause candidate fixed: renderer layer detection could trust preload `process.argv` over per-window query params, which can drift under shared renderer process and force wrong layer behavior.
|
||||||
|
- Changed `src/renderer/utils/platform.ts` to prioritize `window.location.search` (`?layer=visible|invisible`) and only fallback to preload layer when query is missing/invalid.
|
||||||
|
- Added regression test in `src/renderer/error-recovery.test.ts` asserting query-layer precedence.
|
||||||
|
- Added explicit `webPreferences.sandbox = false` in `src/core/services/overlay-window.ts` to keep preload Node-style API availability stable on newer Electron defaults.
|
||||||
|
- Added regression test `src/core/services/overlay-window-config.test.ts` to guard sandbox setting.
|
||||||
|
- Validation: `bun test src/core/services/overlay-window-config.test.ts src/renderer/error-recovery.test.ts`; `bun run build`.
|
||||||
|
<!-- SECTION:NOTES:END -->
|
||||||
@@ -68,3 +68,4 @@ Read first. Keep concise.
|
|||||||
| `codex-fix-rebase-errors-20260222T062235Z-73h4` | `codex-fix-rebase-errors` | `Resolve current git rebase conflicts in ipc/main runtime files and land clean rebase state` | `done` | `docs/subagents/agents/codex-fix-rebase-errors-20260222T062235Z-73h4.md` | `2026-02-22T06:30:48Z` |
|
| `codex-fix-rebase-errors-20260222T062235Z-73h4` | `codex-fix-rebase-errors` | `Resolve current git rebase conflicts in ipc/main runtime files and land clean rebase state` | `done` | `docs/subagents/agents/codex-fix-rebase-errors-20260222T062235Z-73h4.md` | `2026-02-22T06:30:48Z` |
|
||||||
| `codex-review-cleanup-20260222T065718Z-9p4m` | `codex-review-cleanup` | `Review post-refactor codebase quality and create cleanup tickets with concrete scope and completion criteria` | `done` | `docs/subagents/agents/codex-review-cleanup-20260222T065718Z-9p4m.md` | `2026-02-22T07:04:48Z` |
|
| `codex-review-cleanup-20260222T065718Z-9p4m` | `codex-review-cleanup` | `Review post-refactor codebase quality and create cleanup tickets with concrete scope and completion criteria` | `done` | `docs/subagents/agents/codex-review-cleanup-20260222T065718Z-9p4m.md` | `2026-02-22T07:04:48Z` |
|
||||||
| `codex-jellyfin-ts-fix-20260222T071530Z-5e50` | `codex-jellyfin-ts-fix` | `Fix Jellyfin token/session type drift causing TS compile failures in config+main.` | `done` | `docs/subagents/agents/codex-jellyfin-ts-fix-20260222T071530Z-5e50.md` | `2026-02-22T07:23:47Z` |
|
| `codex-jellyfin-ts-fix-20260222T071530Z-5e50` | `codex-jellyfin-ts-fix` | `Fix Jellyfin token/session type drift causing TS compile failures in config+main.` | `done` | `docs/subagents/agents/codex-jellyfin-ts-fix-20260222T071530Z-5e50.md` | `2026-02-22T07:23:47Z` |
|
||||||
|
| `codex-overlay-toggle-regression-20260222T073450Z-q7m4` | `codex-overlay-toggle-regression` | `Fix post-rebase overlay toggle regression causing transparent non-interactable windows and broken keybinds (TASK-107).` | `testing` | `docs/subagents/agents/codex-overlay-toggle-regression-20260222T073450Z-q7m4.md` | `2026-02-22T07:45:58Z` |
|
||||||
|
|||||||
@@ -0,0 +1,33 @@
|
|||||||
|
# Agent: `codex-overlay-toggle-regression-20260222T073450Z-q7m4`
|
||||||
|
|
||||||
|
- alias: `codex-overlay-toggle-regression`
|
||||||
|
- mission: `Fix post-rebase overlay toggle regression causing transparent non-interactable windows and broken keybinds (TASK-107).`
|
||||||
|
- status: `testing`
|
||||||
|
- branch: `main`
|
||||||
|
- started_at: `2026-02-22T07:34:50Z`
|
||||||
|
- heartbeat_minutes: `5`
|
||||||
|
|
||||||
|
## Current Work (newest first)
|
||||||
|
- [2026-02-22T07:45:58Z] progress: Added `src/core/services/overlay-window-config.test.ts` red test for explicit `webPreferences.sandbox=false`; patched `src/core/services/overlay-window.ts` with `sandbox: false`; verified focused tests + full build pass.
|
||||||
|
- [2026-02-22T07:44:40Z] test: Added red regression in `src/renderer/error-recovery.test.ts` proving `resolvePlatformInfo` incorrectly preferred preload layer over query layer; patched `src/renderer/utils/platform.ts` to prioritize `?layer=` and reran tests/build green.
|
||||||
|
- [2026-02-22T07:34:50Z] intent: Initialize session record, associate work with TASK-107, inspect overlay runtime + renderer regressions after rebase.
|
||||||
|
|
||||||
|
## Files Touched
|
||||||
|
- `backlog/tasks/task-107 - Fix-post-rebase-overlay-toggle-regression.md`
|
||||||
|
- `docs/subagents/agents/codex-overlay-toggle-regression-20260222T073450Z-q7m4.md`
|
||||||
|
- `docs/subagents/INDEX.md`
|
||||||
|
- `docs/subagents/collaboration.md`
|
||||||
|
- `src/renderer/error-recovery.test.ts`
|
||||||
|
- `src/renderer/utils/platform.ts`
|
||||||
|
- `src/core/services/overlay-window.ts`
|
||||||
|
- `src/core/services/overlay-window-config.test.ts`
|
||||||
|
|
||||||
|
## Assumptions
|
||||||
|
- Regression likely introduced by recent rebase merge in main/runtime overlay wiring.
|
||||||
|
- Existing overlay regression tests can be extended for red-green fix.
|
||||||
|
|
||||||
|
## Open Questions / Blockers
|
||||||
|
- Exact break location (window options vs renderer preload/load path vs toggle handler state).
|
||||||
|
|
||||||
|
## Next Step
|
||||||
|
- Hand off fix summary + validation commands to user for runtime confirmation on host machine.
|
||||||
@@ -98,3 +98,6 @@ Shared notes. Append-only.
|
|||||||
- [2026-02-22T07:15:30Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] overlap note: fixing Jellyfin config/runtime type drift touching `src/main.ts`, `src/config/definitions/*`, `src/config/resolve/*`; preserving recent refactor structure and limiting scope to TS errors only.
|
- [2026-02-22T07:15:30Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] overlap note: fixing Jellyfin config/runtime type drift touching `src/main.ts`, `src/config/definitions/*`, `src/config/resolve/*`; preserving recent refactor structure and limiting scope to TS errors only.
|
||||||
- [2026-02-22T07:18:07Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] completed TS fix: removed legacy `jellyfin.accessToken/userId` from defaults/resolver, aligned `src/main.ts` to `loadSession/saveSession/clearSession`, added resolver regression test for legacy keys, and verified `bun run tsc --noEmit` + focused jellyfin tests green.
|
- [2026-02-22T07:18:07Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] completed TS fix: removed legacy `jellyfin.accessToken/userId` from defaults/resolver, aligned `src/main.ts` to `loadSession/saveSession/clearSession`, added resolver regression test for legacy keys, and verified `bun run tsc --noEmit` + focused jellyfin tests green.
|
||||||
- [2026-02-22T07:23:47Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] docs follow-up: fixed stale `docs/configuration.md` Jellyfin table/text to remove `accessToken/userId` config guidance; now states stored encrypted session + env override keys.
|
- [2026-02-22T07:23:47Z] [codex-jellyfin-ts-fix-20260222T071530Z-5e50|codex-jellyfin-ts-fix] docs follow-up: fixed stale `docs/configuration.md` Jellyfin table/text to remove `accessToken/userId` config guidance; now states stored encrypted session + env override keys.
|
||||||
|
- [2026-02-22T07:34:50Z] [codex-overlay-toggle-regression-20260222T073450Z-q7m4|codex-overlay-toggle-regression] starting TASK-107 bugfix for post-rebase overlay regression: toggling visible/invisible opens transparent non-interactable window; keybinds + subtitle rendering fail in both modes.
|
||||||
|
- [2026-02-22T07:44:40Z] [codex-overlay-toggle-regression-20260222T073450Z-q7m4|codex-overlay-toggle-regression] identified renderer layer-resolution bug risk under shared renderer process (`process.argv` preload arg drift); fixed `resolvePlatformInfo` to prioritize per-window `?layer=` query, added regression test, verified `bun test src/renderer/error-recovery.test.ts` + `bun run build`.
|
||||||
|
- [2026-02-22T07:45:58Z] [codex-overlay-toggle-regression-20260222T073450Z-q7m4|codex-overlay-toggle-regression] added explicit overlay BrowserWindow sandbox guard (`webPreferences.sandbox=false`) to avoid preload API break on newer Electron defaults; added regression test `src/core/services/overlay-window-config.test.ts`; verified focused tests + build green.
|
||||||
|
|||||||
11
src/core/services/overlay-window-config.test.ts
Normal file
11
src/core/services/overlay-window-config.test.ts
Normal file
@@ -0,0 +1,11 @@
|
|||||||
|
import test from 'node:test';
|
||||||
|
import assert from 'node:assert/strict';
|
||||||
|
import fs from 'node:fs';
|
||||||
|
import path from 'node:path';
|
||||||
|
|
||||||
|
test('overlay window config explicitly disables renderer sandbox for preload compatibility', () => {
|
||||||
|
const sourcePath = path.join(process.cwd(), 'src/core/services/overlay-window.ts');
|
||||||
|
const source = fs.readFileSync(sourcePath, 'utf8');
|
||||||
|
|
||||||
|
assert.match(source, /webPreferences:\s*\{[\s\S]*sandbox:\s*false[\s\S]*\}/m);
|
||||||
|
});
|
||||||
@@ -75,6 +75,7 @@ export function createOverlayWindow(
|
|||||||
preload: path.join(__dirname, '..', '..', 'preload.js'),
|
preload: path.join(__dirname, '..', '..', 'preload.js'),
|
||||||
contextIsolation: true,
|
contextIsolation: true,
|
||||||
nodeIntegration: false,
|
nodeIntegration: false,
|
||||||
|
sandbox: false,
|
||||||
webSecurity: true,
|
webSecurity: true,
|
||||||
additionalArguments: [`--overlay-layer=${kind}`],
|
additionalArguments: [`--overlay-layer=${kind}`],
|
||||||
},
|
},
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import test from 'node:test';
|
|||||||
import assert from 'node:assert/strict';
|
import assert from 'node:assert/strict';
|
||||||
|
|
||||||
import { createRendererRecoveryController } from './error-recovery.js';
|
import { createRendererRecoveryController } from './error-recovery.js';
|
||||||
|
import { resolvePlatformInfo } from './utils/platform.js';
|
||||||
|
|
||||||
test('handleError logs context and recovers overlay state', () => {
|
test('handleError logs context and recovers overlay state', () => {
|
||||||
const payloads: unknown[] = [];
|
const payloads: unknown[] = [];
|
||||||
@@ -120,3 +121,37 @@ test('nested recovery errors are ignored while current recovery is active', () =
|
|||||||
assert.equal(payloads.length, 1);
|
assert.equal(payloads.length, 1);
|
||||||
assert.equal(restored, 1);
|
assert.equal(restored, 1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('resolvePlatformInfo prefers query layer over preload layer', () => {
|
||||||
|
const previousWindow = (globalThis as { window?: unknown }).window;
|
||||||
|
const previousNavigator = (globalThis as { navigator?: unknown }).navigator;
|
||||||
|
|
||||||
|
Object.defineProperty(globalThis, 'window', {
|
||||||
|
configurable: true,
|
||||||
|
value: {
|
||||||
|
electronAPI: {
|
||||||
|
getOverlayLayer: () => 'invisible',
|
||||||
|
},
|
||||||
|
location: { search: '?layer=visible' },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
Object.defineProperty(globalThis, 'navigator', {
|
||||||
|
configurable: true,
|
||||||
|
value: {
|
||||||
|
platform: 'MacIntel',
|
||||||
|
userAgent: 'Mozilla/5.0 (Macintosh)',
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
const info = resolvePlatformInfo();
|
||||||
|
assert.equal(info.overlayLayer, 'visible');
|
||||||
|
assert.equal(info.isInvisibleLayer, false);
|
||||||
|
} finally {
|
||||||
|
Object.defineProperty(globalThis, 'window', { configurable: true, value: previousWindow });
|
||||||
|
Object.defineProperty(globalThis, 'navigator', {
|
||||||
|
configurable: true,
|
||||||
|
value: previousNavigator,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|||||||
@@ -13,15 +13,15 @@ export type PlatformInfo = {
|
|||||||
|
|
||||||
export function resolvePlatformInfo(): PlatformInfo {
|
export function resolvePlatformInfo(): PlatformInfo {
|
||||||
const overlayLayerFromPreload = window.electronAPI.getOverlayLayer();
|
const overlayLayerFromPreload = window.electronAPI.getOverlayLayer();
|
||||||
const overlayLayerFromQuery =
|
const queryLayer = new URLSearchParams(window.location.search).get('layer');
|
||||||
new URLSearchParams(window.location.search).get('layer') === 'invisible'
|
const overlayLayerFromQuery: OverlayLayer | null =
|
||||||
? 'invisible'
|
queryLayer === 'visible' || queryLayer === 'invisible' ? queryLayer : null;
|
||||||
: 'visible';
|
|
||||||
|
|
||||||
const overlayLayer: OverlayLayer =
|
const overlayLayer: OverlayLayer =
|
||||||
overlayLayerFromPreload === 'visible' || overlayLayerFromPreload === 'invisible'
|
overlayLayerFromQuery ??
|
||||||
|
(overlayLayerFromPreload === 'visible' || overlayLayerFromPreload === 'invisible'
|
||||||
? overlayLayerFromPreload
|
? overlayLayerFromPreload
|
||||||
: overlayLayerFromQuery;
|
: 'visible');
|
||||||
|
|
||||||
const isInvisibleLayer = overlayLayer === 'invisible';
|
const isInvisibleLayer = overlayLayer === 'invisible';
|
||||||
const isLinuxPlatform = navigator.platform.toLowerCase().includes('linux');
|
const isLinuxPlatform = navigator.platform.toLowerCase().includes('linux');
|
||||||
|
|||||||
Reference in New Issue
Block a user