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

Null check native tracks #2466

Closed
wants to merge 6 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
19 changes: 13 additions & 6 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,11 @@ class Html5 extends Tech {
let emulatedTt = this.textTracks();

// remove native event listeners
tt.removeEventListener('change', this.handleTextTrackChange_);
tt.removeEventListener('addtrack', this.handleTextTrackAdd_);
tt.removeEventListener('removetrack', this.handleTextTrackRemove_);
if (tt) {
tt.removeEventListener('change', this.handleTextTrackChange_);
tt.removeEventListener('addtrack', this.handleTextTrackAdd_);
tt.removeEventListener('removetrack', this.handleTextTrackRemove_);
}

// clearout the emulated text track list.
let i = emulatedTt.length;
Expand Down Expand Up @@ -206,9 +208,11 @@ class Html5 extends Tech {
proxyNativeTextTracks_() {
let tt = this.el().textTracks;

tt.addEventListener('change', this.handleTextTrackChange_);
tt.addEventListener('addtrack', this.handleTextTrackAdd_);
tt.addEventListener('removetrack', this.handleTextTrackRemove_);
if (tt) {
tt.addEventListener('change', this.handleTextTrackChange_);
tt.addEventListener('addtrack', this.handleTextTrackAdd_);
tt.addEventListener('removetrack', this.handleTextTrackRemove_);
}
}

handleTextTrackChange(e) {
Expand Down Expand Up @@ -889,6 +893,9 @@ Html5.supportsNativeTextTracks = function() {
if (supportsTextTracks && browser.IS_FIREFOX) {
supportsTextTracks = false;
}
if (supportsTextTracks && !('onremovetrack' in Html5.TEST_VID.textTracks)) {
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 supportsNativeTextTracks could use some simplification now, but not worth blocking this.

supportsTextTracks = false;
}

return supportsTextTracks;
};
Expand Down
19 changes: 7 additions & 12 deletions test/unit/tracks/text-track-list-converter.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ q.module('Text Track List Converter');
let clean = (item) => {
delete item.id;
delete item.inBandMetadataTrackDispatchType;
delete item.cues;
};

let cleanup = (item) => {
Expand All @@ -34,8 +35,7 @@ if (Html5.supportsNativeTextTracks()) {
kind: 'captions',
label: 'English',
language: 'en',
mode: 'disabled',
cues: null
mode: 'disabled'
}, 'the json output is same');
});

Expand Down Expand Up @@ -76,15 +76,13 @@ if (Html5.supportsNativeTextTracks()) {
kind: 'captions',
label: 'Spanish',
language: 'es',
mode: 'disabled',
cues: null
mode: 'disabled'
}, {
src: 'http://example.com/english.vtt',
kind: 'captions',
label: 'English',
language: 'en',
mode: 'disabled',
cues: null
mode: 'disabled'
}], 'the output is correct');
});

Expand Down Expand Up @@ -147,8 +145,7 @@ q.test('trackToJson_ produces correct representation for emulated track object',
kind: 'captions',
label: 'English',
language: 'en',
mode: 'disabled',
cues: null
mode: 'disabled'
}, 'the json output is same');
});

Expand Down Expand Up @@ -191,15 +188,13 @@ q.test('textTracksToJson produces good json output for emulated only', function(
kind: 'captions',
label: 'Spanish',
language: 'es',
mode: 'disabled',
cues: null
mode: 'disabled'
}, {
src: 'http://example.com/english.vtt',
kind: 'captions',
label: 'English',
language: 'en',
mode: 'disabled',
cues: null
mode: 'disabled'
}], 'the output is correct');
});

Expand Down
3 changes: 1 addition & 2 deletions test/unit/tracks/tracks.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ test('when switching techs, we should not get a new text track', function() {
});

if (Html5.supportsNativeTextTracks()) {
test('listen to remove and add track events in native text tracks', function(assert) {
test('listen to native remove and add track events in native text tracks', function(assert) {
let done = assert.async();

let el = document.createElement('video');
Expand Down Expand Up @@ -357,6 +357,5 @@ if (Html5.supportsNativeTextTracks()) {
done();
};
emulatedTt.on('addtrack', addtrack);

});
}