Skip to content

Commit

Permalink
fix: group subtitles and captions when switching tracks (#6008)
Browse files Browse the repository at this point in the history
This fixes a regression created by #5741.
  • Loading branch information
alex-barstow authored and gkatsev committed May 24, 2019
1 parent 9954470 commit cd6be5b
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ class OffTextTrackMenuItem extends TextTrackMenuItem {
// Requires options['kind']
options.track = {
player,
// it is no longer necessary to store `kind` or `kinds` on the track itself
// since they are now stored in the `kinds` property of all instances of
// TextTrackMenuItem, but this will remain for backwards compatibility
kind: options.kind,
kinds: options.kinds,
default: false,
Expand Down
2 changes: 2 additions & 0 deletions src/js/control-bar/text-track-controls/text-track-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ class TextTrackButton extends TrackButton {

const item = new TrackMenuItem(this.player_, {
track,
kinds: this.kinds_,
kind: this.kind_,
// MenuItem is selectable
selectable: true,
// MenuItem is NOT multiSelectable (i.e. only one can be marked "selected" at a time)
Expand Down
10 changes: 5 additions & 5 deletions src/js/control-bar/text-track-controls/text-track-menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ class TextTrackMenuItem extends MenuItem {
super(player, options);

this.track = track;
// Determine the relevant kind(s) of tracks for this component and filter
// out empty kinds.
this.kinds = (options.kinds || [options.kind || this.track.kind]).filter(Boolean);

const changeHandler = (...args) => {
this.handleTracksChange.apply(this, args);
};
Expand Down Expand Up @@ -102,16 +106,12 @@ class TextTrackMenuItem extends MenuItem {
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 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) {
if (this.kinds.indexOf(track.kind) === -1) {
continue;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,39 +29,58 @@ QUnit.test('clicking should enable the selected track', function(assert) {
fooItem.dispose();
});

QUnit.test('clicking should disable non-selected tracks of the same kind', function(assert) {
assert.expect(9);
QUnit.test('clicking should disable non-selected tracks of the relevant kind(s)', function(assert) {
assert.expect(16);

const foo = this.player.addTextTrack('captions', 'foo', 'en');
const bar = this.player.addTextTrack('captions', 'bar', 'es');
const bip = this.player.addTextTrack('subtitles', 'bip', 'fr');
const bop = this.player.addTextTrack('metadata', 'bop');

bop.mode = 'hidden';

const fooItem = new TextTrackMenuItem(this.player, {
track: foo
track: foo,
kinds: ['captions', 'subtitles']
});

const barItem = new TextTrackMenuItem(this.player, {
track: bar
track: bar,
kinds: ['captions', 'subtitles']
});

const bipItem = new TextTrackMenuItem(this.player, {
track: bip,
kinds: ['captions', 'subtitles']
});

assert.strictEqual(foo.mode, 'disabled', 'captions track "foo" begins "disabled"');
assert.strictEqual(bar.mode, 'disabled', 'captions track "bar" begins "disabled"');
assert.strictEqual(bip.mode, 'disabled', 'subtitles track "bip" 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(bip.mode, 'disabled', 'subtitles track "bip" is still "disabled"');
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(bip.mode, 'disabled', 'subtitles track "bip" is still "disabled"');
assert.strictEqual(bop.mode, 'hidden', 'metadata track "bop" is still "hidden"');

bipItem.trigger('click');

assert.strictEqual(foo.mode, 'disabled', 'captions track "foo" is now "disabled"');
assert.strictEqual(bar.mode, 'disabled', 'captions track "bar" is still "disabled"');
assert.strictEqual(bip.mode, 'showing', 'subtitles track "bip" is now "showing"');
assert.strictEqual(bop.mode, 'hidden', 'metadata track "bop" is still "hidden"');

fooItem.dispose();
barItem.dispose();
bipItem.dispose();
});

0 comments on commit cd6be5b

Please sign in to comment.