refactor: streamline chapter handling by introducing cleanup registries for per-chapter and persistent resources, and update URL signing logic in prepareChaptersVttSrc

Author: aman-imagekit

packages/video-player/javascript/modules/chapters/chapters.ts

@@ -1,7 +1,7 @@
 import type Player from 'video.js/dist/types/player';
 import type { SourceOptions, IKPlayerOptions } from '../../interfaces';
 import { ChapterMarker, parseChaptersFromVTT } from './chapterMarkerProgressBar';
-import { prepareChaptersVttSrc } from '../../utils';
+import { prepareChaptersVttSrc, CleanupRegistry } from '../../utils';
 
 interface ChapterTrackMetadata {
   langCode: string;
@@ -12,6 +12,12 @@ interface ChapterTrackMetadata {
 // Map to store chapter tracks by language
 const chapterTracksCache = new Map<string, ChapterTrackMetadata>();
 
+// Cleanup registry for per-chapter resources (tracks, labels, progress bar)
+let perChapterCleanup: CleanupRegistry | null = null;
+
+// Cleanup registry for persistent resources (subtitle sync listener)
+let persistentCleanup: CleanupRegistry | null = null;
+
 /**
  * Clean up existing chapter text tracks from the player.
  */
@@ -48,17 +54,39 @@ function cleanupChapterLabelDisplay(player: Player): void {
 
 /**
  * Clean up all chapter-related components from the player.
- * Removes chapter text tracks, label display, and progress bar control.
+ * Removes chapter text tracks, label display, progress bar control, and per-chapter event listeners.
+ * Does NOT clean up persistent listeners (like subtitle sync).
  */
 function cleanupChapters(player: Player): void {
   cleanupChapterTextTracks(player);
   cleanupChapterLabelDisplay(player);
+  
+  // Dispose per-chapter cleanups (cuechange listeners, etc.)
+  if (perChapterCleanup) {
+    perChapterCleanup.dispose();
+    perChapterCleanup = null;
+  }
+  
   const existing = player.getChild('ChapterMarkersProgressBarControl');
   if (existing) {
     existing.dispose();
   }
 }
 
+/**
+ * Clean up ALL chapter resources including persistent listeners.
+ * Should be called when completely removing chapters (e.g., new source).
+ */
+function cleanupAllChapters(player: Player): void {
+  cleanupChapters(player);
+  
+  // Dispose persistent cleanups (subtitle sync listener)
+  if (persistentCleanup) {
+    persistentCleanup.dispose();
+    persistentCleanup = null;
+  }
+}
+
 
 /**
  * Load chapters for a specific language
@@ -160,12 +188,19 @@ function applyChaptersToPlayer(player: Player, chapterList: ChapterMarker[]): vo
 function setupSubtitleChapterSync(player: Player): void {
   const textTracks = player.textTracks();
 
-  textTracks.addEventListener('change', () => {
+  // Initialize persistent cleanup registry if needed
+  if (!persistentCleanup) {
+    persistentCleanup = new CleanupRegistry();
+  }
+  
+  const handler = () => {
     let foundActiveTrack = false;
 
     // Check for active subtitle track
-    for (let i = 0; i < textTracks.length; i++) {
-      const track = textTracks[i];
+    // TextTrackList is array-like, iterate using index access
+    const textTracksList = textTracks as unknown as TextTrack[];
+    for (let i = 0; i < textTracksList.length; i++) {
+      const track = textTracksList[i];
 
       // Find the active subtitle track
       if ((track.kind === 'subtitles' || track.kind === 'captions') && track.mode === 'showing') {
@@ -184,7 +219,10 @@ function setupSubtitleChapterSync(player: Player): void {
     if (!foundActiveTrack && chapterTracksCache.has('base')) {
       switchChaptersLanguage(player, 'base');
     }
-  });
+  };
+  
+  // Register the event listener with persistent cleanup registry
+  persistentCleanup.registerEventListener(textTracks as unknown as EventTarget, 'change', handler);
 }
 
 /**
@@ -211,7 +249,7 @@ function setupChapterLabelDisplay(player: Player, chaptersTrack: TextTrack): voi
   spacer.classList.add('vjs-control-bar-chapter-wrapper');
   spacer.appendChild(controlBarChapterHolder);
 
-  chaptersTrack.addEventListener('cuechange', () => {
+  const cueChangeHandler = () => {
     // Safari needs Array.from() for activeCues
     const activeCues = Array.from(chaptersTrack.activeCues);
     if (activeCues.length > 0) {
@@ -220,7 +258,13 @@ function setupChapterLabelDisplay(player: Player, chaptersTrack: TextTrack): voi
     } else {
       controlBarChapterHolder.innerText = '';
     }
-  });
+  };
+
+  // Register the cuechange listener with per-chapter cleanup registry
+  if (!perChapterCleanup) {
+    perChapterCleanup = new CleanupRegistry();
+  }
+  perChapterCleanup.registerEventListener(chaptersTrack, 'cuechange', cueChangeHandler);
 }
 
 /**
@@ -236,7 +280,8 @@ export async function initChapterMarkers(
   ikGlobalSettings: IKPlayerOptions
 ): Promise<void> {
 
-  cleanupChapters(player);
+  // Clean up ALL chapters including persistent listeners for new source
+  cleanupAllChapters(player);
 
   // Clear cache for new source
   chapterTracksCache.clear();

packages/video-player/javascript/utils.ts

@@ -268,7 +268,7 @@ export async function prepareChaptersVttSrc(
   opts: IKPlayerOptions
 ): Promise<{ baseUrl: string; translatedUrls: Map<string, string> }> {
   let videoSrcUrl = input.src;
-  let chaptersVttSrc: string;
+  let baseUrl: string;
 
   const url = new URL(videoSrcUrl);
 
@@ -282,21 +282,12 @@ export async function prepareChaptersVttSrc(
 
   filterTrQueryParam(url, ALLOWED_TRANSFORM_PARAMS_CHAPTERS);
 
-  chaptersVttSrc = ikBuild({
+  baseUrl = ikBuild({
     src: url.toString(),
     urlEndpoint: '',
     transformation: [],
   });
 
-  if (opts.signerFn) {
-    try {
-      chaptersVttSrc = await opts.signerFn(chaptersVttSrc);
-    } catch (err) {
-      throw new Error(`Signing failed: ${err}`);
-    }
-  }
-
-  const baseUrl = chaptersVttSrc;
   const translatedUrls = new Map<string, string>();
 
   // Check if user has translation options in textTracks
@@ -321,8 +312,6 @@ export async function prepareChaptersVttSrc(
           // Sign the URL if signerFn is provided
           if (opts.signerFn) {
             try {
-              // remove ik-s and ik-t query params before signing
-              finalUrl = finalUrl.replace(/ik-s=[^&]*&?/g, '').replace(/ik-t=[^&]*&?/g, '');
               finalUrl = await opts.signerFn(finalUrl);
             } catch (err) {
               console.error(`Failed to sign translated chapter URL for ${langCode}:`, err);
@@ -336,6 +325,15 @@ export async function prepareChaptersVttSrc(
     }
   }
 
+  // sign the base URL if signerFn is provided
+  if (opts.signerFn) {
+    try {
+      baseUrl = await opts.signerFn(baseUrl);
+    } catch (err) {
+      throw new Error(`Signing failed: ${err}`);
+    }
+  }
+
   return { baseUrl, translatedUrls };
 }