Skip to content

Commit

Permalink
fix: remote text track deprecation warnings (#3864)
Browse files Browse the repository at this point in the history
  • Loading branch information
gkatsev authored Dec 15, 2016
1 parent 3f724f9 commit a7ffa34
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 13 deletions.
2 changes: 1 addition & 1 deletion src/js/tracks/text-track-display.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ class TextTrackDisplay extends Component {
const tracks = this.options_.playerOptions.tracks || [];

for (let i = 0; i < tracks.length; i++) {
this.player_.addRemoteTextTrack(tracks[i]);
this.player_.addRemoteTextTrack(tracks[i], true);
}

const modes = {captions: 1, subtitles: 1};
Expand Down
24 changes: 20 additions & 4 deletions test/unit/tech/tech.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import AudioTrackList from '../../../src/js/tracks/audio-track-list';
import VideoTrackList from '../../../src/js/tracks/video-track-list';
import TextTrackList from '../../../src/js/tracks/text-track-list';
import sinon from 'sinon';
import log from '../../../src/js/utils/log.js';

QUnit.module('Media Tech', {
beforeEach(assert) {
Expand Down Expand Up @@ -135,8 +136,8 @@ QUnit.test('dispose() should clear all tracks that are passed when its created',
QUnit.test('dispose() should clear all tracks that are added after creation', function(assert) {
const tech = new Tech();

tech.addRemoteTextTrack({});
tech.addRemoteTextTrack({});
tech.addRemoteTextTrack({}, true);
tech.addRemoteTextTrack({}, true);

tech.audioTracks().addTrack_(new AudioTrack());
tech.audioTracks().addTrack_(new AudioTrack());
Expand Down Expand Up @@ -168,6 +169,14 @@ QUnit.test('dispose() should clear all tracks that are added after creation', fu
});

QUnit.test('switching sources should clear all remote tracks that are added with manualCleanup = false', function(assert) {

const oldLogWarn = log.warn;
let warning;

log.warn = function(wrning) {
warning = wrning;
};

// Define a new tech class
const MyTech = extendFn(Tech);

Expand All @@ -194,6 +203,11 @@ QUnit.test('switching sources should clear all remote tracks that are added with

// default value for manualCleanup is true
tech.addRemoteTextTrack({});

assert.equal(warning,
'Calling addRemoteTextTrack without explicitly setting the "manualCleanup" parameter to `true` is deprecated and default to `false` in future version of video.js',
'we log a warning when `addRemoteTextTrack` is called without a manualCleanup argument');

// should be automatically cleaned up when source changes
tech.addRemoteTextTrack({}, false);

Expand Down Expand Up @@ -221,6 +235,8 @@ QUnit.test('switching sources should clear all remote tracks that are added with
assert.equal(tech.autoRemoteTextTracks_.length,
0,
'should have zero auto-cleanup remote text tracks');

log.warn = oldLogWarn;
});

QUnit.test('should add the source handler interface to a tech', function(assert) {
Expand Down Expand Up @@ -335,8 +351,8 @@ QUnit.test('should add the source handler interface to a tech', function(assert)
'',
'the Tech returned an empty string for the invalid source');

tech.addRemoteTextTrack({});
tech.addRemoteTextTrack({});
tech.addRemoteTextTrack({}, true);
tech.addRemoteTextTrack({}, true);

tech.audioTracks().addTrack_(new AudioTrack());
tech.audioTracks().addTrack_(new AudioTrack());
Expand Down
8 changes: 4 additions & 4 deletions test/unit/tracks/text-track-controls.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ QUnit.test('should be displayed when text tracks list is not empty', function(as
QUnit.test('should be displayed when a text track is added to an empty track list', function(assert) {
const player = TestHelpers.makePlayer();

player.addRemoteTextTrack(track);
player.addRemoteTextTrack(track, true);

assert.ok(!player.controlBar.captionsButton.hasClass('vjs-hidden'),
'control is displayed');
Expand Down Expand Up @@ -94,7 +94,7 @@ QUnit.test('menu should update with addRemoteTextTrack', function(assert) {

this.clock.tick(1000);

player.addRemoteTextTrack(track);
player.addRemoteTextTrack(track, true);

assert.equal(player.controlBar.captionsButton.items.length,
4,
Expand Down Expand Up @@ -143,7 +143,7 @@ QUnit.test('descriptions should be displayed when text tracks list is not empty'
QUnit.test('descriptions should be displayed when a text track is added to an empty track list', function(assert) {
const player = TestHelpers.makePlayer();

player.addRemoteTextTrack(descriptionstrack);
player.addRemoteTextTrack(descriptionstrack, true);

assert.ok(!player.controlBar.descriptionsButton.hasClass('vjs-hidden'),
'control is displayed');
Expand Down Expand Up @@ -421,7 +421,7 @@ test('chapters should be displayed when remote track added and load event fired'

this.clock.tick(1000);

const chaptersEl = player.addRemoteTextTrack(chaptersTrack);
const chaptersEl = player.addRemoteTextTrack(chaptersTrack, true);

chaptersEl.track.addCue({
startTime: 0,
Expand Down
8 changes: 4 additions & 4 deletions test/unit/tracks/text-tracks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -420,7 +420,7 @@ QUnit.test('should return correct remote text track values', function(assert) {
const htmlTrackElement = player.addRemoteTextTrack({
kind: 'captions',
label: 'label'
});
}, true);

assert.equal(player.remoteTextTracks().length, 2, 'add text track via method');
assert.equal(player.remoteTextTrackEls().length, 2, 'add html track element via method');
Expand All @@ -447,7 +447,7 @@ QUnit.test('should uniformly create html track element when adding text track',

assert.equal(player.remoteTextTrackEls().length, 0, 'no html text tracks');

const htmlTrackElement = player.addRemoteTextTrack(track);
const htmlTrackElement = player.addRemoteTextTrack(track, true);

assert.equal(htmlTrackElement.kind,
htmlTrackElement.track.kind,
Expand Down Expand Up @@ -539,7 +539,7 @@ QUnit.test('removeRemoteTextTrack should be able to take both a track and the re
label: 'label',
default: 'default'
};
let htmlTrackElement = player.addRemoteTextTrack(track);
let htmlTrackElement = player.addRemoteTextTrack(track, true);

assert.equal(player.remoteTextTrackEls().length, 1, 'html track element exist');

Expand All @@ -549,7 +549,7 @@ QUnit.test('removeRemoteTextTrack should be able to take both a track and the re
0,
'the track element was removed correctly');

htmlTrackElement = player.addRemoteTextTrack(track);
htmlTrackElement = player.addRemoteTextTrack(track, true);
assert.equal(player.remoteTextTrackEls().length, 1, 'html track element exist');

player.removeRemoteTextTrack(htmlTrackElement.track);
Expand Down

0 comments on commit a7ffa34

Please sign in to comment.