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

No video.removeTextTrack method #1921

Open
egreaves opened this issue Oct 18, 2016 · 20 comments
Open

No video.removeTextTrack method #1921

egreaves opened this issue Oct 18, 2016 · 20 comments
Labels
addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests normative change topic: media

Comments

@egreaves
Copy link

The addTextTrack method can be used to add text tracks to a video element, but there is no removeTextTrack method, though the TextTrackList interface has an onremovetrack handler.

There are two use cases for the addTextTrack method in lieu of appending a track element:

  1. Circumventing CORS issues with text tracks loaded from a different domain than the HTMLMediaElement.src.
  2. Dynamically adding/removing tracks for use with 608 Captions or ID3 Metadata.

The only way to remove textTracks created via addTextTrack is to destroy the video element, which has adverse side effects for playback of playlist content.

To date, most video players have opted to create their own text track management solutions. There are multiple implementations where a single track is used and VTTCues are cleared/replaced whenever the user switches captions/subtitles tracks. However, this approach falls apart on iOS devices where there's a need to rely on native controls in fullscreen.

Adding support for this method would get video players closer to being able to rely on browsers for text track management.

@domenic domenic added addition/proposal New features or enhancements needs implementer interest Moving the issue forward requires implementers to express interest labels Nov 24, 2016
@domenic
Copy link
Member

domenic commented Nov 24, 2016

@zcorpan, any thoughts on this, or ideas on which browser implementers to contact to gain interest?

@zcorpan
Copy link
Member

zcorpan commented Nov 24, 2016

The proposal sounds reasonable to me.

cc @eric-carlson @foolip @cpearce @travisleithead @fsoder

@eric-carlson
Copy link

The proposal sounds reasonable to me as well.

@foolip
Copy link
Member

foolip commented Nov 29, 2016

I believe this should be pretty easy to implement, so if it helps people trying to do custom captions, let's do it.

@egreaves, would you be willing to start by writing tests in web-platform-tests for how this should work? It would be in html/semantics/embedded-content/media-elements/interfaces/HTMLElement/HTMLMediaElement/removeTextTrack.html

The only interesting question I think is what happens when you try to remove a TextTrack object that comes from a track element or from in-band tracks. One could make it do something sensible, like removing the track element itself and forgetting the in-band track, or throw an exception. Preferences?

@egreaves
Copy link
Author

@foolip, sure - I can undertake that.

Re: your questions...

  • Removing a TextTrack object that comes from a track element removes the element (existing behavior).
  • I'm ok with throwing an exception when there's an attempt to remove in-band tracks added by the browser. Browsers that add in-band tracks remove them when the media changes, so there's never a need, really, to remove them programmatically. At JW Player, we use a custom property to identify tracks added programmatically so we can target them for "cleanup" before changing media.

@foolip
Copy link
Member

foolip commented Nov 29, 2016

Removing a TextTrack object that comes from a track element removes the element (existing behavior).

Do you mean the fact that removing the track element from the document also removes the TextTrack object from the media element's textTracks list? This would be the other way around, but I think it'd be OK to make the behavior symmetrical if that's what you mean.

@gkatsev
Copy link

gkatsev commented Nov 29, 2016

I think the symmetrical behavior for removing the track element vs track object on the textTracks list makes sense.
I think that just forgetting the in-band track 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.

@foolip foolip added normative change needs tests Moving the issue forward requires someone to write tests and removed needs implementer interest Moving the issue forward requires implementers to express interest labels Dec 13, 2016
@foolip
Copy link
Member

foolip commented Dec 13, 2016

Is anyone willing to drive this? I've changed the labels around a bit to reflect the fact that this is waiting for tests (and then spec PR) and not so much implementor interest anymore.

@gkatsev
Copy link

gkatsev commented Dec 13, 2016

Is submitted a PR to add tests to the web platform tests the next step to move this along?

@foolip
Copy link
Member

foolip commented Dec 13, 2016

Yes, even if we're not exactly sure of the right behavior and wouldn't merge it yet, I think that's a good way to discuss the precise behavior intended.

@foolip
Copy link
Member

foolip commented Dec 13, 2016

(One would be right to point out that writing tests with no implementation is error-prone, but the only alternative here is for an implementer to volunteer to implement and write tests at the same time.)

@gkatsev
Copy link

gkatsev commented Jul 20, 2017

It happened! web-platform-tests/wpt#6594

@domenic
Copy link
Member

domenic commented Jul 20, 2017

Awesome work! The next step is writing the spec. @foolip and @zcorpan are both away for a while (although @foolip should be back in a week or so). We could wait for them, or you could try to contribute the spec text yourself, in which case I'm happy to provide editorial review.

@gkatsev
Copy link

gkatsev commented Jul 21, 2017

I could try but not really sure where to start with making spec text changes.

@foolip
Copy link
Member

foolip commented Aug 1, 2017

I've fleshed out the general structure of this in #2881, but there's a TODO about in-band tracks that I haven't thought much about yet. Still, review on the general shape of that would be appreciated.

@domenic
Copy link
Member

domenic commented Aug 1, 2017

I did mostly-editorial review. @gkatsev thoughts on the in-band tracks case? Did you test that?

@foolip
Copy link
Member

foolip commented Sep 4, 2017

The spec and test change is sitting idle in review now. @gkatsev, are you interested in taking over to complete the work?

@gkatsev
Copy link

gkatsev commented Sep 5, 2017

Yes, I'd be very interested. Also, sorry I haven't replied got busy with other work.

@foolip
Copy link
Member

foolip commented Sep 6, 2017

@gkatsev, great! The main thing that needs doing now is to address the comments and TODOs in #2881, and then make sure that web-platform-tests/wpt#6594 matches what the spec ends up saying.

@foolip
Copy link
Member

foolip commented Aug 26, 2020

I have abandoned #2881. If someone would like to help fix this, preparing a new pull request for the spec and a pull request like web-platform-tests/wpt#6594 is what it would take.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
addition/proposal New features or enhancements needs tests Moving the issue forward requires someone to write tests normative change topic: media
Development

Successfully merging a pull request may close this issue.

7 participants