fix(web): initialize caption tracks after load

[puneetdixit200]

Summary

• add a shared caption-track binding helper for the web video players
• keep caption/subtitle tracks hidden for the custom overlay while listening for async <track> load events
• cover the self-hosted async VTT load path with a focused unit test

Fixes #1773.

Tests

• pnpm --dir apps/web exec vitest run tests/unit/caption-tracks.test.ts
• pnpm --dir apps/web exec vitest run tests/unit/caption-tracks.test.ts tests/unit/caption-cues.test.ts
• pnpm exec biome check "apps/web/app/s/[videoId]/_components/caption-tracks.ts" "apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx" "apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx" apps/web/tests/unit/caption-tracks.test.ts
• git diff --check

Note: a broad pnpm --dir apps/web exec tsc --noEmit --pretty false --incremental false run was attempted, but it stayed silent for several minutes and was stopped to avoid tying up the sweep environment.

Greptile Summary

Extracts caption-track binding logic into a shared bindCaptionTrackCueText helper and fixes the async VTT load race by waiting for the load event on <track> DOM elements before registering cuechange listeners, rather than relying solely on loadedmetadata.

• caption-tracks.ts — new helper uses a MutationObserver + <track> element load listeners + textTracks events to keep caption tracks in hidden mode and deliver cue-text changes; cleanup is comprehensive.
• CapVideoPlayer.tsx / HLSVideoPlayer.tsx — inline track-setup blocks replaced with a single bindCaptionTrackCueText call; event wiring and cleanup are correctly delegated to the helper.
• caption-tracks.test.ts — focused unit test validates the async load path for self-hosted VTT.

Confidence Score: 4/5

Safe to merge — the refactor is clean, event cleanup is correct in both players, and the core async-load fix addresses the reported issue.

The caption-track helper is well-structured and the two player components are straightforward call-site cleanups. The only concern is that trackLoadHandlers retains entries for <track> elements removed from the DOM until the binding is torn down, a mild memory retention issue. The test suite covers the motivating bug path but misses the synchronous already-loaded initialization case.

apps/web/app/s/[videoId]/_components/caption-tracks.ts — specifically the bindTrackElementLoads function and its interaction with track removal.

Important Files Changed

Filename	Overview
apps/web/app/s/[videoId]/_components/caption-tracks.ts	New shared helper that registers load listeners on <track> DOM elements and syncs TextTrack cuechange bindings; stale trackLoadHandlers entries for removed elements are not pruned during the binding's lifetime.
apps/web/tests/unit/caption-tracks.test.ts	New unit test covering the async VTT-load path; the synchronous already-loaded initialization path is untested.
apps/web/app/s/[videoId]/_components/CapVideoPlayer.tsx	Replaces inline track-setup logic with bindCaptionTrackCueText; cleanup and event wiring look correct.
apps/web/app/s/[videoId]/_components/HLSVideoPlayer.tsx	Replaces inline track-setup logic with bindCaptionTrackCueText; the returned cleanup is now returned directly from the effect, which is correct.

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
apps/web/app/s/[videoId]/_components/caption-tracks.ts:61-63
When `handleTrackUpdate` fires after a `<track>` element is removed from the DOM (via `MutationObserver` or `removetrack`), `syncTracks` correctly cleans up the `TextTrack` from `cueTracks`, but `trackLoadHandlers` is never pruned. The detached `HTMLTrackElement` and its handler closure are held in memory until the returned cleanup function runs. Pruning stale entries in `bindTrackElementLoads` keeps the Map tidy across the binding's lifetime.

```suggestion
	const bindTrackElementLoads = (): void => {
		const liveElements = new Set(video.querySelectorAll("track"));

		for (const [el, handler] of trackLoadHandlers) {
			if (!liveElements.has(el)) {
				el.removeEventListener("load", handler);
				trackLoadHandlers.delete(el);
			}
		}

		for (const trackElement of liveElements) {
			if (trackLoadHandlers.has(trackElement)) continue;
```

### Issue 2 of 2
apps/web/__tests__/unit/caption-tracks.test.ts:86-111
**Missing synchronous-init test case**

The suite only covers the async VTT-load path (track element loads after `loadedmetadata`). The synchronous path — where `<track>` elements are already in the DOM with `readyState === 2` before `bindCaptionTrackCueText` is called — is exercised by the immediate `handleTrackUpdate()` call at the end of setup, but is not tested. A regression here (e.g., the sync `syncTracks` call being dropped) would go undetected.

_{Reviews (1): Last reviewed commit: "fix(web): initialize caption tracks afte..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[greptile-apps]

When handleTrackUpdate fires after a <track> element is removed from the DOM (via MutationObserver or removetrack), syncTracks correctly cleans up the TextTrack from cueTracks, but trackLoadHandlers is never pruned. The detached HTMLTrackElement and its handler closure are held in memory until the returned cleanup function runs. Pruning stale entries in bindTrackElementLoads keeps the Map tidy across the binding's lifetime.

Suggested change

	const bindTrackElementLoads = (): void => {
	for (const trackElement of video.querySelectorAll("track")) {
	if (trackLoadHandlers.has(trackElement)) continue;
	const bindTrackElementLoads = (): void => {
	const liveElements = new Set(video.querySelectorAll("track"));
	for (const [el, handler] of trackLoadHandlers) {
	if (!liveElements.has(el)) {
	el.removeEventListener("load", handler);
	trackLoadHandlers.delete(el);
	}
	}
	for (const trackElement of liveElements) {
	if (trackLoadHandlers.has(trackElement)) continue;

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/app/s/[videoId]/_components/caption-tracks.ts
Line: 61-63

Comment:
When `handleTrackUpdate` fires after a `<track>` element is removed from the DOM (via `MutationObserver` or `removetrack`), `syncTracks` correctly cleans up the `TextTrack` from `cueTracks`, but `trackLoadHandlers` is never pruned. The detached `HTMLTrackElement` and its handler closure are held in memory until the returned cleanup function runs. Pruning stale entries in `bindTrackElementLoads` keeps the Map tidy across the binding's lifetime.

```suggestion
	const bindTrackElementLoads = (): void => {
		const liveElements = new Set(video.querySelectorAll("track"));

		for (const [el, handler] of trackLoadHandlers) {
			if (!liveElements.has(el)) {
				el.removeEventListener("load", handler);
				trackLoadHandlers.delete(el);
			}
		}

		for (const trackElement of liveElements) {
			if (trackLoadHandlers.has(trackElement)) continue;
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

Missing synchronous-init test case

The suite only covers the async VTT-load path (track element loads after loadedmetadata). The synchronous path — where <track> elements are already in the DOM with readyState === 2 before bindCaptionTrackCueText is called — is exercised by the immediate handleTrackUpdate() call at the end of setup, but is not tested. A regression here (e.g., the sync syncTracks call being dropped) would go undetected.

Prompt To Fix With AI

This is a comment left during a code review.
Path: apps/web/__tests__/unit/caption-tracks.test.ts
Line: 86-111

Comment:
**Missing synchronous-init test case**

The suite only covers the async VTT-load path (track element loads after `loadedmetadata`). The synchronous path — where `<track>` elements are already in the DOM with `readyState === 2` before `bindCaptionTrackCueText` is called — is exercised by the immediate `handleTrackUpdate()` call at the end of setup, but is not tested. A regression here (e.g., the sync `syncTracks` call being dropped) would go undetected.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!