Null check native tracks #2466
Null check native tracks #2466
Conversation
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: cdb0740 (Please note that this is a fully automated comment.) |
|
@pam retry |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: cdb0740 (Please note that this is a fully automated comment.) |
|
@pam retry |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: cdb0740 (Please note that this is a fully automated comment.) |
1 similar comment
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: cdb0740 (Please note that this is a fully automated comment.) |
| @@ -308,8 +308,8 @@ test('when switching techs, we should not get a new text track', function() { | |||
| ok(htmltracks === flashtracks, 'the tracks are equal'); | |||
| }); | |||
|
|
|||
| if (Html5.supportsNativeTextTracks()) { | |||
| test('listen to remove and add track events in native text tracks', function(assert) { | |||
| if (Html5.supportsNativeTextTracks() && ('removetrack' in Html5.TEST_VID.textTracks)) { | |||
There was a problem hiding this comment.
since needs to change to onremovetrack.
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: e90738c (Please note that this is a fully automated comment.) |
| @@ -308,8 +308,8 @@ test('when switching techs, we should not get a new text track', function() { | |||
| ok(htmltracks === flashtracks, 'the tracks are equal'); | |||
| }); | |||
|
|
|||
| if (Html5.supportsNativeTextTracks()) { | |||
| test('listen to remove and add track events in native text tracks', function(assert) { | |||
| if (Html5.supportsNativeTextTracks() && ('onremovetrack' in Html5.TEST_VID.textTracks)) { | |||
There was a problem hiding this comment.
Should we just add this onremovetrack check to the supportsNativeTextTracks check?
There was a problem hiding this comment.
If the solution to #2465 is to disable it for now, then, yes.
|
lgtm! |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 887e45c (Please note that this is a fully automated comment.) |
|
Tests failed. Automated cross-browser testing via Sauce Labs and Travis CI shows that the JavaScript changes in this pull request are: BUSTED Commit: 98dfa9c (Please note that this is a fully automated comment.) |
| @@ -889,6 +893,9 @@ Html5.supportsNativeTextTracks = function() { | |||
| if (supportsTextTracks && browser.IS_FIREFOX) { | |||
| supportsTextTracks = false; | |||
| } | |||
| if (supportsTextTracks && !('onremovetrack' in Html5.TEST_VID.textTracks)) { | |||
There was a problem hiding this comment.
Looks like supportsNativeTextTracks could use some simplification now, but not worth blocking this.
|
Judging by pam it does look like this fixes the track specific issues, though we're still getting disconnect issues. Do we have any idea what those are related to? |
This fixes #2454 and hopefully any of the text-track related CI test failures in master.
One thing I've noticed when working on this is that IE11 doesn't have
removetrackhandler (and seems likeaddtrackhandler doesn't work if added viaaddEventListener). I've disabled the native text track tests that depend onremovetrackif not available but this does mean that our custom controls won't update on IE11 and we need a solution for it. I've filed #2465 to figure that out since this PR should be just for fixing the tests.