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

Persist caption/description choice over source changes #4295

Merged
merged 10 commits into from
May 25, 2017
20 changes: 20 additions & 0 deletions src/js/control-bar/text-track-controls/off-text-track-menu-item.js
Expand Up @@ -70,6 +70,26 @@ class OffTextTrackMenuItem extends TextTrackMenuItem {
this.selected(selected);
}

handleSelectedLanguageChange(event) {
const tracks = this.player().textTracks();
let allHidden = true;

for (let i = 0, l = tracks.length; i < l; i++) {
const track = tracks[i];

if ((['captions', 'descriptions', 'subtitles'].indexOf(track.kind) > -1) && track.mode === 'showing') {
allHidden = false;
break;
}
}

if (allHidden) {
this.player_.cache_.selectedLanguage = {
enabled: false
};
}
}

}

Component.registerComponent('OffTextTrackMenuItem', OffTextTrackMenuItem);
Expand Down
Expand Up @@ -66,6 +66,7 @@ class TextTrackButton extends TrackButton {

// only add tracks that are of an appropriate kind and have a label
if (this.kinds_.indexOf(track.kind) > -1) {

const item = new TrackMenuItem(this.player_, {
track,
// MenuItem is selectable
Expand Down
24 changes: 23 additions & 1 deletion src/js/control-bar/text-track-controls/text-track-menu-item.js
Expand Up @@ -29,17 +29,20 @@ class TextTrackMenuItem extends MenuItem {

// Modify options for parent MenuItem class's init.
options.label = track.label || track.language || 'Unknown';
options.selected = track.default || track.mode === 'showing';
options.selected = track.mode === 'showing';

super(player, options);

this.track = track;
const changeHandler = Fn.bind(this, this.handleTracksChange);
const selectedLanguageChangeHandler = Fn.bind(this, this.handleSelectedLanguageChange);

player.on(['loadstart', 'texttrackchange'], changeHandler);
tracks.addEventListener('change', changeHandler);
tracks.addEventListener('selectedlanguagechange', selectedLanguageChangeHandler);
this.on('dispose', function() {
tracks.removeEventListener('change', changeHandler);
tracks.removeEventListener('selectedlanguagechange', selectedLanguageChangeHandler);
});

// iOS7 doesn't dispatch change events to TextTrackLists when an
Expand Down Expand Up @@ -120,6 +123,25 @@ class TextTrackMenuItem extends MenuItem {
this.selected(this.track.mode === 'showing');
}

handleSelectedLanguageChange(event) {
if (this.track.mode === 'showing') {
const selectedLanguage = this.player_.cache_.selectedLanguage;

// Don't replace the kind of track across the same language
if (selectedLanguage && selectedLanguage.enabled &&
selectedLanguage.language === this.track.language &&
selectedLanguage.kind !== this.track.kind) {
return;
}

this.player_.cache_.selectedLanguage = {
enabled: true,
language: this.track.language,
kind: this.track.kind
};
}
}

}

Component.registerComponent('TextTrackMenuItem', TextTrackMenuItem);
Expand Down
78 changes: 56 additions & 22 deletions src/js/tracks/text-track-display.js
Expand Up @@ -92,6 +92,7 @@ class TextTrackDisplay extends Component {

player.on('loadstart', Fn.bind(this, this.toggleDisplay));
player.on('texttrackchange', Fn.bind(this, this.updateDisplay));
player.on('loadstart', Fn.bind(this, this.preselectTrack));
Copy link
Member

Choose a reason for hiding this comment

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

did we decide on whether we wanted this to happen on addtrack as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe that we discussed it and decided that it would be disorienting to preselect a new track if a track had already been selected by the user.


// This used to be called during player init, but was causing an error
// if a track should show by default and the display hadn't loaded yet.
Expand All @@ -111,33 +112,66 @@ class TextTrackDisplay extends Component {
this.player_.addRemoteTextTrack(tracks[i], true);
}

const modes = {captions: 1, subtitles: 1};
const trackList = this.player_.textTracks();
let firstDesc;
let firstCaptions;
this.preselectTrack();
}));
}

for (let i = 0; i < trackList.length; i++) {
const track = trackList[i];
/**
* Preselect a track following this precedence:
* - matches the previously selected {@link TextTrack}'s language and kind
* - matches the previously selected {@link TextTrack}'s language only
* - is the first default captions track
* - is the first default descriptions track
*
* @listens Player#loadstart
*/
preselectTrack() {
const modes = {captions: 1, subtitles: 1};
const trackList = this.player_.textTracks();
const userPref = this.player_.cache_.selectedLanguage;
let firstDesc;
let firstCaptions;
let preferredTrack;

for (let i = 0; i < trackList.length; i++) {
const track = trackList[i];

if (userPref && userPref.enabled &&
userPref.language === track.language) {
// Always choose the track that matches both language and kind
if (track.kind === userPref.kind) {
preferredTrack = track;
// or choose the first track that matches language
} else if (!preferredTrack) {
preferredTrack = track;
}

if (track.default) {
if (track.kind === 'descriptions' && !firstDesc) {
firstDesc = track;
} else if (track.kind in modes && !firstCaptions) {
firstCaptions = track;
}
// clear everything if offTextTrackMenuItem was clicked
} else if (userPref && !userPref.enabled) {
preferredTrack = null;
firstDesc = null;
firstCaptions = null;

} else if (track.default) {
if (track.kind === 'descriptions' && !firstDesc) {
firstDesc = track;
} else if (track.kind in modes && !firstCaptions) {
firstCaptions = track;
}
}
}

// We want to show the first default track but captions and subtitles
// take precedence over descriptions.
// So, display the first default captions or subtitles track
// and otherwise the first default descriptions track.
if (firstCaptions) {
firstCaptions.mode = 'showing';
} else if (firstDesc) {
firstDesc.mode = 'showing';
}
}));
// The preferredTrack matches the user preference and takes
// precendence over all the other tracks.
// So, display the preferredTrack before the first default track
// and the subtitles/captions track before the descriptions track
if (preferredTrack) {
preferredTrack.mode = 'showing';
} else if (firstCaptions) {
firstCaptions.mode = 'showing';
} else if (firstDesc) {
firstDesc.mode = 'showing';
}
}

/**
Expand Down
8 changes: 8 additions & 0 deletions src/js/tracks/text-track-list.js
Expand Up @@ -61,6 +61,14 @@ class TextTrackList extends TrackList {
track.addEventListener('modechange', Fn.bind(this, function() {
this.trigger('change');
}));

const nonLanguageTextTrackKind = ['metadata', 'chapters'];

if (nonLanguageTextTrackKind.indexOf(track.kind) === -1) {
track.addEventListener('modechange', Fn.bind(this, function() {
this.trigger('selectedlanguagechange');
Copy link
Member

Choose a reason for hiding this comment

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

just realized it's possible that the language doesn't change, so, the naming is a big weird, but maybe still ok.
For example, if you go from english captions to english subtitles, the language is the same but only the kind changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe displayedtexttrackchange would be a better name?

}));
}
}
}
export default TextTrackList;
1 change: 1 addition & 0 deletions src/js/tracks/text-track.js
Expand Up @@ -253,6 +253,7 @@ class TextTrack extends Track {
* @type {EventTarget~Event}
*/
this.trigger('modechange');

}
});

Expand Down