fix: TextTrackMenuItem components should not disable text tracks of different kind(s). (videojs#5741)

misteroneill · gkatsev · commit b27f71347ece · 2019-01-10T14:24:15.000-05:00
diff --git a/src/js/control-bar/text-track-controls/text-track-menu-item.js b/src/js/control-bar/text-track-controls/text-track-menu-item.js
@@ -93,27 +93,37 @@ class TextTrackMenuItem extends MenuItem {
    * @listens click
    */
   handleClick(event) {
-    const kind = this.track.kind;
-    let kinds = this.track.kinds;
+    const referenceTrack = this.track;
     const tracks = this.player_.textTracks();
 
-    if (!kinds) {
-      kinds = [kind];
-    }
-
     super.handleClick(event);
 
     if (!tracks) {
       return;
     }
 
+    // Determine the relevant kind(s) of tracks for this component and filter
+    // out empty kinds.
+    const kinds = (referenceTrack.kinds || [referenceTrack.kind]).filter(Boolean);
+
     for (let i = 0; i < tracks.length; i++) {
       const track = tracks[i];
 
-      if (track === this.track && (kinds.indexOf(track.kind) > -1)) {
+      // If the track from the text tracks list is not of the right kind,
+      // skip it. We do not want to affect tracks of incompatible kind(s).
+      if (kinds.indexOf(track.kind) === -1) {
+        continue;
+      }
+
+      // If this text track is the component's track and it is not showing,
+      // set it to showing.
+      if (track === referenceTrack) {
         if (track.mode !== 'showing') {
           track.mode = 'showing';
         }
+
+      // If this text track is not the component's track and it is not
+      // disabled, set it to disabled.
       } else if (track.mode !== 'disabled') {
         track.mode = 'disabled';
       }
diff --git a/test/unit/control-bar/text-track-controls/text-track-menu-item.test.js b/test/unit/control-bar/text-track-controls/text-track-menu-item.test.js
@@ -0,0 +1,67 @@
+/* eslint-env qunit */
+import TextTrackMenuItem from '../../../../src/js/control-bar/text-track-controls/text-track-menu-item.js';
+import TestHelpers from '../../test-helpers.js';
+
+QUnit.module('TextTrackMenuItem', {
+  beforeEach(assert) {
+    this.player = TestHelpers.makePlayer();
+  },
+  afterEach(assert) {
+    this.player.dispose();
+  }
+});
+
+QUnit.test('clicking should enable the selected track', function(assert) {
+  assert.expect(2);
+
+  const foo = this.player.addTextTrack('captions', 'foo', 'en');
+
+  const fooItem = new TextTrackMenuItem(this.player, {
+    track: foo
+  });
+
+  assert.strictEqual(foo.mode, 'disabled', 'track "foo" begins "disabled"');
+
+  fooItem.trigger('click');
+
+  assert.strictEqual(foo.mode, 'showing', 'clicking set track "foo" to "showing"');
+
+  fooItem.dispose();
+});
+
+QUnit.test('clicking should disable non-selected tracks of the same kind', function(assert) {
+  assert.expect(9);
+
+  const foo = this.player.addTextTrack('captions', 'foo', 'en');
+  const bar = this.player.addTextTrack('captions', 'bar', 'es');
+  const bop = this.player.addTextTrack('metadata', 'bop');
+
+  bop.mode = 'hidden';
+
+  const fooItem = new TextTrackMenuItem(this.player, {
+    track: foo
+  });
+
+  const barItem = new TextTrackMenuItem(this.player, {
+    track: bar
+  });
+
+  assert.strictEqual(foo.mode, 'disabled', 'captions track "foo" begins "disabled"');
+  assert.strictEqual(bar.mode, 'disabled', 'captions track "bar" begins "disabled"');
+  assert.strictEqual(bop.mode, 'hidden', 'metadata track "bop" is "hidden"');
+
+  barItem.trigger('click');
+
+  assert.strictEqual(foo.mode, 'disabled', 'captions track "foo" is still "disabled"');
+  assert.strictEqual(bar.mode, 'showing', 'captions track "bar" is now "showing"');
+  assert.strictEqual(bop.mode, 'hidden', 'metadata track "bop" is still "hidden"');
+
+  fooItem.trigger('click');
+
+  assert.strictEqual(foo.mode, 'showing', 'captions track "foo" is now "showing"');
+  assert.strictEqual(bar.mode, 'disabled', 'captions track "bar" is now "disabled"');
+  assert.strictEqual(bop.mode, 'hidden', 'metadata track "bop" is still "hidden"');
+
+  fooItem.dispose();
+  barItem.dispose();
+});
