videojs / video.js

Video.js - open source HTML5 video player
https://videojs.com
Other
37.82k stars 7.43k forks source link

Accessibility issues on 'captions settings' modal #6516

Open rezaies opened 4 years ago

rezaies commented 4 years ago

Description

There are orphaned labels on the captions settings modal.

Steps to reproduce

Explain in detail the exact steps necessary to reproduce the issue.

  1. You'd need the WAVE browser extension for this test
  2. You'd need a video with caption loaded into the videojs player
  3. Play the video
  4. Click on the cc button and then choose 'captions settings'
  5. Press the WAVE icon on your browser

Results

Expected

There should not be any 'Orphaned form label' warning

Actual

There are some 'Orphaned form label' warnings

Error output

If there are any errors at all, please include them here.

Additional Information

Please include any additional information necessary here. Including the following:

versions

videojs

7.6.5

browsers

NA

OSes

NA

plugins

NA

welcome[bot] commented 4 years ago

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can. To help make it easier for us to investigate your issue, please follow the contributing guidelines.

rezaies commented 4 years ago

Fig  8 01

gkatsev commented 4 years ago

The way that we have implemented the caption settings dialog is definitely far from perfect, partially due to me not really knowing much about accessibility ahead of time and learning as I go. The warnings are because we use a label element but don't use the for attribute. Instead, we use aria-labelledby on the items it labels, it isn't ideal but it works. We've tested on VoiceOver, NVDA, and JAWS, so we're fairly confident it isn't an issue to screen readers. Because it works, we're unlikely to change it in the short term, but we always want to improve our accessibility, and we will eventually fix this up to eliminate the warnings.

OwenEdwards commented 4 years ago

To be clear, this is a "warning" from the WAVE toolbar, not an "error". As @gkatsev mentions, video.js uses aria-labelledby to provide labels for several of the Captions Settings controls; it actually associates two parts of the label with several select, which isn't possible with the conventional label and for structure.

It is not invalid HTML to have a label that isn't associated with an input or control; as WAVE's description of this warning notes, "It usually indicates a coding or other form labeling issues." Video.js is an exception to that "usually".

Certainly, the <label> elements could be changed to <span> elements to fix this warning, but that would impact CSS compatibility. Other fixes may be possible, but we don't want to break something that works currently.