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

Update aria settings with KLG's suggestions as appropriate #841

Closed
heff opened this issue Nov 21, 2013 · 7 comments
Closed

Update aria settings with KLG's suggestions as appropriate #841

heff opened this issue Nov 21, 2013 · 7 comments

Comments

@heff
Copy link
Member

heff commented Nov 21, 2013

@karlgroves posted about how div's aren't appropriate for buttons. I emailed him the Video.js reasoning, and he made the following suggestions for updates to the ARIA settings.

I'd like to get a second set of eyes on these first, hopefully from @gdkraus who did the most recent work on this, and maybe @majornista or @jtangelder.

*Notes: *
I don't think the comment "The 'display:none' here negates all the ARIA..." actually applies. We only add display:none on those buttons if there are no subtitles/captions/chapters tracks. If there were, those buttons would have display:block and would be showing.

<div class="video span10 offset1">
    <div id="home_video" class="video-js vjs-default-skin vjs-paused vjs-controls-enabled vjs-user-inactive">
        <video class="vjs-tech" poster="/img/poster.jpg" preload="none" id="home_video_html5_api">

            <source type="video/mp4" src="http://vjs.zencdn.net/v/oceans.mp4"></source>
            <source type="video/webm" src="http://vjs.zencdn.net/v/oceans.webm"></source>

        </video>
        <div></div>

        <div class="vjs-poster" tabindex="-1" style="background-image: url(&quot;/img/poster.jpg&quot;);"></div>
        <div class="vjs-text-track-display"></div>

        <!--// KLG: May be good to add some text in here when the spinner is visible, or maybe mark the video as aria-busy='true' until loaded //-->
        <div class="vjs-loading-spinner"></div>

        <!--// KLG: use role of button. Toggle aria-pressed states on keydown & keyup. //-->
        <div class="vjs-big-play-button" aria-live="polite" tabindex="0" aria-label="play video">
            <span></span>
        </div>
        <div class="vjs-control-bar">
            <!--// KLG: use role of button. Toggle aria-pressed states on keydown & keyup. Get rid of aria-live //-->
            <div class="vjs-play-control vjs-control " aria-live="polite" tabindex="0">
                <div class="vjs-control-content">
                    <span class="vjs-control-text">Play</span>
                </div>
            </div>

            <!--// KLG: Give role of timer. See http://www.w3.org/WAI/PF/aria/roles#timer //-->
            <div class="vjs-current-time vjs-time-controls vjs-control">
                <div class="vjs-current-time-display" aria-live="off">
                    <span class="vjs-control-text">Current Time </span>0:00
                </div>
            </div>
            <div class="vjs-time-divider">
                <div>
                    <!--// KLG: Put this divider in CSS generated content instead  //-->
                    <span>/</span>
                </div>
            </div>
            <div class="vjs-duration vjs-time-controls vjs-control">

                <!--// KLG: Give role of timer. See http://www.w3.org/WAI/PF/aria/roles#timer //-->
                <div class="vjs-duration-display" aria-live="off">
                    <span class="vjs-control-text">Duration Time </span>0:00
                </div>
            </div>
            <div class="vjs-remaining-time vjs-time-controls vjs-control">

                <!--// KLG: Give role of timer. See http://www.w3.org/WAI/PF/aria/roles#timer //-->
                <div class="vjs-remaining-time-display" aria-live="off">
                    <span class="vjs-control-text">Remaining Time </span>-0:00
                </div>
            </div>
            <div class="vjs-progress-control vjs-control">
                <!--// KLG: Give role of progress //-->
                <div aria-valuenow="NaN" aria-valuemin="0" aria-valuemax="100" tabindex="0" class="vjs-progress-holder vjs-slider" aria-label="video progress bar" aria-valuetext="0:00">
                    <div class="vjs-load-progress">
                        <span class="vjs-control-text">Loaded: 0%</span>
                    </div>
                    <div class="vjs-play-progress" style="width: 0%;">
                        <span class="vjs-control-text">Progress: 0%</span>
                    </div>
                    <div class="vjs-seek-handle vjs-slider-handle" style="left: 0%;">
                        <span class="vjs-control-text">00:00</span>
                    </div>
                </div>
            </div>

            <!--// KLG: Remove aria-live attribute. Give button role.  Toggle aria-pressed states on keydown keyup.  //-->
            <div class="vjs-fullscreen-control vjs-control " aria-live="polite" tabindex="0">
                <div class="vjs-control-content">
                    <span class="vjs-control-text">Fullscreen</span>
                </div>
            </div>

            <!--// KLG: Give role of slider  //-->
            <div class="vjs-volume-control vjs-control">
                <div aria-valuenow="100" aria-valuemin="0" aria-valuemax="100" tabindex="0" class="vjs-volume-bar vjs-slider" aria-label="volume level" aria-valuetext="100%">
                    <div class="vjs-volume-level" style="width: 100%;">
                        <span class="vjs-control-text"></span>
                    </div>
                    <div class="vjs-volume-handle vjs-slider-handle" style="left: 100%;">
                        <span class="vjs-control-text">00:00</span>
                    </div>
                </div>
            </div>

            <!--// KLG: Remove aria-live. use role of button. Toggle aria-pressed states on keydown keyup. //-->
            <div class="vjs-mute-control vjs-control" aria-live="polite" tabindex="0">
                <div>
                    <span class="vjs-control-text">Mute</span>
                </div>
            </div>

            <!--// KLG: The 'display:none' here negates all the ARIA. It will be ignored. Check out http://oaa-accessibility.org/examplep/menubar1/  //-->
            <div class="vjs-subtitles-button vjs-menu-button vjs-control " aria-live="polite" tabindex="0" aria-haspopup="true" role="button" style="display: none;" aria-label="Subtitles Menu">
                <div class="vjs-control-content">
                    <span class="vjs-control-text">Subtitles</span>
                </div>
                <div class="vjs-menu">
                    <ul class="vjs-menu-content">
                        <li class="vjs-menu-item vjs-selected" aria-live="polite" tabindex="0" aria-selected="true">
                            subtitles off
                        </li>
                    </ul>
                </div>
            </div>

            <!--//  KLG: Check out http://oaa-accessibility.org/examplep/menubar1/ //-->
            <div class="vjs-captions-button vjs-menu-button vjs-control " aria-live="polite" tabindex="0" aria-haspopup="true" role="button" aria-label="Captions Menu">
                <div class="vjs-control-content">
                    <span class="vjs-control-text">Captions</span>
                </div>
                <div class="vjs-menu">
                    <ul class="vjs-menu-content">
                        <li class="vjs-menu-item vjs-selected" aria-live="polite" tabindex="0" aria-selected="true">
                            captions off
                        </li>
                        <li class="vjs-menu-item" aria-live="polite" tabindex="0" aria-selected="false">
                            English
                        </li>
                    </ul>
                </div>
            </div>

            <!--// KLG: The 'display:none' here negates all the ARIA. It will be ignored. Check out http://oaa-accessibility.org/examplep/menubar1/  //-->
            <div class="vjs-chapters-button vjs-menu-button vjs-control " aria-live="polite" tabindex="0" style="display: none;" aria-haspopup="true" role="button" aria-label="Chapters Menu">
                <div class="vjs-control-content">
                    <span class="vjs-control-text">Chapters</span>
                </div>
                <div class="vjs-menu">
                    <ul class="vjs-menu-content"></ul>
                    <li class="vjs-menu-title">
                        Chapters
                    </li>
                </div>
            </div>
        </div>
    </div>
</div>
@karlgroves
Copy link

"I don't think the comment "The 'display:none' here negates all the ARIA..." actually applies. We only add display:none on those buttons if there are no subtitles/captions/chapters tracks. If there were, those buttons would have display:block and would be showing."

Oh OK. In that case, it is fine.

@gdkraus
Copy link

gdkraus commented Nov 22, 2013

<button> vs. <div role="button">?

With <button> vs. <div role="button">, I think you have to look at what function the button is playing in the page. I don't think <button> has a state of pressed that persists after you press it. In other words, the only time a <button> is pressed is when you are in the act of pressing it. You can, however, make <div role="button"> that acts more like a toggle button. In this case there is a case of pressed (and unpressed) that persists when you are not interacting with the element.

The way the play/pause button is implemented kind of falls in both categories. On one hand, it is a single button which has two states - playing or paused. On the other hand, visually, when the play/pause button is toggled, it doesn't really look pressed. In fact, I'd have a hard time deciding whether "play" should be the pressed position or "pause" should be the pressed position. If VideoJS implements it one way, what's to keep another vendor from implementing it another way and just confusing matters? In the end, I don't think the state of pressed or unpressed is that important here.

I do agree that <button> is usually preferred over <div role="button">, however, both do work. The former should usually be the way it is implemented, but functionally, the latter does the same thing, just a little less elegantly. When I submitted the accessibility patches way back when I just used role="button" since the entire UI was composed of divs and spans. It would have been a lot more changes at the time to change the divs to buttons. role="button" used to be in there, so it looks like it got dropped since I submitted my patches. If we stick with divs, role="button" definitely needs to be back in there.

So should VideoJS use <button> or <div role="button">? Once we start talking about button vs. div, and whether we should include aria-live and aria-pressed, we get into screen reader support issues and what features they actually support, because not all ARIA attributes are equally supported. I put together this test page, because I was curious and didn't know.

http://accessibility.oit.ncsu.edu/dev/html/aria/button.html

Here are my takeaways.

  1. If you change the text of either <button> or <div role="button"> you need to include an aria-live="polite" attribute.
  2. If you want to convey the state of a button being in the pressed state you need to use <div role="button"> with aria-pressed attributes.

While <button> is the most semantically correct, you're really overloading a single element with multiple functions (play and pause). Usually that would be set up as two separate buttons, but because of design considerations you are combining them into a single element. Because of that I tend toward <div role="button"> because you are creating something different from what we expect with <button>. (However, if you want to go with <button> I think that's fine too.)

That being said, should you include aria-pressed with a <div role="button">? Semantically, yes you should, but functionally, I don't know that it adds much meaningful information. Again, what does pressed mean here? Visually, nothing is staying pressed, unless we all agree that playing is the up position and paused is the pressed position, or was that the other way around? :)

role="timer"

For role="timer", semantically I agree that it should be there. However, functionally, it does nothing at this time for assistive technology users. To me it looks like the role of timer is not even passed on to the OS layer yet in order for the screen reader to actually do something with it. Functionally, setting something to role="timer" also makes an item inherit aria-live="off". Since the OS does not seem to understand role="timer", for now I'd prefer to explicitly say aria-live="off" because then I know what will be happening with it for screen reader users.

I think of the timer role as the appendix of ARIA attributes. It just sits there doing nothing, waiting for something unexpected to happen.

role="progress" or "slider"

For the time progress bar, it should have role="slider". If I didn't include that in there last time that was a mistake on my part. I'll have to go back and look to see if there are any progress bars that would need role="progress". I see the spot in the code that Karl is referencing. I just don't remember seeing it in the UI.

Caption menu

The caption menu could definitely use some work. What I had submitted previously was basically just to get it to a functional level for screen reader users. Again, at the time it would have required significant changes to get it to where it should be.

@karlgroves
Copy link

<button> vs. <div role="button">?

I would strongly recommend <button> in all cases where it is feasible. Currently voice dictation software is not as robust as text-to-speech software and a <button> is the most reliable for all users.

For the rest of the comments, I agree.

@gdkraus
Copy link

gdkraus commented Nov 25, 2013

I agree that speech recognition software (Dragon Naturally Speaking) does not do a good job recognizing <div role="button"> as a clickable item.

@gdkraus
Copy link

gdkraus commented Dec 6, 2013

I looked back through the code and what I had contributed previously where it would write out role="button" and role="slider" is still there, but for some reason it's not getting written out to the UI anymore. It looks like the code has been restructured fairly significantly so I'll need to do some digging to see why those roles aren't getting written out. I'm happy to do that in the interim, but I guess I need to know what direction you want to go long term. Do you want to use <button> or <div role="button">?

Here's what is boils down to for Dragon Naturally Speaking users. For <button>OK</button> they can say "click ok" and the button will get pressed. For <div role="button">OK</div> Dragon doesn't recognize it as a clickable item, so they can't say "click ok". They have to keep saying "tab" to press the tab key until they are focused on the element and then the have to say "space" to press the space key to activate the button. So either implementation allows Dragon users to interact with it, but <button> makes is significantly easier.

@heff heff added the confirmed label Mar 5, 2014
@mmcc
Copy link
Member

mmcc commented May 23, 2014

Regarding the role's not getting written out to the UI, that was fixed with this PR.

The big issue with using <button> instead of div is simply cross-browser styling. This was a nightmare a few years ago and it's still not in a warm fuzzy place today. That's not to say it's not doable and we're willing, but this would take work on the styling side of things.

That being said, would using anchor tags help the situation at all? Forgive my ignorance of Dragon Naturally Speaking, but would they improve usability from tabbing all over the place? That would certainly ease the transition from what we have now.

Either way, that kind of change would need to make for a major version bump due to the potential affect on users.

@gkatsev
Copy link
Member

gkatsev commented Jul 25, 2016

I think this is mostly either fixed or no longer applies or we're working on it. Closing.

@gkatsev gkatsev closed this as completed Jul 25, 2016
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants