Skip to content

Commit 15df4e1

Browse files
authored
fix(text-tracks): cuechange handler not triggering correctly (#5446)
We were only triggering cuechange events if a metadata track started out as not disabled or only when setting the mode to 'showing' Fixes #5308
1 parent 74bbc5d commit 15df4e1

File tree

2 files changed

+83
-2
lines changed

2 files changed

+83
-2
lines changed

src/js/tracks/text-track.js

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -226,11 +226,12 @@ class TextTrack extends Track {
226226
return;
227227
}
228228
mode = newMode;
229-
if (mode === 'showing') {
230-
229+
if (mode !== 'disabled') {
231230
this.tech_.ready(() => {
232231
this.tech_.on('timeupdate', timeupdateHandler);
233232
}, true);
233+
} else {
234+
this.tech_.off('timeupdate', timeupdateHandler);
234235
}
235236
/**
236237
* An event that fires when mode changes on this track. This allows

test/unit/tracks/text-track.test.js

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,86 @@ QUnit.test('fires cuechange when cues become active and inactive', function(asse
325325
player.dispose();
326326
});
327327

328+
QUnit.test('enabled and disabled cuechange handler when changing mode to hidden', function(assert) {
329+
const player = TestHelpers.makePlayer();
330+
let changes = 0;
331+
const tt = new TextTrack({
332+
tech: player.tech_
333+
});
334+
const cuechangeHandler = function() {
335+
changes++;
336+
};
337+
338+
tt.mode = 'hidden';
339+
340+
tt.addCue({
341+
id: '1',
342+
startTime: 1,
343+
endTime: 5
344+
});
345+
346+
tt.addEventListener('cuechange', cuechangeHandler);
347+
348+
player.tech_.currentTime = function() {
349+
return 2;
350+
};
351+
player.tech_.trigger('timeupdate');
352+
353+
assert.equal(changes, 1, 'a cuechange event trigger');
354+
355+
changes = 0;
356+
tt.mode = 'disabled';
357+
358+
player.tech_.currentTime = function() {
359+
return 7;
360+
};
361+
player.tech_.trigger('timeupdate');
362+
363+
assert.equal(changes, 0, 'NO cuechange event trigger');
364+
365+
player.dispose();
366+
});
367+
368+
QUnit.test('enabled and disabled cuechange handler when changing mode to showing', function(assert) {
369+
const player = TestHelpers.makePlayer();
370+
let changes = 0;
371+
const tt = new TextTrack({
372+
tech: player.tech_
373+
});
374+
const cuechangeHandler = function() {
375+
changes++;
376+
};
377+
378+
tt.mode = 'showing';
379+
380+
tt.addCue({
381+
id: '1',
382+
startTime: 1,
383+
endTime: 5
384+
});
385+
386+
tt.addEventListener('cuechange', cuechangeHandler);
387+
388+
player.tech_.currentTime = function() {
389+
return 2;
390+
};
391+
player.tech_.trigger('timeupdate');
392+
393+
assert.equal(changes, 1, 'a cuechange event trigger');
394+
395+
changes = 0;
396+
tt.mode = 'disabled';
397+
398+
player.tech_.currentTime = function() {
399+
return 7;
400+
};
401+
player.tech_.trigger('timeupdate');
402+
403+
assert.equal(changes, 0, 'NO cuechange event trigger');
404+
405+
player.dispose();
406+
});
407+
328408
QUnit.test('tracks are parsed if vttjs is loaded', function(assert) {
329409
const clock = sinon.useFakeTimers();
330410
const oldVTT = window.WebVTT;

0 commit comments

Comments
 (0)