Add failing tests for video.removeTextTrack #6594
Conversation
gkatsev
changed the title
Firefox (nightly)Testing web-platform-tests at revision 733d055 All results1 test ran/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLMediaElement/removeTextTrack.html
|
Sauce (safari)Testing web-platform-tests at revision 733d055 All results1 test ran/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLMediaElement/removeTextTrack.html
|
Chrome (unstable)Testing web-platform-tests at revision 4c0bd4b All results1 test ran/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLMediaElement/removeTextTrack.html
|
Sauce (MicrosoftEdge)Testing web-platform-tests at revision 4c0bd4b All results1 test ran/html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLMediaElement/removeTextTrack.html
|
|
w3c-test:mirror |
| assert_false(video.contains(tel)); | ||
| assert_equals(video.textTracks.length, 0); | ||
|
|
||
| }, document.title + ' also remove associated track element'); |
There was a problem hiding this comment.
Could you also test that the removetrack event is fired?
There was a problem hiding this comment.
Yes. We forgot about that when writing these :)
| assert_equals(video.textTracks.length, 0); | ||
| assert_throws('NOT_FOUND_ERROR', function() { | ||
| video.removeTextTrack(t); | ||
| }, 'standalone'); |
There was a problem hiding this comment.
The assertion description ('standalone') isn't needed here, crafting useful descriptions is hard so if there's just one assert_throws in a test I don't bother, one can tell from the error message what has failed anyway.
| }, 'standalone'); | ||
| assert_equals(video.textTracks, video.textTracks); | ||
| assert_equals(video.textTracks.length, 0); | ||
| }, document.title + ' orphaned track'); |
There was a problem hiding this comment.
In addition to this, can you try removing a track which isn't from a dangling track element, but attached to another media element? If the order of steps in the spec were different, that could accidentally work, which would be curious.
| video.removeTextTrack(t); | ||
| assert_equals(video.textTracks, video.textTracks); | ||
| assert_equals(video.textTracks.length, 0); | ||
| }, document.title + ' captions'); |
There was a problem hiding this comment.
I think it's probably fine to only test one of these cases, but if you do want to keep them all, you can do something like ['captions', 'subtitles', ...].forEach(function(kind) { test(...) }) to avoid repeating the body of the test so many times.
|
Safari does support in-band metadata tracks. I could look into writing a test for it. |
These are some failing tests for
video.removeTextTrackbased on feedback from whatwg/html#1921.Worked with @egreaves during textAV.tech to finally get it out the door.
As I mentioned in whatwg/html#1921, I think that just removing the in-band track from the TextTrackList rather than throwing would be a better approach as it matches more closely what happens with the track elements. It'll also be a signal to the browser that we don't care about parsing the rest of this track until the source gets reset. However, I'm not sure that's the best approach nor do I know how I would go about writing tests for or against such a track.
In addition, once this method is available, authors that create text tracks and are actively adding cues to tracks would need to do null-checking or what not for removed text track.
Ultimately, I think
removeTextTrackandaddTextTrackshould mirror removing and adding track elements to the video element as closely as possible.