Skip to content

Commit

Permalink
fix: use guid to ensure uniqueness of track setting options (#8762)
Browse files Browse the repository at this point in the history
The fix for the previous issue with the new text track settings
introduced that ids would be re-used across players as there was no
prefix for the settings that don't have a `<label>` for their
`<select>`.

This change uses a generated guid for items without that label.
Fixes #8761

## Requirements Checklist
- [x] Feature implemented / Bug fixed
- [ ] If necessary, more likely in a feature request than a bug fix
- [x] Change has been verified in an actual browser (Chrome, Firefox,
IE)
  - [ ] Unit Tests updated or fixed
  - [ ] Docs/guides updated
- [ ] Example created ([starter template on
JSBin](https://codepen.io/gkatsev/pen/GwZegv?editors=1000#0))
- [x] Has no DOM changes which impact accessiblilty or trigger warnings
(e.g. Chrome issues tab)
  - [x] Has no changes to JSDoc which cause `npm run docs:api` to error
- [ ] Reviewed by Two Core Contributors
  • Loading branch information
mister-ben authored Jun 10, 2024
1 parent fc1f7a6 commit f4186a0
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions src/js/tracks/text-track-select.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import Component from '../component';
import * as Dom from '../utils/dom';
import { newGUID } from '../utils/guid';

/** @import Player from './player' */
/** @import { ContentDescriptor } from '../utils/dom' */
Expand Down Expand Up @@ -29,11 +30,10 @@ class TextTrackSelect extends Component {
* @param {string} [options.id]
* A text with part of an string to create atribute of aria-labelledby.
*
* @param {array} [options.SelectOptions]
* @param {Array} [options.SelectOptions]
* Array that contains the value & textContent of for each of the
* options elements.
*/

constructor(player, options = {}) {
super(player, options);

Expand All @@ -57,7 +57,11 @@ class TextTrackSelect extends Component {
},
{},
this.options_.SelectOptions.map((optionText) => {
const optionId = this.options_.labelId + '-' + optionText[1].replace(/\W+/g, '');
// Constructs an id for the <option>.
// For the colour settings that have two <selects> with a <label> each, generates an id based off the label value
// For font size/family and edge style with one <select> and no <label>, generates an id with a guid
const optionId = (this.options_.labelId ? this.options_.labelId : `vjs-track-option-${newGUID()}`) +
'-' + optionText[1].replace(/\W+/g, '');

const option = Dom.createEl(
'option',
Expand Down

0 comments on commit f4186a0

Please sign in to comment.