Skip to content

Commit

Permalink
Fix native text track capable browsers to initialize remote text trac…
Browse files Browse the repository at this point in the history
…ks correctly from DOM.

- Added documentation
- Added player test
- Added api test

NOTE: important that `addTrack_` order matters in tech.js::addRemoteTextTrack.
  • Loading branch information
Carey Hinoki committed Dec 7, 2015
1 parent 4ad1d2f commit eec4a2c
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 5 deletions.
8 changes: 6 additions & 2 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class Html5 extends Tech {
while (nodesLength--) {
let node = nodes[nodesLength];
let nodeName = node.nodeName.toLowerCase();

if (nodeName === 'track') {
if (!this.featuresNativeTextTracks) {
// Empty video tag tracks so the built-in player doesn't use them also.
Expand All @@ -57,6 +58,8 @@ class Html5 extends Tech {
// captions and subtitles. videoElement.textTracks
removeNodes.push(node);
} else {
// store HTMLTrackElement and TextTrack to remote list
this.remoteTextTrackEls().addTrackElement_(node);
this.remoteTextTracks().addTrack_(node.track);
}
}
Expand Down Expand Up @@ -766,6 +769,7 @@ class Html5 extends Tech {

this.el().appendChild(htmlTrackElement);

// store HTMLTrackElement and TextTrack to remote list
this.remoteTextTrackEls().addTrackElement_(htmlTrackElement);
this.remoteTextTracks().addTrack_(htmlTrackElement.track);

Expand All @@ -785,10 +789,10 @@ class Html5 extends Tech {

let tracks, i;

// TODO: track can be html text element due to addRemoteTextTrack—consider refactoring
let trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);
this.remoteTextTrackEls().removeTrackElement_(trackElement);

// remove HTMLTrackElement and TextTrack from remote list
this.remoteTextTrackEls().removeTrackElement_(trackElement);
this.remoteTextTracks().removeTrack_(track);

tracks = this.$$('track');
Expand Down
8 changes: 5 additions & 3 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -424,10 +424,12 @@ class Tech extends Component {

let htmlTrackElement = new HTMLTrackElement(track);

// store HTMLTrackElement and TextTrack to remote list
this.remoteTextTrackEls().addTrackElement_(htmlTrackElement);
this.remoteTextTracks().addTrack_(htmlTrackElement.track);

// must come after remoteTextTracks()
this.textTracks().addTrack_(htmlTrackElement.track);
this.remoteTextTracks().addTrack_(htmlTrackElement.track);

return htmlTrackElement;
}
Expand All @@ -441,10 +443,10 @@ class Tech extends Component {
removeRemoteTextTrack(track) {
this.textTracks().removeTrack_(track);

// TODO: track can be html text element due to addRemoteTextTrack—consider refactoring
let trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);
this.remoteTextTrackEls().removeTrackElement_(trackElement);

// remove HTMLTrackElement and TextTrack from remote list
this.remoteTextTrackEls().removeTrackElement_(trackElement);
this.remoteTextTracks().removeTrack_(track);
}

Expand Down
1 change: 1 addition & 0 deletions test/api/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ test('should be able to access expected player API methods', function() {

// TextTrack methods
ok(player.textTracks, 'textTracks exists');
ok(player.remoteTextTrackEls, 'remoteTextTrackEls exists');
ok(player.remoteTextTracks, 'remoteTextTracks exists');
ok(player.addTextTrack, 'addTextTrack exists');
ok(player.addRemoteTextTrack, 'addRemoteTextTrack exists');
Expand Down
33 changes: 33 additions & 0 deletions test/unit/player.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -835,6 +835,39 @@ test('createModal()', function() {
strictEqual(player.children().indexOf(modal), -1, 'modal was removed from player\'s children');
});

test('should return correct remote text track values', function () {
var fixture = document.getElementById('qunit-fixture');

var html = '<video id="example_1" class="video-js" autoplay preload="none">';
html += '<source src="http://google.com" type="video/mp4">';
html += '<track kind="captions" label="label">';
html += '</video>';

fixture.innerHTML += html;

var tag = document.getElementById('example_1');

var player = TestHelpers.makePlayer({
techOrder: ['html5']
}, tag);

equal(player.remoteTextTracks().length, 1, 'add text track via html');
equal(player.remoteTextTrackEls().length, 1, 'add html track element via html');

let htmlTextTrack = player.addRemoteTextTrack({
kind: 'captions',
label: 'label'
});

equal(player.remoteTextTracks().length, 2, 'add text track via method');
equal(player.remoteTextTrackEls().length, 2, 'add html track element via method');

player.removeRemoteTextTrack(htmlTextTrack.track);

equal(player.remoteTextTracks().length, 1, 'remove text track via method');
equal(player.remoteTextTrackEls().length, 1, 'remove html track element via method');
});

test('createModal() options object', function() {
var player = TestHelpers.makePlayer();
var modal = player.createModal('foo', {content: 'bar', label: 'boo'});
Expand Down

0 comments on commit eec4a2c

Please sign in to comment.