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

More duplicate HTML IDs #8761

Closed
boris-petrov opened this issue Jun 8, 2024 · 0 comments · Fixed by #8762 · 4 remaining pull requests
Closed

More duplicate HTML IDs #8761

boris-petrov opened this issue Jun 8, 2024 · 0 comments · Fixed by #8762 · 4 remaining pull requests

Comments

@boris-petrov
Copy link
Contributor

Description

After updating from 8.12.0 to 8.15.0 I started getting more duplicate ID errors from my HTML validator.

Reduced test case

N/A

Steps to reproduce

The duplicate IDs that the validator gives are: -50, -75, -100, -125, -150, -175, -200, -300, -400, -None, -Raised, -Depressed, -Uniform, -Dropshadow, -ProportionalSansSerif, -MonospaceSansSerif, -ProportionalSerif, -MonospaceSerif, -Casual, -Script, -SmallCaps. The first one, for example, I see here:

<div class="vjs-track-settings-font"><fieldset class="vjs-font-percent vjs-track-setting"><legend id="captions-font-size-vjs_video_2739_component_3312">Font Size</legend><select id="vjs_select_3389" aria-labelledby="captions-font-size-vjs_video_2739_component_3312"><option id="-50" value="0.50" aria-labelledby="captions-font-size-vjs_video_2739_component_3312 -50">50%</option>

I have a second video in the page so I guess that's why there are two of these.

Errors

No response

What version of Video.js are you using?

8.15.0

Video.js plugins used.

No response

What browser(s) including version(s) does this occur with?

Chromium

What OS(es) and version(s) does this occur with?

Linux

@boris-petrov boris-petrov added the needs: triage This issue needs to be reviewed label Jun 8, 2024
@mister-ben mister-ben removed the needs: triage This issue needs to be reviewed label Jun 9, 2024
mister-ben added a commit that referenced this issue Jun 10, 2024
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
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 12, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.