fix: generate chapters menu only when needed and don't create orphane…

…d event listeners (#7604)
videojs · Mar 21, 2022 · 5af81ca · 5af81ca
1 parent 28bdc7d
commit 5af81ca
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 22 deletions.
diff --git a/src/js/control-bar/text-track-controls/chapters-button.js b/src/js/control-bar/text-track-controls/chapters-button.js
@@ -29,6 +29,12 @@ class ChaptersButton extends TextTrackButton {
    */
   constructor(player, options, ready) {
     super(player, options, ready);
+
+    this.selectCurrentItem_ = () => {
+      this.items.forEach(item => {
+        item.selected(this.track_.activeCues[0] === item.cue);
+      });
+    };
   }
 
   /**
@@ -56,10 +62,19 @@ class ChaptersButton extends TextTrackButton {
    * @listens TextTrackList#change
    */
   update(event) {
-    if (!this.track_ || (event && (event.type === 'addtrack' || event.type === 'removetrack'))) {
-      this.setTrack(this.findChaptersTrack());
+    if (event && event.track && event.track.kind !== 'chapters') {
+      return;
+    }
+
+    const track = this.findChaptersTrack();
+
+    if (track !== this.track_) {
+      this.setTrack(track);
+      super.update();
+    } else if (!this.items || (track && track.cues && track.cues.length !== this.items.length)) {
+      // Update the menu initially or if the number of cues has changed since set
+      super.update();
     }
-    super.update();
   }
 
   /**
@@ -86,6 +101,8 @@ class ChaptersButton extends TextTrackButton {
         remoteTextTrackEl.removeEventListener('load', this.updateHandler_);
       }
 
+      this.track_.removeEventListener('cuechange', this.selectCurrentItem_);
+
       this.track_ = null;
     }
 
@@ -100,6 +117,8 @@ class ChaptersButton extends TextTrackButton {
       if (remoteTextTrackEl) {
         remoteTextTrackEl.addEventListener('load', this.updateHandler_);
       }
+
+      this.track_.addEventListener('cuechange', this.selectCurrentItem_);
     }
   }
 
@@ -175,6 +194,7 @@ class ChaptersButton extends TextTrackButton {
 
     return items;
   }
+
 }
 
 /**

diff --git a/src/js/control-bar/text-track-controls/chapters-track-menu-item.js b/src/js/control-bar/text-track-controls/chapters-track-menu-item.js
@@ -3,7 +3,6 @@
  */
 import MenuItem from '../../menu/menu-item.js';
 import Component from '../../component.js';
-import * as Fn from '../../utils/fn.js';
 
 /**
  * The chapter track menu item
@@ -35,7 +34,6 @@ class ChaptersTrackMenuItem extends MenuItem {
 
     this.track = track;
     this.cue = cue;
-    track.addEventListener('cuechange', Fn.bind(this, this.update));
   }
 
   /**
@@ -52,23 +50,6 @@ class ChaptersTrackMenuItem extends MenuItem {
   handleClick(event) {
     super.handleClick();
     this.player_.currentTime(this.cue.startTime);
-    this.update(this.cue.startTime);
-  }
-
-  /**
-   * Update chapter menu item
-   *
-   * @param {EventTarget~Event} [event]
-   *        The `cuechange` event that caused this function to run.
-   *
-   * @listens TextTrack#cuechange
-   */
-  update(event) {
-    const cue = this.cue;
-    const currentTime = this.player_.currentTime();
-
-    // vjs.log(currentTime, cue.startTime);
-    this.selected(cue.startTime <= currentTime && currentTime < cue.endTime);
   }
 
 }

diff --git a/test/unit/tracks/text-track-controls.test.js b/test/unit/tracks/text-track-controls.test.js
@@ -527,3 +527,46 @@ QUnit.test('chapters should be displayed when remote track added and load event
 
   player.dispose();
 });
+
+QUnit.test('chapters button should update selected menu item', function(assert) {
+  const player = TestHelpers.makePlayer();
+
+  this.clock.tick(1000);
+
+  const chaptersEl = player.addRemoteTextTrack(chaptersTrack, true);
+
+  chaptersEl.track.addCue({
+    startTime: 0,
+    endTime: 2,
+    text: 'Chapter 1'
+  });
+  chaptersEl.track.addCue({
+    startTime: 2,
+    endTime: 4,
+    text: 'Chapter 2'
+  });
+
+  assert.equal(chaptersEl.track.cues.length, 2);
+
+  if (player.tech_.featuresNativeTextTracks) {
+    TestHelpers.triggerDomEvent(chaptersEl, 'load');
+  } else {
+    chaptersEl.trigger('load');
+  }
+
+  const menuItems = player.controlBar.chaptersButton.items;
+
+  assert.ok(menuItems.find(i => i.isSelected_) === menuItems[0], 'item with startTime 0 selected on init');
+
+  player.currentTime(4);
+  player.trigger('timeupdate');
+
+  assert.ok(menuItems.find(i => i.isSelected_) === menuItems[1], 'second item selected on cuechange');
+
+  player.currentTime(1);
+  player.trigger('timeupdate');
+
+  assert.ok(menuItems.find(i => i.isSelected_) === menuItems[0], 'first item selected on cuechange');
+
+  player.dispose();
+});