1
0
mirror of https://github.com/videojs/video.js.git synced 2025-03-05 15:16:06 +02:00

fix: cues at startTime 0 do not fire (#4152)

Previously timeupdate would fire before the video was playing, and the tech was not ready. This caused issues when preload was set to auto, because cuechange would fire before the video was even started for cues with a startTime of 0.

Wait for tech to be ready before watching for timeupdate
update unit tests to use TechFaker
Add a unit test to verify that we wait for Tech to be ready.
This commit is contained in:
Brandon Casey 2017-03-02 14:35:45 -05:00 committed by Gary Katsevman
parent 9ef2d07b41
commit a2b1a33606
5 changed files with 91 additions and 53 deletions

View File

@ -198,7 +198,9 @@ class TextTrack extends Track {
});
if (mode !== 'disabled') {
tt.tech_.on('timeupdate', timeupdateHandler);
tt.tech_.ready(() => {
tt.tech_.on('timeupdate', timeupdateHandler);
}, true);
}
/**
@ -236,7 +238,10 @@ class TextTrack extends Track {
}
mode = newMode;
if (mode === 'showing') {
this.tech_.on('timeupdate', timeupdateHandler);
this.tech_.ready(() => {
this.tech_.on('timeupdate', timeupdateHandler);
}, true);
}
/**
* An event that fires when mode changes on this track. This allows
@ -339,7 +344,7 @@ class TextTrack extends Track {
addCue(originalCue) {
let cue = originalCue;
if (!(originalCue instanceof window.vttjs.VTTCue)) {
if (window.vttjs && !(originalCue instanceof window.vttjs.VTTCue)) {
cue = new window.vttjs.VTTCue(originalCue.startTime, originalCue.endTime, originalCue.text);
for (const prop in originalCue) {
@ -347,6 +352,9 @@ class TextTrack extends Track {
cue[prop] = originalCue[prop];
}
}
// make sure that `id` is copied over
cue.id = originalCue.id;
}
const tracks = this.tech_.textTracks();

View File

@ -9,7 +9,9 @@ class TechFaker extends Tech {
constructor(options, handleReady) {
super(options, handleReady);
this.triggerReady();
if (!options || options.autoReady !== false) {
this.triggerReady();
}
}
createEl() {

View File

@ -1,17 +1,16 @@
/* eslint-env qunit */
import HTMLTrackElement from '../../../src/js/tracks/html-track-element.js';
import TextTrackList from '../../../src/js/tracks/text-track-list.js';
import TechFaker from '../tech/tech-faker';
const defaultTech = {
textTracks() {
return new TextTrackList();
QUnit.module('HTML Track Element', {
beforeEach() {
this.tech = new TechFaker();
},
on() {},
off() {},
currentTime() {}
};
QUnit.module('HTML Track Element');
afterEach() {
this.tech.dispose();
this.tech = null;
}
});
QUnit.test('html track element requires a tech', function(assert) {
assert.throws(
@ -34,7 +33,7 @@ QUnit.test('can create a html track element with various properties', function(a
label,
language,
src,
tech: defaultTech
tech: this.tech
});
assert.equal(typeof htmlTrackElement.default, 'undefined', 'we have a default');
@ -48,7 +47,7 @@ QUnit.test('can create a html track element with various properties', function(a
QUnit.test('defaults when items not provided', function(assert) {
const htmlTrackElement = new HTMLTrackElement({
tech: defaultTech
tech: this.tech
});
assert.equal(typeof htmlTrackElement.default, 'undefined', 'we have a default');

View File

@ -2,6 +2,7 @@
import TextTrackList from '../../../src/js/tracks/text-track-list.js';
import TextTrack from '../../../src/js/tracks/text-track.js';
import EventTarget from '../../../src/js/event-target.js';
import TechFaker from '../tech/tech-faker';
QUnit.module('Text Track List');
QUnit.test('trigger "change" event when "modechange" is fired on a track', function(assert) {
@ -23,11 +24,7 @@ QUnit.test('trigger "change" event when "modechange" is fired on a track', funct
});
QUnit.test('trigger "change" event when mode changes on a TextTrack', function(assert) {
const tt = new TextTrack({
tech: {
on() {}
}
});
const tt = new TextTrack({tech: new TechFaker()});
const ttl = new TextTrackList([tt]);
let changes = 0;
const changeHandler = function() {

View File

@ -7,27 +7,16 @@ import TextTrack from '../../../src/js/tracks/text-track.js';
import TestHelpers from '../test-helpers.js';
import proxyquireify from 'proxyquireify';
import sinon from 'sinon';
import TextTrackList from '../../../src/js/tracks/text-track-list.js';
const proxyquire = proxyquireify(require);
const defaultTech = {
textTracks() {
return new TextTrackList();
},
on() {},
off() {},
currentTime() {}
};
QUnit.module('Text Track', {
beforeEach() {
this.oldVttjs = window.vttjs;
window.vttjs = {
VTTCue: Object
};
this.tech = new TechFaker();
},
afterEach() {
window.vttjs = this.oldVttjs;
this.tech.dispose();
this.tech = null;
}
});
@ -38,7 +27,7 @@ TrackBaseline(TextTrack, {
mode: 'disabled',
label: 'English',
language: 'en',
tech: defaultTech
tech: new TechFaker()
});
QUnit.test('requires a tech', function(assert) {
@ -52,7 +41,7 @@ QUnit.test('can create a TextTrack with a mode property', function(assert) {
const mode = 'disabled';
const tt = new TextTrack({
mode,
tech: defaultTech
tech: this.tech
});
assert.equal(tt.mode, mode, 'we have a mode');
@ -60,7 +49,7 @@ QUnit.test('can create a TextTrack with a mode property', function(assert) {
QUnit.test('defaults when items not provided', function(assert) {
const tt = new TextTrack({
tech: TechFaker
tech: this.tech
});
assert.equal(tt.kind, 'subtitles', 'kind defaulted to subtitles');
@ -71,7 +60,7 @@ QUnit.test('defaults when items not provided', function(assert) {
QUnit.test('kind can only be one of several options, defaults to subtitles', function(assert) {
let tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'foo'
});
@ -79,35 +68,35 @@ QUnit.test('kind can only be one of several options, defaults to subtitles', fun
assert.notEqual(tt.kind, 'foo', 'the kind is set to subtitles, not foo');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'subtitles'
});
assert.equal(tt.kind, 'subtitles', 'the kind is set to subtitles');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'captions'
});
assert.equal(tt.kind, 'captions', 'the kind is set to captions');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'descriptions'
});
assert.equal(tt.kind, 'descriptions', 'the kind is set to descriptions');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'chapters'
});
assert.equal(tt.kind, 'chapters', 'the kind is set to chapters');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
kind: 'metadata'
});
@ -116,7 +105,7 @@ QUnit.test('kind can only be one of several options, defaults to subtitles', fun
QUnit.test('mode can only be one of several options, defaults to disabled', function(assert) {
let tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
mode: 'foo'
});
@ -124,21 +113,21 @@ QUnit.test('mode can only be one of several options, defaults to disabled', func
assert.notEqual(tt.mode, 'foo', 'the mode is set to disabld, not foo');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
mode: 'disabled'
});
assert.equal(tt.mode, 'disabled', 'the mode is set to disabled');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
mode: 'hidden'
});
assert.equal(tt.mode, 'hidden', 'the mode is set to hidden');
tt = new TextTrack({
tech: defaultTech,
tech: this.tech,
mode: 'showing'
});
@ -149,7 +138,7 @@ QUnit.test('cue and activeCues are read only', function(assert) {
const mode = 'disabled';
const tt = new TextTrack({
mode,
tech: defaultTech
tech: this.tech
});
tt.cues = 'foo';
@ -161,7 +150,7 @@ QUnit.test('cue and activeCues are read only', function(assert) {
QUnit.test('mode can only be set to a few options', function(assert) {
const tt = new TextTrack({
tech: defaultTech
tech: this.tech
});
tt.mode = 'foo';
@ -186,7 +175,7 @@ QUnit.test('mode can only be set to a few options', function(assert) {
QUnit.test('cues and activeCues return a TextTrackCueList', function(assert) {
const tt = new TextTrack({
tech: defaultTech
tech: this.tech
});
assert.ok(tt.cues.getCueById, 'cues are a TextTrackCueList');
@ -195,7 +184,7 @@ QUnit.test('cues and activeCues return a TextTrackCueList', function(assert) {
QUnit.test('cues can be added and removed from a TextTrack', function(assert) {
const tt = new TextTrack({
tech: defaultTech
tech: this.tech
});
const cues = tt.cues;
@ -216,6 +205,49 @@ 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('does not fire cuechange before Tech is ready', function(assert) {
const done = assert.async();
const player = TestHelpers.makePlayer({techfaker: {autoReady: false}});
let changes = 0;
const tt = new TextTrack({
tech: player.tech_,
mode: 'showing'
});
const cuechangeHandler = function() {
changes++;
};
tt.addCue({
id: '1',
startTime: 0,
endTime: 5
});
tt.oncuechange = cuechangeHandler;
tt.addEventListener('cuechange', cuechangeHandler);
player.tech_.currentTime = function() {
return 0;
};
player.tech_.trigger('timeupdate');
assert.equal(changes, 0, 'a cuechange event is not triggered');
player.tech_.on('ready', function() {
player.tech_.currentTime = function() {
return 0.2;
};
player.tech_.trigger('timeupdate');
assert.equal(changes, 2, 'a cuechange event trigger addEventListener and oncuechange');
player.dispose();
done();
});
player.tech_.triggerReady();
});
QUnit.test('fires cuechange when cues become active and inactive', function(assert) {
const player = TestHelpers.makePlayer();
let changes = 0;
@ -283,7 +315,7 @@ QUnit.test('tracks are parsed if vttjs is loaded', function(assert) {
/* eslint-disable no-unused-vars */
const tt = new TextTrack_({
tech: defaultTech,
tech: this.tech,
src: 'http://example.com'
});
/* eslint-enable no-unused-vars */