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

3.x dev support multiple tracks with same language code #1957

Merged
merged 3 commits into from
Dec 14, 2016
Merged

3.x dev support multiple tracks with same language code #1957

merged 3 commits into from
Dec 14, 2016

Conversation

laupow
Copy link
Contributor

@laupow laupow commented Dec 5, 2016

This pull request fixes a couple oddities/bugs surrounding the assumption that a video will only have one track per language. I didn't file Issue reports, but I can if that's also desirable. Having two or more tracks of the same language produces the following bugs:

  • addTrackButton() produces two labels with the same ID, which breaks the caption track selector UI partially
  • setTrack() doesn't allow a user to activate the Nth track of a particular language
  • enableTrackButton() incorrectly removes the "(loading)" with the content from a different track label. This markup
<track kind="captions" src="" srclang="en" label="English" />
<track kind="subtitles" src="" srclang="en" label="English (Alt)" />
<track kind="subtitles" src="" srclang="en" label="More Subtitles" />

produces this broken UI: screen shot 2016-12-05 at 11 37 39 am


Per the example below from in the HTML5 spec, I believe these changes in this PR result in a more-correct handling of track elements, as it appears to be acceptable to support multiple tracks of one language.

This video has subtitles in several languages:

<video src="brave.webm">
 <track kind=subtitles src=brave.en.vtt srclang=en label="English">
 <track kind=captions src=brave.en.hoh.vtt srclang=en label="English for the Hard of Hearing">
 <track kind=subtitles src=brave.fr.vtt srclang=fr lang=fr label="Français">
 <track kind=subtitles src=brave.de.vtt srclang=de lang=de label="Deutsch">
</video>

(The lang attributes on the last two describe the language of the label attribute, not the language of the subtitles themselves. The language of the subtitles is given by the srclang attribute.)

While I'm not positive these changes represent a perfect changeset for handling track elements, these changes definitely fix bugs in a common use case: multiple tracks of the same language.

@rafa8626
Copy link
Contributor

rafa8626 commented Dec 5, 2016

Thanks. I'll review this and if no comments to add I'll merge it by the end of this week. Thanks

@rafa8626 rafa8626 merged commit 292e059 into mediaelement:3.x-dev Dec 14, 2016
@rafa8626
Copy link
Contributor

@laupow PR merged; I just modified something for the "None" caption since it wasn't being selected properly. Other than that, great job.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants