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

fix: switching between 'subtitles' and 'captions' text tracks is broken #6008

Merged
merged 2 commits into from
May 24, 2019

Conversation

alex-barstow
Copy link
Contributor

@alex-barstow alex-barstow commented May 24, 2019

Description

This fixes a regression introduced in v7.4.3. The addition of this code made it so that, when turning on captions, other text tracks are only disabled if they are of the same kind as the one being selected. This resulted in broken behavior when switching between a subtitles track and a captions track or vice-versa.

Specific Changes proposed

"Subtitle" tracks should be disabled if another "caption" track is enabled, and vice-versa (in other words, they should be considered the same when enabling/disabling tracks). There is existing infrastructure that stores the appropriate text track kinds_ or kind_ for a given instance of TextTrackButton, each of which creates a list of TextTrackMenuItem instances. We can pass that information on to each instance of TextTrackMenuItem where it can be stored as a property and used to aid in text track enabling/disabling decisions.

Requirements Checklist

  • Feature implemented / Bug fixed
  • If necessary, more likely in a feature request than a bug fix
    • Change has been verified in an actual browser (Chome, Firefox, IE)
    • Unit Tests updated or fixed
    • Docs/guides updated
    • Example created (starter template on JSBin)
  • Reviewed by Two Core Contributors

@gkatsev gkatsev changed the base branch from master to 7.5.x May 24, 2019 20:05
@gkatsev gkatsev changed the base branch from 7.5.x to master May 24, 2019 20:06
@alex-barstow alex-barstow changed the base branch from master to 7.5.x May 24, 2019 20:13
Copy link
Member

@gkatsev gkatsev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified this as well.

@gkatsev gkatsev merged commit cd6be5b into videojs:7.5.x May 24, 2019
@alex-barstow alex-barstow deleted the fix-caption-switching branch May 28, 2019 14:38
gkatsev pushed a commit to gkatsev/video.js that referenced this pull request Jun 10, 2019
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