Skip to content

Commit

Permalink
fix: removeCue should work with native passed in cue (#4208)
Browse files Browse the repository at this point in the history
  • Loading branch information
brandonocasey authored Mar 23, 2017
1 parent dbfba28 commit f2b5a05
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 8 deletions.
14 changes: 6 additions & 8 deletions src/js/tracks/text-track.js
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ class TextTrack extends Track {

// make sure that `id` is copied over
cue.id = originalCue.id;
cue.originalCue_ = originalCue;
}

const tracks = this.tech_.textTracks();
Expand All @@ -376,20 +377,17 @@ class TextTrack extends Track {
* The cue to remove from our internal list
*/
removeCue(removeCue) {
let removed = false;
let i = this.cues_.length;

for (let i = 0, l = this.cues_.length; i < l; i++) {
while (i--) {
const cue = this.cues_[i];

if (cue === removeCue) {
if (cue === removeCue || (cue.originalCue_ && cue.originalCue_ === removeCue)) {
this.cues_.splice(i, 1);
removed = true;
this.cues.setCues_(this.cues_);
break;
}
}

if (removed) {
this.cues.setCues_(this.cues_);
}
}
}

Expand Down
40 changes: 40 additions & 0 deletions test/unit/tracks/text-track.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,46 @@ QUnit.test('cues can be added and removed from a TextTrack', function(assert) {
assert.equal(cues.length, 3, 'we now have 3 cues');
});

QUnit.test('original cue can be used to remove cue from cues list', function(assert) {
const tt = new TextTrack({
tech: this.tech
});
const Cue = window.VTTCue ||
window.vttjs && window.vttjs.VTTCue ||
window.TextTrackCue;

const cue1 = new Cue(0, 1, 'some-cue');

assert.equal(tt.cues.length, 0, 'start with zero cues');
tt.addCue(cue1);
assert.equal(tt.cues.length, 1, 'we have one cue');

tt.removeCue(cue1);
assert.equal(tt.cues.length, 0, 'we have removed cue1');
});

QUnit.test('can only remove one cue at a time', function(assert) {
const tt = new TextTrack({
tech: this.tech
});
const Cue = window.VTTCue ||
window.vttjs && window.vttjs.VTTCue ||
window.TextTrackCue;

const cue1 = new Cue(0, 1, 'some-cue');

assert.equal(tt.cues.length, 0, 'start with zero cues');
tt.addCue(cue1);
tt.addCue(cue1);
assert.equal(tt.cues.length, 2, 'we have two cues');

tt.removeCue(cue1);
assert.equal(tt.cues.length, 1, 'we have removed one instance of cue1');

tt.removeCue(cue1);
assert.equal(tt.cues.length, 0, 'we have removed the other instance of cue1');
});

QUnit.test('does not fire cuechange before Tech is ready', function(assert) {
const done = assert.async();
const player = TestHelpers.makePlayer({techfaker: {autoReady: false}});
Expand Down

0 comments on commit f2b5a05

Please sign in to comment.