fix(playback): harden external-subtitle handling from PR #1471 review

Addresses the confirmed code-review findings: - Deselect the hls.js engine track BEFORE activating an external subtitle track: hls.js reacts to subtitleTrack=-1 by disabling every subtitle-kind TextTrack, which undid the just-selected external track. - Track ownership of every TextTrack the session ever created (addTextTrack tracks cannot leave the element), so stale or attach-failed tracks stay excluded from the native enumeration instead of reappearing as ghost engine tracks after source changes; failed attaches are silenced and their partial cues removed. - Guard the file pick with a source generation so a pick that outlives a stream change (Up Next, zapping, failover) is discarded instead of attaching the previous stream's subtitles to the next one. - Decode picked files encoding-aware (UTF-16 BOMs, strict UTF-8, then a Windows-1251/1252 heuristic) instead of Blob.text()'s silent UTF-8 substitution that rendered legacy-encoded SRT files as mojibake. - Gate the delay row on an external track being SELECTED, not merely loaded, so it can no longer sit enabled while visually inert. - Keep real (possibly negative) cue times under a negative delay instead of clamping pre-roll cues into a simultaneous stack at t=0. - Fix subtitleDelayLabel returning a signed negative zero for sub-tenth values. - Hoist the canonical PlayerSubtitleStyle shape plus clamp/normalize rules into @iptvnator/shared/interfaces (subtitle-style.util.ts); the renderer and the Electron main process now share one implementation, removing the triplicated literals and the toLowerCase divergence. Adds regression coverage for each fix; updates the player-controls contract doc and CLAUDE.md accordingly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-10-09 09:26:14 -08:00 · 2026-08-23 00:01:14 +02:00
1 parent 404deb92b0
commit 10bd5acd6f
@@ -1101,7 +1101,7 @@ engine` (restart required) or
  helper: `apps/electron-backend/native/helper/`; canonical packaging/runtime
  contracts: `docs/architecture/embedded-mpv-native.md` and
  `tools/embedded-mpv/README.md`.
- Shared player-controls layer: `libs/ui/playback/src/lib/player-controls/` exports the engine-neutral `PlayerController` contract, standalone `app-player-controls`, a generic web-video adapter/helper, and component-scoped `WEB_PLAYER_SHARED_CONTROLS` rollout token. Its subtitle menu carries capability-gated advanced subtitle support (#1408): external subtitle file loading, a ±0.5 s timing-offset row, and size/color styling persisted in the shared `subtitleStyle` localStorage key. HTML5/ArtPlayer implement it through the neutral source bridge (`.srt`/`.vtt` via a DOM file picker, native `TextTrack` rendering, `::cue` styling, delay only for the loaded file's owned cues); Embedded MPV frame-copy implements it through new helper protocol commands (`sub-add`/`sub-delay`/`sub-scale`/`sub-color`, main-process file dialog, ASS supported, delay for all tracks). Video.js shared mode, vendor-chrome paths, native-view, and the Linux out-of-process path advertise no such capability and render no UI. Contract details: `docs/architecture/player-controls-contract.md` ("Advanced subtitle support"). In fullscreen, `app-player-controls` shows a pointer-transparent media-title overlay at the top while controls are revealed (`mediaTitle` input: movie/channel/series name, plus an `S01E03` second line for episodes; series names flow from the detail views through `PortalInlinePlayerComponent.seriesTitle` and `WebPlayerViewComponent.mediaTitle`). Persisted `Settings.webPlayerSharedControls` is default-off, and its checkbox appears only when HTML5, Video.js, or ArtPlayer is selected. `WebPlayerViewComponent` snapshots the preference into the immutable token for each new player host. The parent `/workspace` route awaits the initial `SettingsStore` load, including cold-start direct links, before this snapshot can occur. Saving applies to the next host without an application restart; an existing session never changes controls mode in place. Embedded MPV ignores the web-player preference: frame-copy always uses shared DOM controls through `EmbeddedMpvControlsAdapter`, native-view retains its compositor-safe legacy dock, and external MPV/VLC retain their own UI. The Embedded MPV host selects exactly one controls UI for its reported engine. `showControls=false` detaches the shared surface, modal overlays gate frame-copy playback shortcuts, fullscreen remains DOM-based with Embedded MPV bounds sync, and a playback/session transition key prevents engine or session handoff from presenting stale recording feedback while timers and pending commands are cancelled. Same-session IPC replies yield to a broadcast snapshot received while the command was pending, so a successful recording acknowledgement cannot be rolled back by a stale reply. The built-in HTML5/hls.js player is the second guarded consumer: `HtmlVideoPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`, while its neutral `web-video-support` bridge is shared with ArtPlayer and owns HLS/Shaka(DASH)/native tracks, MPEG-TS VOD duration correction, caption preference, and source cleanup. `HtmlVideoElementSession` owns native video-event lifecycle, persisted volume, and start-time/time/ended propagation. Video.js is the third guarded consumer: `VjsPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`; its bridge rebinds the current Tech video after `playerreset`, exposes source-stable audio/subtitle IDs, preserves caption preference and explicit subtitle-off state, and reads Video.js duration. Reset-driven raw MPEG-TS changes pause first, coalesce to the latest desired source, preserve actual volume across Video.js's reset, and restart when authoritative live/VOD metadata changes. In shared-controls mode, Video.js native controls, click/double-click/hotkey actions, and spatial navigation are disabled. ArtPlayer is the fourth guarded consumer: `ArtPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`; `ArtPlayerSourceSession` owns HLS/DASH(Shaka)/MPEG-TS/native sources, the neutral web-video bridge, exact cleanup, and a destroyed-session guard for delayed `customType` callbacks, while `ArtPlayerVideoSession` owns native media/ArtPlayer events. Shared ArtPlayer mode uses authoritative live/VOD metadata, HLS/Shaka/native tracks and caption preference, MPEG-TS VOD duration correction, and reapplies app volume directly after ArtPlayer restores its own stored volume. Vendor chrome/hotkeys are disabled, and a transparent capture layer gives shared controls exclusive click and double-click ownership. `WebPlayerViewComponent.resolvedIsLive` supplies authoritative metadata; visible playback diagnostics disable shared pointer/keyboard ownership and exit only the active HTML5, Video.js, or ArtPlayer shell's own fullscreen so ranked recovery actions remain visible. On the preference-off path, all three web players retain their existing controls, source behavior, and legacy series navigation — but the playback keyboard shortcuts (Space/K, F, arroLine truncated
+- Shared player-controls layer: `libs/ui/playback/src/lib/player-controls/` exports the engine-neutral `PlayerController` contract, standalone `app-player-controls`, a generic web-video adapter/helper, and component-scoped `WEB_PLAYER_SHARED_CONTROLS` rollout token. Its subtitle menu carries capability-gated advanced subtitle support (#1408): external subtitle file loading, a ±0.5 s timing-offset row, and size/color styling persisted in the shared `subtitleStyle` localStorage key. HTML5/ArtPlayer implement it through the neutral source bridge (`.srt`/`.vtt` via a DOM file picker with encoding detection, native `TextTrack` rendering, `::cue` styling, delay only while the loaded file is the selected track; picks are source-generation-guarded and engine deselection precedes external track activation); the canonical style shape and clamp/normalize rules are shared with the main process via `@iptvnator/shared/interfaces` (`subtitle-style.util.ts`). Embedded MPV frame-copy implements it through new helper protocol commands (`sub-add`/`sub-delay`/`sub-scale`/`sub-color`, main-process file dialog, ASS supported, delay for all tracks). Video.js shared mode, vendor-chrome paths, native-view, and the Linux out-of-process path advertise no such capability and render no UI. Contract details: `docs/architecture/player-controls-contract.md` ("Advanced subtitle support"). In fullscreen, `app-player-controls` shows a pointer-transparent media-title overlay at the top while controls are revealed (`mediaTitle` input: movie/channel/series name, plus an `S01E03` second line for episodes; series names flow from the detail views through `PortalInlinePlayerComponent.seriesTitle` and `WebPlayerViewComponent.mediaTitle`). Persisted `Settings.webPlayerSharedControls` is default-off, and its checkbox appears only when HTML5, Video.js, or ArtPlayer is selected. `WebPlayerViewComponent` snapshots the preference into the immutable token for each new player host. The parent `/workspace` route awaits the initial `SettingsStore` load, including cold-start direct links, before this snapshot can occur. Saving applies to the next host without an application restart; an existing session never changes controls mode in place. Embedded MPV ignores the web-player preference: frame-copy always uses shared DOM controls through `EmbeddedMpvControlsAdapter`, native-view retains its compositor-safe legacy dock, and external MPV/VLC retain their own UI. The Embedded MPV host selects exactly one controls UI for its reported engine. `showControls=false` detaches the shared surface, modal overlays gate frame-copy playback shortcuts, fullscreen remains DOM-based with Embedded MPV bounds sync, and a playback/session transition key prevents engine or session handoff from presenting stale recording feedback while timers and pending commands are cancelled. Same-session IPC replies yield to a broadcast snapshot received while the command was pending, so a successful recording acknowledgement cannot be rolled back by a stale reply. The built-in HTML5/hls.js player is the second guarded consumer: `HtmlVideoPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`, while its neutral `web-video-support` bridge is shared with ArtPlayer and owns HLS/Shaka(DASH)/native tracks, MPEG-TS VOD duration correction, caption preference, and source cleanup. `HtmlVideoElementSession` owns native video-event lifecycle, persisted volume, and start-time/time/ended propagation. Video.js is the third guarded consumer: `VjsPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`; its bridge rebinds the current Tech video after `playerreset`, exposes source-stable audio/subtitle IDs, preserves caption preference and explicit subtitle-off state, and reads Video.js duration. Reset-driven raw MPEG-TS changes pause first, coalesce to the latest desired source, preserve actual volume across Video.js's reset, and restart when authoritative live/VOD metadata changes. In shared-controls mode, Video.js native controls, click/double-click/hotkey actions, and spatial navigation are disabled. ArtPlayer is the fourth guarded consumer: `ArtPlayerComponent` provides a component-scoped `WebVideoControlsAdapter`; `ArtPlayerSourceSession` owns HLS/DASH(Shaka)/MPEG-TS/native sources, the neutral web-video bridge, exact cleanup, and a destroyed-session guard for delayed `customType` callbacks, while `ArtPlayerVideoSession` owns native media/ArtPlayer events. Shared ArtPlayer mode uses authoritative live/VOD metadata, HLS/Shaka/native tracks and caption preference, MPEG-TS VOD duration correction, and reapplies app volume directly after ArtPlayer restores its own stored volume. Vendor chrome/hotkeys are disabled, and a transparent capture layer gives shared controls exclusive click and double-click ownership. `WebPlayerViewComponent.resolvedIsLive` supplies authoritative metadata; visible playback diagnostics disable shared pointer/keyboard ownership and exit only the active HTMLine truncated
 - Shared web picture-in-picture stays inside that default-off rollout.
  `PlayerController` exposes capability `pictureInPicture`, state
  `pictureInPictureActive`/`canPictureInPicture`, and command
@@ -27,6 +27,8 @@ import {
    EMBEDDED_MPV_FRAME_SOURCE_CHANGED,
    EMBEDDED_MPV_SESSION_UPDATE,
    ResolvedPortalPlayback,
+    clampSubtitleDelay,
+    normalizeSubtitleStyle,
 } from '@iptvnator/shared/interfaces';
 import { toNativeViewBounds } from './embedded-mpv-bounds.util';
 import { EmbeddedMpvFrameCopyAdapter } from './embedded-mpv-frame-copy.adapter';
@@ -598,10 +600,9 @@ export class EmbeddedMpvNativeService {
                'Embedded MPV addon does not support subtitle delay. Rebuild the native addon to enable this feature.'
            );
        }
-        const clamped = Number.isFinite(seconds)
-            ? Math.max(-60, Math.min(60, seconds))
-            : 0;
-        addon.setSubtitleDelay(sessionId, clamped);
+        // Same rules as the renderer, same implementation: the shared helper
+        // is the one place the limits are defined.
+        addon.setSubtitleDelay(sessionId, clampSubtitleDelay(seconds));
        return this.refreshSession(sessionId);
    }

@@ -616,17 +617,8 @@ export class EmbeddedMpvNativeService {
                'Embedded MPV addon does not support subtitle styling. Rebuild the native addon to enable this feature.'
            );
        }
-        const sizePercent =
-            typeof style?.sizePercent === 'number' &&
-            Number.isFinite(style.sizePercent)
-                ? Math.max(25, Math.min(400, Math.round(style.sizePercent)))
-                : 100;
-        const color =
-            typeof style?.color === 'string' &&
-            /^#[0-9a-f]{6}$/i.test(style.color)
-                ? style.color
-                : null;
-        addon.setSubtitleStyle(sessionId, { sizePercent, color });
+        // Re-validate untrusted IPC input with the exact renderer rules.
+        addon.setSubtitleStyle(sessionId, normalizeSubtitleStyle(style));
        return this.refreshSession(sessionId);
    }

@@ -618,23 +618,44 @@ as the shared `volume` key, and is normalized/clamped on every read and write.
 The delay and any loaded file are deliberately per-session/per-source — they
 correct one specific stream.

+The canonical `PlayerSubtitleStyle` shape and the clamp/normalize rules
+(delay limit, size bounds, color validation) live in
+`@iptvnator/shared/interfaces` (`subtitle-style.util.ts`). The renderer
+applies them to user input and the Electron main process re-applies the exact
+same implementation to untrusted IPC payloads — deliberate defense-in-depth
+with a single source of truth, so widening a limit on one side cannot
+silently re-clamp on the other.
+
 Per-engine implementations:

 - **HTML5 + ArtPlayer (shared-controls mode, neutral source bridge).** The
  picker is a renderer-side DOM file input (`.srt`/`.vtt` only; works in the
-  PWA and Electron alike, and no filesystem path ever enters the app).
-  `WebVideoExternalSubtitles` parses the file
-  (`external-subtitle-cues.util.ts`) and renders it through a native
+  PWA and Electron alike, and no filesystem path ever enters the app). File
+  bytes are decoded encoding-aware (`decodeExternalSubtitleBytes`: UTF-16
+  BOMs, strict UTF-8, then a Windows-1251/1252 heuristic keyed on high-byte
+  density), because `Blob.text()`'s silent UTF-8 substitution turns common
+  legacy-encoded SRT files into mojibake. `WebVideoExternalSubtitles` parses
+  the file (`external-subtitle-cues.util.ts`) and renders it through a native
  `TextTrack` on the video element, so it works under every source kind. The
-  native track enumeration excludes externally owned tracks, and
-  `WebVideoSourceTracks` merges them into the subtitle listing with IDs from
-  100000 up, routing selection so exactly one owner (engine or external) is
-  active. The delay capability is runtime-gated on a loaded file: only owned
-  cues can be re-timed exactly, while engine/stream cues arrive incrementally
-  and are left untouched. Style applies through a scoped `::cue` rule
-  (`WebVideoSubtitleStyle`), which covers embedded, hls.js-managed, and
-  external native cues. ASS rendering would need libass and is out of scope
-  for the web engines.
+  native track enumeration excludes externally owned tracks — ownership is
+  tracked for every track the session EVER created, because `addTextTrack`
+  tracks cannot leave the element and per-source ownership would let stale or
+  attach-failed tracks reappear as ghost engine tracks. `WebVideoSourceTracks`
+  merges external tracks into the subtitle listing with IDs from 100000 up,
+  routing selection so exactly one owner (engine or external) is active;
+  external selection deselects the engine BEFORE setting track modes, since
+  hls.js reacts to `subtitleTrack = -1` by disabling every subtitle-kind
+  `TextTrack` on the element. A pick captures the source generation and is
+  discarded if the stream changed while the dialog was open (mirroring the
+  Embedded MPV runner's session recheck). The delay capability is
+  runtime-gated on an external track being the SELECTED one — only owned cues
+  can be re-timed exactly, and with an engine track active the row would be
+  enabled yet visually inert. Negatively shifted cues keep their real
+  (possibly negative) times, which are valid and simply never active;
+  clamping them to t≈0 would stack every pre-roll cue at playback start.
+  Style applies through a scoped `::cue` rule (`WebVideoSubtitleStyle`),
+  which covers embedded, hls.js-managed, and external native cues. ASS
+  rendering would need libass and is out of scope for the web engines.
 - **Embedded MPV frame-copy.** The helper protocol gained `sub-add`,
  `sub-delay`, `sub-scale`, and `sub-color` commands. The picker is a
  main-process open dialog (`.srt/.ass/.ssa/.vtt/.sub` — mpv renders ASS
@@ -5,6 +5,7 @@ export * from './lib/content-metadata.interface';
 export * from './lib/dev-logger.util';
 export * from './lib/download-metadata.interface';
 export * from './lib/embedded-mpv-session.interface';
+export * from './lib/subtitle-style.util';
 export * from './lib/electron-api.interface';
 export * from './lib/epg-channel-metadata.model';
 export * from './lib/epg-channel-with-programs.interface';
@@ -1,3 +1,5 @@
+import type { PlayerSubtitleStyle } from './subtitle-style.util';
+
 export type EmbeddedMpvSessionStatus =
    | 'idle'
    | 'loading'
@@ -29,14 +31,12 @@ export interface EmbeddedMpvCapabilities {
 }

 /**
- * Engine-neutral subtitle presentation preferences forwarded to mpv.
- * `sizePercent` maps to `sub-scale` (100 = 1.0); `color` maps to `sub-color`,
- * null restores mpv's default.
+ * Subtitle presentation preferences forwarded to mpv: `sizePercent` maps to
+ * `sub-scale` (100 = 1.0); `color` maps to `sub-color`, null restores mpv's
+ * default. Alias of the canonical shared shape so the renderer controls and
+ * the IPC contract cannot drift structurally.
 */
-export interface EmbeddedMpvSubtitleStyle {
-    sizePercent: number;
-    color: string | null;
-}
+export type EmbeddedMpvSubtitleStyle = PlayerSubtitleStyle;

 export type EmbeddedMpvEngine = 'native' | 'frame-copy';

@@ -0,0 +1,71 @@
+/**
+ * Canonical subtitle presentation contract shared by the renderer controls
+ * (`libs/ui/playback`) and the Electron main process. The clamp/normalize
+ * rules live HERE so the two sides cannot drift: the renderer applies them to
+ * user input, and the main process re-applies the exact same rules to
+ * untrusted IPC payloads (deliberate defense-in-depth, same implementation).
+ */
+
+/**
+ * Engine-neutral subtitle presentation preferences. `sizePercent` is relative
+ * to the engine's default rendering size (100 = default); `color` is a
+ * lowercase CSS hex color, or null for the engine default.
+ */
+export interface PlayerSubtitleStyle {
+    sizePercent: number;
+    color: string | null;
+}
+
+export const DEFAULT_SUBTITLE_STYLE: PlayerSubtitleStyle = {
+    sizePercent: 100,
+    color: null,
+};
+
+export const SUBTITLE_DELAY_LIMIT_SECONDS = 60;
+export const SUBTITLE_SIZE_MIN_PERCENT = 25;
+export const SUBTITLE_SIZE_MAX_PERCENT = 400;
+
+const HEX_COLOR_PATTERN = /^#[0-9a-f]{6}$/i;
+
+export function clampSubtitleDelay(seconds: number): number {
+    if (!Number.isFinite(seconds)) {
+        return 0;
+    }
+    const clamped = Math.max(
+        -SUBTITLE_DELAY_LIMIT_SECONDS,
+        Math.min(SUBTITLE_DELAY_LIMIT_SECONDS, seconds)
+    );
+    // Avoid float drift from repeated ±0.5 steps ("0.30000000000000004").
+    return Math.round(clamped * 1000) / 1000;
+}
+
+export function normalizeSubtitleStyle(value: unknown): PlayerSubtitleStyle {
+    if (typeof value !== 'object' || value === null) {
+        return { ...DEFAULT_SUBTITLE_STYLE };
+    }
+    const candidate = value as Partial<PlayerSubtitleStyle>;
+    const sizePercent =
+        typeof candidate.sizePercent === 'number' &&
+        Number.isFinite(candidate.sizePercent)
+            ? Math.max(
+                  SUBTITLE_SIZE_MIN_PERCENT,
+                  Math.min(
+                      SUBTITLE_SIZE_MAX_PERCENT,
+                      Math.round(candidate.sizePercent)
+                  )
+              )
+            : DEFAULT_SUBTITLE_STYLE.sizePercent;
+    const color =
+        typeof candidate.color === 'string' &&
+        HEX_COLOR_PATTERN.test(candidate.color)
+            ? candidate.color.toLowerCase()
+            : null;
+    return { sizePercent, color };
+}
+
+export function isDefaultSubtitleStyle(style: PlayerSubtitleStyle): boolean {
+    return (
+        style.sizePercent === DEFAULT_SUBTITLE_STYLE.sizePercent &&
+        style.color === DEFAULT_SUBTITLE_STYLE.color
+    );
+}
@@ -1,4 +1,5 @@
 import type { Signal } from '@angular/core';
+import type { PlayerSubtitleStyle } from '@iptvnator/shared/interfaces';

 export type PlayerStatus =
    | 'idle'
@@ -28,15 +29,9 @@ export interface PlayerControlsCapabilities {
    seriesNavigation: boolean;
 }

-/**
- * Engine-neutral subtitle presentation preferences. `sizePercent` is relative
- * to the engine's default rendering size (100 = default); `color` is a CSS hex
- * color or null for the engine default.
- */
-export interface PlayerSubtitleStyle {
-    sizePercent: number;
-    color: string | null;
-}
+// Canonical shape lives in @iptvnator/shared/interfaces so the Electron main
+// process validates IPC payloads against the identical contract.
+export type { PlayerSubtitleStyle };

 export interface PlayerTrack {
    id: number;
@@ -95,5 +95,10 @@ describe('subtitle-style', () => {
            expect(subtitleDelayLabel(0.5)).toBe('+0.5 s');
            expect(subtitleDelayLabel(-1.5)).toBe('−1.5 s');
        });
+
+        it('never shows a signed negative zero for sub-tenth values', () => {
+            expect(subtitleDelayLabel(0.02)).toBe('0 s');
+            expect(subtitleDelayLabel(-0.04)).toBe('0 s');
+        });
    });
 });
@@ -1,19 +1,36 @@
-import type { PlayerPreset, PlayerSubtitleStyle } from './player-controls.model';
+import {
+    DEFAULT_SUBTITLE_STYLE,
+    type PlayerSubtitleStyle,
+    isDefaultSubtitleStyle,
+    normalizeSubtitleStyle,
+} from '@iptvnator/shared/interfaces';
+import type { PlayerPreset } from './player-controls.model';

 /**
- * Shared subtitle presentation preferences. The style (size/color) persists
- * across sessions through the same localStorage mechanism the players already
- * use for the shared 'volume' key, so every engine adapter reads one source of
- * truth. The delay and any loaded external subtitle file are deliberately
- * per-session: they correct one specific stream, not a user preference.
+ * UI-side subtitle presentation preferences. The canonical shape and the
+ * clamp/normalize rules live in `@iptvnator/shared/interfaces`
+ * (`subtitle-style.util.ts`) so the Electron main process re-validates IPC
+ * input with the exact same implementation; this file adds only what the
+ * controls UI needs — presets, the delay step, persistence, and labels.
+ *
+ * The style (size/color) persists across sessions through the same
+ * localStorage mechanism the players already use for the shared 'volume' key,
+ * so every engine adapter reads one source of truth. The delay and any loaded
+ * external subtitle file are deliberately per-session: they correct one
+ * specific stream, not a user preference.
 */

-export const SUBTITLE_STYLE_STORAGE_KEY = 'subtitleStyle';
+export {
+    DEFAULT_SUBTITLE_STYLE,
+    SUBTITLE_DELAY_LIMIT_SECONDS,
+    SUBTITLE_SIZE_MAX_PERCENT,
+    SUBTITLE_SIZE_MIN_PERCENT,
+    clampSubtitleDelay,
+    isDefaultSubtitleStyle,
+    normalizeSubtitleStyle,
+} from '@iptvnator/shared/interfaces';

-export const DEFAULT_SUBTITLE_STYLE: PlayerSubtitleStyle = {
-    sizePercent: 100,
-    color: null,
-};
+export const SUBTITLE_STYLE_STORAGE_KEY = 'subtitleStyle';

 export const SUBTITLE_SIZE_PRESETS: ReadonlyArray<PlayerPreset<number>> = [
    { value: 75, label: '75%' },
@@ -36,51 +53,6 @@ export const SUBTITLE_COLOR_PRESETS: ReadonlyArray<PlayerPreset<string | null>>
    ];

 export const SUBTITLE_DELAY_STEP_SECONDS = 0.5;
-export const SUBTITLE_DELAY_LIMIT_SECONDS = 60;
-
-const MIN_SIZE_PERCENT = 25;
-const MAX_SIZE_PERCENT = 400;
-const HEX_COLOR_PATTERN = /^#[0-9a-f]{6}$/i;
-
-export function clampSubtitleDelay(seconds: number): number {
-    if (!Number.isFinite(seconds)) {
-        return 0;
-    }
-    const clamped = Math.max(
-        -SUBTITLE_DELAY_LIMIT_SECONDS,
-        Math.min(SUBTITLE_DELAY_LIMIT_SECONDS, seconds)
-    );
-    // Avoid float drift from repeated ±0.5 steps ("0.30000000000000004").
-    return Math.round(clamped * 1000) / 1000;
-}
-
-export function normalizeSubtitleStyle(value: unknown): PlayerSubtitleStyle {
-    if (typeof value !== 'object' || value === null) {
-        return { ...DEFAULT_SUBTITLE_STYLE };
-    }
-    const candidate = value as Partial<PlayerSubtitleStyle>;
-    const sizePercent =
-        typeof candidate.sizePercent === 'number' &&
-        Number.isFinite(candidate.sizePercent)
-            ? Math.max(
-                  MIN_SIZE_PERCENT,
-                  Math.min(MAX_SIZE_PERCENT, Math.round(candidate.sizePercent))
-              )
-            : DEFAULT_SUBTITLE_STYLE.sizePercent;
-    const color =
-        typeof candidate.color === 'string' &&
-        HEX_COLOR_PATTERN.test(candidate.color)
-            ? candidate.color.toLowerCase()
-            : null;
-    return { sizePercent, color };
-}
-
-export function isDefaultSubtitleStyle(style: PlayerSubtitleStyle): boolean {
-    return (
-        style.sizePercent === DEFAULT_SUBTITLE_STYLE.sizePercent &&
-        style.color === DEFAULT_SUBTITLE_STYLE.color
-    );
-}

 export function readStoredSubtitleStyle(): PlayerSubtitleStyle {
    try {
@@ -112,10 +84,15 @@ export function persistSubtitleStyle(style: PlayerSubtitleStyle): void {

 /** "+0.5 s" / "−1.5 s" / "0 s" display label for the delay row. */
 export function subtitleDelayLabel(seconds: number): string {
-    if (!Number.isFinite(seconds) || seconds === 0) {
+    if (!Number.isFinite(seconds)) {
        return '0 s';
    }
    const rounded = Math.round(seconds * 10) / 10;
+    // Derive the sign AFTER rounding: 0.02 rounds to 0 and must render as
+    // "0 s", not "−0.0 s".
+    if (rounded === 0) {
+        return '0 s';
+    }
    const magnitude = Math.abs(rounded).toFixed(1);
    return `${rounded > 0 ? '+' : '−'}${magnitude} s`;
 }
@@ -1,4 +1,5 @@
 import {
+    decodeExternalSubtitleBytes,
    detectExternalSubtitleFormat,
    parseExternalSubtitleCues,
 } from './external-subtitle-cues.util';
@@ -18,6 +19,54 @@ describe('external-subtitle-cues.util', () => {
        });
    });

+    describe('decodeExternalSubtitleBytes', () => {
+        const toBuffer = (bytes: number[]): ArrayBuffer =>
+            Uint8Array.from(bytes).buffer;
+
+        it('decodes valid UTF-8 as-is', () => {
+            const utf8 = new TextEncoder().encode('Привет\nmonde');
+            expect(decodeExternalSubtitleBytes(utf8.buffer)).toBe(
+                'Привет\nmonde'
+            );
+        });
+
+        it('decodes Cyrillic Windows-1251 bytes (high-byte-heavy text)', () => {
+            // "Привет мир" in CP1251: every letter is a high byte.
+            const cp1251 = [
+                0xcf, 0xf0, 0xe8, 0xe2, 0xe5, 0xf2, 0x20, 0xec, 0xe8, 0xf0,
+            ];
+            expect(decodeExternalSubtitleBytes(toBuffer(cp1251))).toBe(
+                'Привет мир'
+            );
+        });
+
+        it('decodes mostly-ASCII Windows-1252 bytes (sparse accents)', () => {
+            // "resume: cafe" with two accented letters among ASCII.
+            const cp1252 = [
+                ...Array.from('r').map((c) => c.charCodeAt(0)),
+                0xe9, // é
+                ...Array.from('sum').map((c) => c.charCodeAt(0)),
+                0xe9, // é
+                ...Array.from(': cafe and plain ascii words').map((c) =>
+                    c.charCodeAt(0)
+                ),
+            ];
+            expect(decodeExternalSubtitleBytes(toBuffer(cp1252))).toBe(
+                'résumé: cafe and plain ascii words'
+            );
+        });
+
+        it('honors a UTF-16LE byte-order mark', () => {
+            const text = '1\n00:00:01,000';
+            const bytes: number[] = [0xff, 0xfe];
+            for (const char of text) {
+                const code = char.charCodeAt(0);
+                bytes.push(code & 0xff, code >> 8);
+            }
+            expect(decodeExternalSubtitleBytes(toBuffer(bytes))).toBe(text);
+        });
+    });
+
    describe('parseExternalSubtitleCues', () => {
        it('parses a standard SRT file', () => {
            const content = [
@@ -39,6 +39,49 @@ export function detectExternalSubtitleFormat(
    return null;
 }

+/**
+ * Decodes a picked subtitle file's bytes. `Blob.text()` is strictly UTF-8 with
+ * silent U+FFFD substitution, which turns the still-common legacy-encoded SRT
+ * files (Windows-1251 Cyrillic, Windows-1252 Western European) and Windows'
+ * UTF-16 saves into mojibake that parses "successfully". Best-effort order:
+ * UTF-16 BOMs, then strict UTF-8, then a single-byte fallback chosen by how
+ * much of the text is non-ASCII — a non-Latin script (Cyrillic) makes high
+ * bytes dominate, while Latin text with accents stays mostly ASCII.
+ */
+export function decodeExternalSubtitleBytes(buffer: ArrayBuffer): string {
+    const bytes = new Uint8Array(buffer);
+    if (bytes.length >= 2) {
+        if (bytes[0] === 0xff && bytes[1] === 0xfe) {
+            return new TextDecoder('utf-16le').decode(buffer);
+        }
+        if (bytes[0] === 0xfe && bytes[1] === 0xff) {
+            return new TextDecoder('utf-16be').decode(buffer);
+        }
+    }
+    try {
+        return new TextDecoder('utf-8', { fatal: true }).decode(buffer);
+    } catch {
+        // Not valid UTF-8: a legacy single-byte encoding.
+    }
+
+    let highBytes = 0;
+    for (const byte of bytes) {
+        if (byte >= 0x80) {
+            highBytes += 1;
+        }
+    }
+    const encoding =
+        highBytes / Math.max(1, bytes.length) > 0.15
+            ? 'windows-1251'
+            : 'windows-1252';
+    try {
+        return new TextDecoder(encoding).decode(buffer);
+    } catch {
+        // Runtime without legacy decoders: non-fatal UTF-8 is the last resort.
+        return new TextDecoder().decode(buffer);
+    }
+}
+
 // SRT uses "00:00:01,500"; VTT uses "00:00:01.500" and allows a missing hour
 // part ("01:23.456"). One pattern covers both.
 const TIMESTAMP_PATTERN =
@@ -121,9 +121,11 @@ describe('WebVideoExternalSubtitles', () => {
            [12.5, 14.5],
        ]);

-        // Negative delay clamps a cue start at zero without inverting it.
+        // Negative delay keeps real cue times: negative values are legal and
+        // simply never active. Clamping to [0, ~0] would stack every pre-roll
+        // cue simultaneously at t=0.
        session.setDelay(-2);
-        expect(track.added[0].startTime).toBe(0);
+        expect(track.added[0].startTime).toBe(-1);
        expect(track.added[0].endTime).toBe(1);

        session.setDelay(0);
@@ -162,6 +164,60 @@ describe('WebVideoExternalSubtitles', () => {
        expect(track.added).toHaveLength(0);
        expect(session.hasTracks()).toBe(false);
        expect(session.getDelay()).toBe(0);
+        // addTextTrack tracks cannot leave the element: ownership must
+        // survive clear() so the native enumeration keeps excluding them
+        // instead of listing ghost tracks on the next source.
+        expect(session.ownsTrack(track as unknown as TextTrack)).toBe(true);
+    });
+
+    it('keeps the external track showing when engine deselect disables all tracks (hls.js)', () => {
+        // hls.js reacts to `subtitleTrack = -1` by disabling every
+        // subtitle-kind TextTrack on the element. select() must deselect the
+        // engine BEFORE setting its own modes so its writes win.
+        deselectEngineSubtitles.mockImplementation(() => {
+            for (const result of video.addTextTrack.mock.results) {
+                (result.value as FakeTextTrack).mode = 'disabled';
+            }
+        });
+
+        expect(session.addFromFile(SRT_FILE)).toBe(true);
+        expect(lastTrack().mode).toBe('showing');
+    });
+
+    it('reports selection separately from loaded files', () => {
+        session.addFromFile(SRT_FILE);
+        expect(session.hasTracks()).toBe(true);
+        expect(session.hasSelectedTrack()).toBe(true);
+
+        session.deselectAll();
+        expect(session.hasTracks()).toBe(true);
+        expect(session.hasSelectedTrack()).toBe(false);
+    });
+
+    it('silences and retains ownership of a track whose attach fails mid-file', () => {
+        const failingTrack = new FakeTextTrack('broken.srt');
+        let added = 0;
+        failingTrack.addCue = (cue: FakeVTTCue) => {
+            added += 1;
+            if (added > 1) {
+                throw new Error('addCue rejected');
+            }
+            failingTrack.added.push(cue);
+        };
+        video.addTextTrack.mockReturnValueOnce(failingTrack);
+
+        expect(
+            session.addFromFile({ ...SRT_FILE, name: 'broken.srt' })
+        ).toBe(false);
+
+        expect(session.hasTracks()).toBe(false);
+        // The half-populated track is silenced, emptied, and stays owned so
+        // the native enumeration cannot surface it as a phantom engine track.
+        expect(failingTrack.mode).toBe('disabled');
+        expect(failingTrack.added).toHaveLength(0);
+        expect(
+            session.ownsTrack(failingTrack as unknown as TextTrack)
+        ).toBe(true);
    });

    it('fails closed when the runtime lacks addTextTrack or VTTCue', () => {
@@ -4,6 +4,7 @@ import {
    type ExternalSubtitleFile,
    type ParsedSubtitleCue,
    WEB_SUBTITLE_FILE_EXTENSIONS,
+    decodeExternalSubtitleBytes,
    detectExternalSubtitleFormat,
    parseExternalSubtitleCues,
 } from './external-subtitle-cues.util';
@@ -43,6 +44,13 @@ export class WebVideoExternalSubtitles {
    private entries: ExternalSubtitleEntry[] = [];
    private delaySeconds = 0;
    private nextId = EXTERNAL_SUBTITLE_TRACK_ID_BASE;
+    /**
+     * Every TextTrack this session ever created. `addTextTrack` tracks cannot
+     * be removed from the element, so ownership must outlive `clear()` — a
+     * dropped-per-source set would let stale (or attach-failed) tracks
+     * reappear in the native enumeration as ghost engine tracks.
+     */
+    private readonly createdTracks = new Set<TextTrack>();

    constructor(private readonly config: WebVideoExternalSubtitlesConfig) {}

@@ -50,8 +58,13 @@ export class WebVideoExternalSubtitles {
        return this.entries.length > 0;
    }

+    /** True while an external track is the one actually rendering. */
+    hasSelectedTrack(): boolean {
+        return this.entries.some((entry) => entry.track?.mode === 'showing');
+    }
+
    ownsTrack(track: TextTrack): boolean {
-        return this.entries.some((entry) => entry.track === track);
+        return this.createdTracks.has(track);
    }

    ownsTrackId(id: number): boolean {
@@ -94,12 +107,15 @@ export class WebVideoExternalSubtitles {
        if (!this.ownsTrackId(id)) {
            return;
        }
+        // Deselect the engine FIRST: hls.js reacts to `subtitleTrack = -1` by
+        // disabling every subtitle-kind TextTrack on the element, which would
+        // immediately undo a mode we had already set.
+        this.config.deselectEngineSubtitles();
        for (const entry of this.entries) {
            if (entry.track) {
                entry.track.mode = entry.id === id ? 'showing' : 'hidden';
            }
        }
-        this.config.deselectEngineSubtitles();
        this.config.refresh();
    }

@@ -151,8 +167,11 @@ export class WebVideoExternalSubtitles {

        try {
            const track = video.addTextTrack('subtitles', entry.label);
+            this.createdTracks.add(track);
            entry.track = track;
-            entry.trackCues = entry.cues.map((cue) => {
+            // Push incrementally so the catch below can remove exactly the
+            // cues that made it onto the track before a mid-loop failure.
+            for (const cue of entry.cues) {
                const shifted = this.shiftCueTimes(cue);
                const vttCue = new CueCtor(
                    shifted.startSeconds,
@@ -160,10 +179,14 @@ export class WebVideoExternalSubtitles {
                    cue.text
                );
                track.addCue(vttCue);
-                return vttCue;
-            });
+                entry.trackCues.push(vttCue);
+            }
            return true;
        } catch {
+            // A mid-loop failure leaves an unremovable track on the element:
+            // silence it so the half-populated cue set can never render. It
+            // stays in `createdTracks`, so the enumeration keeps excluding it.
+            this.detachEntry(entry);
            return false;
        }
    }
@@ -197,12 +220,13 @@ export class WebVideoExternalSubtitles {
        startSeconds: number;
        endSeconds: number;
    } {
-        const startSeconds = Math.max(0, cue.startSeconds + this.delaySeconds);
-        const endSeconds = Math.max(
-            startSeconds + 0.001,
-            cue.endSeconds + this.delaySeconds
-        );
-        return { startSeconds, endSeconds };
+        // No clamping: negative cue times are valid VTTCue values that are
+        // simply never active. Clamping early cues to [0, ~0] would stack
+        // every pre-roll cue simultaneously at t=0 under a negative delay.
+        return {
+            startSeconds: cue.startSeconds + this.delaySeconds,
+            endSeconds: cue.endSeconds + this.delaySeconds,
+        };
    }
 }

@@ -227,8 +251,15 @@ export function pickExternalSubtitleFile(
        if (!format) {
            return;
        }
-        void file.text().then(
-            (content) => onPicked({ name: file.name, format, content }),
+        // Raw bytes, not file.text(): legacy encodings (CP1251/1252, UTF-16)
+        // are still common for downloaded subtitles and need detection.
+        void file.arrayBuffer().then(
+            (buffer) =>
+                onPicked({
+                    name: file.name,
+                    format,
+                    content: decodeExternalSubtitleBytes(buffer),
+                }),
            () => undefined
        );
    });
@@ -99,8 +99,15 @@ export class WebVideoSourceControlsBridge {
    }

    private pickExternalSubtitle(): void {
+        // The picker is modal-slow: the stream can change (Up Next, zapping,
+        // failover) before the user confirms. A pick made for one source must
+        // never attach to its successor.
+        const generation = this.tracks.getSourceGeneration();
        pickExternalSubtitleFile(this.config.video.ownerDocument, (file) => {
-            if (this.destroyed) {
+            if (
+                this.destroyed ||
+                this.tracks.getSourceGeneration() !== generation
+            ) {
                return;
            }
            this.tracks.addExternalSubtitleFile(file);
@@ -0,0 +1,98 @@
+import { EXTERNAL_SUBTITLE_TRACK_ID_BASE } from './web-video-external-subtitles';
+import { WebVideoSourceTracks } from './web-video-source-tracks';
+
+class FakeVTTCue {
+    constructor(
+        public startTime: number,
+        public endTime: number,
+        public text: string
+    ) {}
+}
+
+class FakeTextTrack {
+    kind = 'subtitles';
+    mode: TextTrackMode = 'hidden';
+    readonly added: FakeVTTCue[] = [];
+
+    constructor(public label: string) {}
+
+    addCue(cue: FakeVTTCue): void {
+        this.added.push(cue);
+    }
+
+    removeCue(cue: FakeVTTCue): void {
+        const index = this.added.indexOf(cue);
+        if (index >= 0) {
+            this.added.splice(index, 1);
+        }
+    }
+}
+
+const SRT_FILE = {
+    name: 'movie.srt',
+    format: 'srt' as const,
+    content: ['1', '00:00:01,000 --> 00:00:03,000', 'Hello', ''].join('\n'),
+};
+
+function createFakeVideo() {
+    return {
+        textTracks: {
+            length: 0,
+            addEventListener: jest.fn(),
+            removeEventListener: jest.fn(),
+        },
+        addTextTrack: jest.fn(
+            (_kind: string, label: string) => new FakeTextTrack(label)
+        ),
+    } as unknown as HTMLVideoElement;
+}
+
+describe('WebVideoSourceTracks external subtitle integration', () => {
+    let tracks: WebVideoSourceTracks;
+
+    beforeEach(() => {
+        (globalThis as { VTTCue?: unknown }).VTTCue = FakeVTTCue;
+        tracks = new WebVideoSourceTracks({
+            video: createFakeVideo(),
+            showCaptions: () => false,
+        });
+        tracks.setSource({ kind: 'native' });
+    });
+
+    afterEach(() => {
+        tracks.destroy();
+        delete (globalThis as { VTTCue?: unknown }).VTTCue;
+    });
+
+    it('advances the source generation on every source change', () => {
+        const initial = tracks.getSourceGeneration();
+        tracks.setSource({ kind: 'mpegts' });
+        expect(tracks.getSourceGeneration()).toBe(initial + 1);
+        tracks.clearSource();
+        expect(tracks.getSourceGeneration()).toBe(initial + 2);
+    });
+
+    it('offers delay adjustment only while an external track is selected', () => {
+        expect(tracks.canAdjustSubtitleDelay()).toBe(false);
+
+        expect(tracks.addExternalSubtitleFile(SRT_FILE)).toBe(true);
+        expect(tracks.canAdjustSubtitleDelay()).toBe(true);
+
+        // Turning subtitles off deselects the external track; the delay UI
+        // must retire with it — otherwise it is enabled yet visually inert.
+        tracks.setSubtitleTrack(-1);
+        expect(tracks.canAdjustSubtitleDelay()).toBe(false);
+
+        tracks.setSubtitleTrack(EXTERNAL_SUBTITLE_TRACK_ID_BASE);
+        expect(tracks.canAdjustSubtitleDelay()).toBe(true);
+    });
+
+    it('drops loaded external files with the source they corrected', () => {
+        tracks.addExternalSubtitleFile(SRT_FILE);
+        expect(tracks.getSubtitleTracks()).toHaveLength(1);
+
+        tracks.setSource({ kind: 'native' });
+        expect(tracks.getSubtitleTracks()).toHaveLength(0);
+        expect(tracks.canAdjustSubtitleDelay()).toBe(false);
+    });
+});
@@ -45,6 +45,7 @@ export class WebVideoSourceTracks {
    private readonly nativeTextTracks: WebVideoNativeTextTracks;
    private readonly externalSubtitles: WebVideoExternalSubtitles;
    private source: WebVideoControlsSource | null = null;
+    private sourceGeneration = 0;
    private playbackStarted = false;
    private playingListener: (() => void) | null = null;
    private destroyed = false;
@@ -90,11 +91,21 @@ export class WebVideoSourceTracks {
        return this.source?.kind ?? null;
    }

+    /**
+     * Changes whenever the bound source changes. Asynchronous work started
+     * against one source (the external subtitle file picker) captures this and
+     * bails when it no longer matches, so a pick cannot land on a later stream.
+     */
+    getSourceGeneration(): number {
+        return this.sourceGeneration;
+    }
+
    setSource(source: WebVideoControlsSource): void {
        if (this.destroyed) {
            return;
        }

+        this.sourceGeneration += 1;
        this.clearActiveSource();
        this.source = source;
        // A new source starts unsettled so its own defaults are seeded again.
@@ -127,6 +138,7 @@ export class WebVideoSourceTracks {
            return;
        }

+        this.sourceGeneration += 1;
        this.clearActiveSource();
        this.source = null;
    }
@@ -197,9 +209,13 @@ export class WebVideoSourceTracks {
        this.externalSubtitles.setDelay(seconds);
    }

-    /** Delay applies to owned external cues only, so it needs a loaded file. */
+    /**
+     * Delay re-times owned external cues only, so the UI is offered exactly
+     * while an external track is the one rendering — with an engine track
+     * selected the row would be enabled yet visually inert.
+     */
    canAdjustSubtitleDelay(): boolean {
-        return this.externalSubtitles.hasTracks();
+        return this.externalSubtitles.hasSelectedTrack();
    }

    private getEngineSubtitleTracks(): PlayerTrack[] {