When removing a track also remove listeners #3046
Conversation
| @@ -111,6 +111,7 @@ TextTrackList.prototype.removeTrack_ = function(rtrack) { | |||
| for (let i = 0, l = this.length; i < l; i++) { | |||
| if (this[i] === rtrack) { | |||
| track = this[i]; | |||
| track.off(); | |||
There was a problem hiding this comment.
See the note in the doc for this method :)
The track in this list could be a native track element which won't have an off method. Can probably check track instanceof TextTrack or something.
However, because users are required to manage their listeners with native tracks, I wonder whether we should do this for the emulated tracks?
There was a problem hiding this comment.
It looks like one of the tests caught that scenario. I'm checking for track.off existence since it seems like the input is still a TextTrack even though it's an element.
forbesjo
force-pushed
the
remove-track-listeners
branch
from
1d147f5
to
cffa58c
Compare
| @@ -111,6 +111,9 @@ TextTrackList.prototype.removeTrack_ = function(rtrack) { | |||
| for (let i = 0, l = this.length; i < l; i++) { | |||
| if (this[i] === rtrack) { | |||
| track = this[i]; | |||
| if (track.off) { | |||
There was a problem hiding this comment.
This works. Sorry, what I meant was checking to see if it's an instanceof videojs.TextTrack.
|
@gkatsev any other changes needed? |
|
LGTM. I'll work on getting these in this week. |
gkatsev
added
the
minor
This PR is to remove event listeners from removed text tracks. This would be useful for a plugin like videojs-playlist.