-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Fix missing native HTML5 tracks #3212
Conversation
type: 'addtrack' | ||
}); | ||
// Do not add duplicate tracks | ||
if (!this.tracks_.includes(track)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Array#includes
is ES6 only, we can't use it.
Also, could you rebase against stable? We could get this out as a patch update. |
I've removed includes(), but rebasing against stable will add other commits as before. Should I do that? |
Don't worry about it then, when merging I'll see if I can get it merged correctly into stable, otherwise, we'll just merge it into master and have it be part of the next minor release. |
Ah, figured out how to do it locally. You'd have wanted to do an interactive rebase and then delete each of the commits that aren't part of this PR $ git rebase -i stable and only leave the last two commits (fe91dc8, and 5fb69e3). But I'm getting this merged locally along with a small tweak to make sure that forloop doesn't run if |
This is how it looks like (#3224), I created the PR so that I can run browserstack on it before merging. |
Description
Fixes missing text tracks when using native tracks in Chrome etc, fixes #2689
Tracks are currently added when the native text tracks
addtrack
event is triggered. Depending on how the player is initialised, this does not occur or fires before the event listener has been added.Specific Changes proposed
addtrack
is triggered.Requirements Checklist