Video.js aria labelledby and ids contain spaces

[Hannah-goodridge]

Description

There's seems to be odd spaces between aria labels in the video-js outputted markup inside the vjs track setting components.

Steps to reproduce

I've created a codepen in which demonstrates this issue
https://codepen.io/HannahG/pen/xXovgx

Annoyingly it's in an iframe but if you do a search for "captions-font-size-video_component_373" in dev tools you can see the issue. You get the below:
aria-labelledby=" captions-font-size-video_component_373"

This is an issue with the current build 6.2.8 and fails accessibility on the aria-labels.

Is there anything we can do to fix this?

Additional Information

We are using this for a government digital service and need AAA Accessibility if possible.

versions

videojs

6.2.8

browsers

Chrome Version 61.0.3163.100 (Official Build) (64-bit)

OSes

Windows

plugins

none

[gkatsev]

I've not seen this as an issue on OSX with voice over. Is any particular screen reader having issues with that extra space? But yes, we should definitely fix it if it's causing issues.

I can see in the code we're trying to set the aria-labelledby to be multiple things and if one doesn't exist there will be a preceding space, so, we could probably trim that.

[Hannah-goodridge]

@gkatsev Thanks for taking a look at this issue.
We use the wave accessibility tool seen here
which flags these issues as errors.
Anything you could to fix would be great :)

[knilob]

I also noticed some issues with this while doing some accessibility checks for the university where i work. I think I have a quick and easy fix I can submit to alleviate this issue. It looks like the legendId variable gets initialized in following methods:

createElFgColor_()
createElBgColor_()
createElWinColor_()

However, there's no legendId initialized for the createElFont_() method. It seems like adding a check for the legendId value in the createElSelect_() method where the aria-labelledby gets set on the select and the option tags should alleviate this issue.

I've added this to my local fork, but I can't get the tests or build to actually complete at the moment to submit a PR on this.

[knilob]

Disregard my comment about not being able to run the tests or the build. It turns out I just needed to pull the updates from upstream. I should have PR for this in a bit. Feel free to let me know if I totally botched something up.

[gkatsev]

Thanks @knilob. Hopefully, that's enough to fix this. I hope to have a release out this week.

[gkatsev]

actually, going to re-open this issue based on the discussion in #4708 so that we don't forget to fix that.

issue comment

looks like we have a bug in the generation of the aria-labelledby for the font families since the values themselves have spaces when we create the idea for it, it ends up messing up. For example, we end up with an id captions-font-family-vid1_component_423-Proportional Sans-Serif instead of something like captions-font-family-vid1_component_423-Proportional-Sans-Serif and then when we reference it it's wrong.

issue comment

trim() is probably better and now that I noticed we need a slightly better refactor, we probably should switch to that

[gkatsev]

Also, see #4884 for a description of the id having a space in it when it shouldn't have.

[jcbreel]

This seems to be an issue still.

https://monosnap.com/file/7ktqePRZb04kkqLVyukAiNhWeyOKYw

[OwenEdwards]

@jcbreel see #5167 (comment)