Store tracks on player #1874
Store tracks on player #1874
Conversation
This does break without the previous commit.
| @@ -345,13 +345,13 @@ vjs.MediaTechController.prototype.emulateTextTracks = function() { | |||
| vjs.MediaTechController.prototype.textTracks_; | |||
|
|
|||
| vjs.MediaTechController.prototype.textTracks = function() { | |||
| this.textTracks_ = this.textTracks_ || new vjs.TextTrackList(); | |||
| return this.textTracks_; | |||
| this.player_.textTracks_ = this.player_.textTracks_ || new vjs.TextTrackList(); | |||
There was a problem hiding this comment.
Should this logic be bumped into player.js?
|
The test turns off the native text tracks, which are the default for HTML5 video. What happens if you don't do that and switch to/from Flash? |
|
Yeah, I discussed this with @dmlap last night. Perhaps in addition to this, we also want to have things like the caption buttons listen to loadstart and if the tracks changed, re-attach their listeners? |
|
However, regardless of the extra corner cases, I think this particular PR is good to go, as it does fix an issue. There's just possibly more things needed to fix. |
|
I'll open up a separate issue to discusse the native<->non-native tracks. |
|
lgtm @videojs/core-committers now that we're locked down for 5.0 make sure any fixes meant for patches are opened against the stable branch, because the master branch is about to get very different. The (@gkatsev I'll handle moving the open PRs) |
|
Thanks. I'll try and remember that for future patches :) |
Previously, we were storing the text tracks on the techs. However, if you switched techs, and you were emulating captions, you would end up getting new text tracks. This meant that UI elements which registered against the initial tech's text tracks would no longer get any events.