Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emulate HTMLTrackElement to enable load event. #2804

Closed
wants to merge 20 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions src/js/control-bar/text-track-controls/chapters-button.js
Original file line number Diff line number Diff line change
Expand Up @@ -77,21 +77,13 @@ class ChaptersButton extends TextTrackButton {
let chaptersTrack;
let items = this.items = [];

for (let i = 0, l = tracks.length; i < l; i++) {
for (let i = 0, length = tracks.length; i < length; i++) {
let track = tracks[i];

if (track['kind'] === this.kind_) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed #1949

if (!track.cues) {
track['mode'] = 'hidden';
/* jshint loopfunc:true */
// TODO see if we can figure out a better way of doing this https://github.com/videojs/video.js/issues/1864
window.setTimeout(Fn.bind(this, function() {
this.createMenu();
}), 100);
/* jshint loopfunc:false */
} else {
chaptersTrack = track;
break;
}
chaptersTrack = track;

break;
}
}

Expand All @@ -105,7 +97,17 @@ class ChaptersButton extends TextTrackButton {
}));
}

if (chaptersTrack) {
if (chaptersTrack && chaptersTrack.cues == null) {
chaptersTrack['mode'] = 'hidden';

let remoteTextTrackEl = this.player_.remoteTextTrackEls().getTrackElementByTrack_(chaptersTrack);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like there an issue here tracks that were added in the html:

  <video id="vid1" class="video-js vjs-default-skin" controls preload="auto" width="640" height="264"
      poster="http://video-js.zencoder.com/oceans-clip.png">
    <source src="oceans-clip.mp4" type='video/mp4'>
    <source src="http://video-js.zencoder.com/oceans-clip.mp4?fo23" type='video/mp4'>
    <track kind="chapters" src="../docs/examples/shared/example-captions.vtt" srclang="en" label="Subs"></track>
</video>

What happens here is that we get the chapters track but when we try to retrieve its associated track el, we get undefined so the chapters button never loads.
I think what we need to do is in proxyNativeTextTracks_ to loop through any track elements and add them to the remoteTextTrackEls list. And then in addRemoteTextTrack we need to make sure the track el gets added to the list. We also want to make sure that remoteRemoteTextTrack removes the associated el.
html-track-element-list might need to get updated to support this but hopefully it'll just work.


if (remoteTextTrackEl) {
remoteTextTrackEl.addEventListener('load', (event) => this.update());
}
}

if (chaptersTrack && chaptersTrack.cues && chaptersTrack.cues.length > 0) {
let cues = chaptersTrack['cues'], cue;

for (let i = 0, l = cues.length; i < l; i++) {
Expand All @@ -120,6 +122,7 @@ class ChaptersButton extends TextTrackButton {

menu.addChild(mi);
}

this.addChild(menu);
}

Expand Down
10 changes: 10 additions & 0 deletions src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -2492,6 +2492,16 @@ class Player extends Component {
return this.tech_ && this.tech_['remoteTextTracks']();
}

/**
* Get an array of remote html track elements
*
* @return {HTMLTrackElement[]}
* @method remoteTextTrackEls
*/
remoteTextTrackEls() {
return this.tech_ && this.tech_['remoteTextTrackEls']();
}

/**
* Add a text track
* In addition to the W3C settings we allow adding additional info through options.
Expand Down
47 changes: 28 additions & 19 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 @@ -731,44 +734,46 @@ class Html5 extends Tech {
}

/**
* Creates and returns a remote text track object
* Creates a remote text track object and returns a html track element
*
* @param {Object} options The object should contain values for
* kind, language, label and src (location of the WebVTT file)
* @return {TextTrackObject}
* @return {HTMLTrackElement}
* @method addRemoteTextTrack
*/
addRemoteTextTrack(options={}) {
if (!this['featuresNativeTextTracks']) {
return super.addRemoteTextTrack(options);
}

var track = document.createElement('track');
let htmlTrackElement = document.createElement('track');

if (options['kind']) {
track['kind'] = options['kind'];
if (options.kind) {
htmlTrackElement.kind = options.kind;
}
if (options['label']) {
track['label'] = options['label'];
if (options.label) {
htmlTrackElement.label = options.label;
}
if (options['language'] || options['srclang']) {
track['srclang'] = options['language'] || options['srclang'];
if (options.language || options.srclang) {
htmlTrackElement.srclang = options.language || options.srclang;
}
if (options['default']) {
track['default'] = options['default'];
if (options.default) {
htmlTrackElement.default = options.default;
}
if (options['id']) {
track['id'] = options['id'];
if (options.id) {
htmlTrackElement.id = options.id;
}
if (options['src']) {
track['src'] = options['src'];
if (options.src) {
htmlTrackElement.src = options.src;
}

this.el().appendChild(track);
this.el().appendChild(htmlTrackElement);

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

return track;
return htmlTrackElement;
}

/**
Expand All @@ -782,8 +787,12 @@ class Html5 extends Tech {
return super.removeRemoteTextTrack(track);
}

var tracks, i;
let tracks, i;

let trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);

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

tracks = this.$$('track');
Expand Down
44 changes: 36 additions & 8 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
*/

import Component from '../component';
import HTMLTrackElement from '../tracks/html-track-element';
import HTMLTrackElementList from '../tracks/html-track-element-list';
import mergeOptions from '../utils/merge-options.js';
import TextTrack from '../tracks/text-track';
import TextTrackList from '../tracks/text-track-list';
import * as Fn from '../utils/fn.js';
Expand Down Expand Up @@ -377,6 +380,17 @@ class Tech extends Component {
return this.remoteTextTracks_;
}

/**
* Get remote htmltrackelements
*
* @returns {HTMLTrackElementList}
* @method remoteTextTrackEls
*/
remoteTextTrackEls() {
this.remoteTextTrackEls_ = this.remoteTextTrackEls_ || new HTMLTrackElementList();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll also probably want to go through the same shenanigans we've gone through with the text track list to have it work across tech changes. See #2391
This can, and probably should, be tackled only once we have everything else working and could probably event go out as a separate PR.

return this.remoteTextTrackEls_;
}

/**
* Creates and returns a remote text track object
*
Expand All @@ -396,19 +410,28 @@ class Tech extends Component {
}

/**
* Creates and returns a remote text track object
* Creates a remote text track object and returns a emulated html track element
*
* @param {Object} options The object should contain values for
* kind, language, label and src (location of the WebVTT file)
* @return {TextTrackObject}
* @return {HTMLTrackElement}
* @method addRemoteTextTrack
*/
addRemoteTextTrack(options) {
let track = createTrackHelper(this, options.kind, options.label, options.language, options);
this.remoteTextTracks().addTrack_(track);
return {
track: track
};
let track = mergeOptions(options, {
tech: this
});

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);

return htmlTrackElement;
}

/**
Expand All @@ -419,6 +442,11 @@ class Tech extends Component {
*/
removeRemoteTextTrack(track) {
this.textTracks().removeTrack_(track);

let trackElement = this.remoteTextTrackEls().getTrackElementByTrack_(track);

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

Expand Down Expand Up @@ -447,7 +475,7 @@ class Tech extends Component {
/*
* Return whether the argument is a Tech or not.
* Can be passed either a Class like `Html5` or a instance like `player.tech_`
*
*
* @param {Object} component An item to check
* @return {Boolean} Whether it is a tech or not
*/
Expand Down
68 changes: 68 additions & 0 deletions src/js/tracks/html-track-element-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* @file html-track-element.js
*/

import * as browser from '../utils/browser.js';
import document from 'global/document';

class HtmlTrackElementList {
constructor(trackElements = []) {
let list = this;

if (browser.IS_IE8) {
list = document.createElement('custom');

for (let prop in HtmlTrackElementList.prototype) {
if (prop !== 'constructor') {
list[prop] = HtmlTrackElementList.prototype[prop];
}
}
}

list.trackElements_ = [];

Object.defineProperty(list, 'length', {
get() {
return this.trackElements_.length;
}
});

for (let i = 0, length = trackElements.length; i < length; i++) {
list.addTrackElement_(trackElements[i]);
}

if (browser.IS_IE8) {
return list;
}
}

addTrackElement_(trackElement) {
this.trackElements_.push(trackElement);
}

getTrackElementByTrack_(track) {
let trackElement_;

for (let i = 0, length = this.trackElements_.length; i < length; i++) {
if (track === this.trackElements_[i].track) {
trackElement_ = this.trackElements_[i];

break;
}
}

return trackElement_;
}

removeTrackElement_(trackElement) {
for (let i = 0, length = this.trackElements_.length; i < length; i++) {
if (trackElement === this.trackElements_[i]) {
this.trackElements_.splice(i, 1);

break;
}
}
}
}

export default HtmlTrackElementList;
Loading