Skip to content

Commit

Permalink
fix: Only select TextTrackMenuItem if unselected (#4920)
Browse files Browse the repository at this point in the history
These changes address an issue where screen readers may repeatedly and redundantly read TextTrackMenuItem's control text on every texttrackchange event in some browsers.

The source of the problem is in the handleTracksChange() method of TextTrackMenuItem and its subclass OffTextTrackMenuItem, in which this.selected(true/false) gets called even if the selected state has not changed since its previous invocation.
  • Loading branch information
alex-barstow authored and gkatsev committed Feb 9, 2018
1 parent 4afabc2 commit 6189baa
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -55,18 +55,22 @@ class OffTextTrackMenuItem extends TextTrackMenuItem {
*/
handleTracksChange(event) {
const tracks = this.player().textTracks();
let selected = true;
let shouldBeSelected = true;

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

if ((this.options_.kinds.indexOf(track.kind) > -1) && track.mode === 'showing') {
selected = false;
shouldBeSelected = false;
break;
}
}

this.selected(selected);
// Prevent redundant selected() calls because they may cause
// screen readers to read the appended control text unnecessarily
if (shouldBeSelected !== this.isSelected_) {
this.selected(shouldBeSelected);
}
}

handleSelectedLanguageChange(event) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,13 @@ class TextTrackMenuItem extends MenuItem {
* @listens TextTrackList#change
*/
handleTracksChange(event) {
this.selected(this.track.mode === 'showing');
const shouldBeSelected = this.track.mode === 'showing';

// Prevent redundant selected() calls because they may cause
// screen readers to read the appended control text unnecessarily
if (shouldBeSelected !== this.isSelected_) {
this.selected(shouldBeSelected);
}
}

handleSelectedLanguageChange(event) {
Expand Down
5 changes: 4 additions & 1 deletion src/js/menu/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,9 @@ class MenuItem extends ClickableComponent {
super(player, options);

this.selectable = options.selectable;
this.isSelected_ = options.selected || false;

this.selected(options.selected);
this.selected(this.isSelected_);

if (this.selectable) {
// TODO: May need to be either menuitemcheckbox or menuitemradio,
Expand Down Expand Up @@ -93,11 +94,13 @@ class MenuItem extends ClickableComponent {
// aria-checked isn't fully supported by browsers/screen readers,
// so indicate selected state to screen reader in the control text.
this.controlText(', selected');
this.isSelected_ = true;
} else {
this.removeClass('vjs-selected');
this.el_.setAttribute('aria-checked', 'false');
// Indicate un-selected state to screen reader
this.controlText('');
this.isSelected_ = false;
}
}
}
Expand Down

0 comments on commit 6189baa

Please sign in to comment.