Skip to content

Commit

Permalink
feat(texttracks): always use emulated text tracks (#3798)
Browse files Browse the repository at this point in the history
Chrome has a bug where if you add a remote text track and try to use it programmatically, you won't get any cues displayed. So, just switch to always emulated mode.

Also, add IS_SAFARI and IS_ANY_SAFARI to the browsers.
  • Loading branch information
gkatsev authored Dec 2, 2016
1 parent bed19be commit 881cfcb
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 40 deletions.
20 changes: 1 addition & 19 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -878,25 +878,7 @@ Html5.canControlPlaybackRate = function() {
* - False otherwise
*/
Html5.supportsNativeTextTracks = function() {
let supportsTextTracks;

// Figure out native text track support
// If mode is a number, we cannot change it because it'll disappear from view.
// Browsers with numeric modes include IE10 and older (<=2013) samsung android models.
// Firefox isn't playing nice either with modifying the mode
// TODO: Investigate firefox: https://github.com/videojs/video.js/issues/1862
supportsTextTracks = !!Html5.TEST_VID.textTracks;
if (supportsTextTracks && Html5.TEST_VID.textTracks.length > 0) {
supportsTextTracks = typeof Html5.TEST_VID.textTracks[0].mode !== 'number';
}
if (supportsTextTracks && browser.IS_FIREFOX) {
supportsTextTracks = false;
}
if (supportsTextTracks && !('onremovetrack' in Html5.TEST_VID.textTracks)) {
supportsTextTracks = false;
}

return supportsTextTracks;
return browser.IS_ANY_SAFARI;
};

/**
Expand Down
3 changes: 3 additions & 0 deletions src/js/utils/browser.js
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,8 @@ export const IE_VERSION = (function(result) {
return result && parseFloat(result[1]);
}((/MSIE\s(\d+)\.\d/).exec(USER_AGENT)));

export const IS_SAFARI = (/Safari/i).test(USER_AGENT) && !IS_CHROME && !IS_ANDROID && !IS_EDGE;
export const IS_ANY_SAFARI = IS_SAFARI || IS_IOS;

export const TOUCH_ENABLED = !!(('ontouchstart' in window) || window.DocumentTouch && document instanceof window.DocumentTouch);
export const BACKGROUND_SIZE_SUPPORTED = 'backgroundSize' in document.createElement('video').style;
29 changes: 8 additions & 21 deletions test/unit/tracks/text-tracks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -250,37 +250,24 @@ QUnit.test('if native text tracks are not supported, create a texttrackdisplay',
player.dispose();
});

QUnit.test('html5 tech supports native text tracks if the video supports it, unless mode is a number', function(assert) {
QUnit.test('emulated tracks are always used, except in safari', function(assert) {
const oldTestVid = Html5.TEST_VID;
const oldIsAnySafari = browser.IS_ANY_SAFARI;

Html5.TEST_VID = {
textTracks: [{
mode: 0
}]
textTracks: []
};

assert.ok(!Html5.supportsNativeTextTracks(),
'native text tracks are not supported if mode is a number');

Html5.TEST_VID = oldTestVid;
});

QUnit.test('html5 tech supports native text tracks if the video supports it, unless it is firefox', function(assert) {
const oldTestVid = Html5.TEST_VID;
const oldIsFirefox = browser.IS_FIREFOX;
browser.IS_ANY_SAFARI = false;

Html5.TEST_VID = {
textTracks: []
};
assert.ok(!Html5.supportsNativeTextTracks(), 'Html5 does not support native text tracks, in non-safari');

browser.IS_FIREFOX = true;
browser.IS_ANY_SAFARI = true;

assert.ok(!Html5.supportsNativeTextTracks(),
'if textTracks are available on video element,' +
' native text tracks are supported');
assert.ok(Html5.supportsNativeTextTracks(), 'Html5 does support native text tracks in safari');

Html5.TEST_VID = oldTestVid;
browser.IS_FIREFOX = oldIsFirefox;
browser.IS_ANY_SAFARI = oldIsAnySafari;
});

QUnit.test('when switching techs, we should not get a new text track', function(assert) {
Expand Down

0 comments on commit 881cfcb

Please sign in to comment.