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

screenreader support #103

Closed
Veyfeyken opened this issue Jul 30, 2015 · 20 comments
Closed

screenreader support #103

Veyfeyken opened this issue Jul 30, 2015 · 20 comments
Assignees
Labels

Comments

@Veyfeyken
Copy link

The current version does not work with the most popular screenreaders.

VoiceOver doesn't read the labels for the checkboxes (toggle mute and captions).
NVDA and Jaws only read the play-button because it has an aria-label. The other controls are read as "button".

Every control has an text-alternative <span class="player-tooltip"> that's shown on hover or on focus.

I think they're being ignored because they're set to "visibility:hidden".
Using CSS "clip" like you do in the class "sr-only" might fix it.

Using <span>'s with a description of the control might be better than using aria-labels. Aria support is not reliable among screenreaders yet.

@sampotts
Copy link
Owner

Ah sorry to hear about this. I'll install NVDA on my Windows box and fix it up. I'd been testing on Voiceover on Mac which I guess is fairly niche but even that doesn't read the tooltips for the checkboxes even though they have labels but I think it's to do with visibility: hidden like you say.

@sampotts sampotts added the Bug label Jul 30, 2015
@sampotts sampotts self-assigned this Jul 30, 2015
@sampotts
Copy link
Owner

What did you mean by "Using 's with a description"?

@Veyfeyken
Copy link
Author

<span>'s

@Veyfeyken
Copy link
Author

I think using the current descriptions that are shown on hover is the best approach. Don't use ARIA if you don't have to.
http://www.w3.org/TR/aria-in-html/#first-rule-of-aria-use

@sampotts
Copy link
Owner

Ok awesome. I'll work on this a bit later today/tomorrow as it should be a quick win. Thanks for reporting it 👍

@Veyfeyken
Copy link
Author

Thx for looking into this. I hope I can recommend the player as fully accessible in the near future.

@sampotts
Copy link
Owner

Hey, can you check out http://plyr.io? I think I've fixed this now?

@Veyfeyken
Copy link
Author

  • Works with NVDA 2015.2 in Firefox
  • Works with Jaws 16 and 15 in IE11.
  • Unfortunately VoiceOver still ignores the <label>'s for toggle mute and captions. (OS X Yosemite 10.10.4 with Safari 8.0.7). You're hiding them with opacity: 0, that shouldn't impact VoiceOver. I can't explain this behaviour.

@ZoeBijl
Copy link

ZoeBijl commented Aug 5, 2015

Why does the mute toggle button use an <input type="checkbox">? Why not use a <button>? You could mark it up as described in the Authoring Practices. Note: visual order for the buttons in that button example are messed up, toggle button is last everywhere in the code; see APG PR #75.

This could potentially solve the issue; as none of the other controls (buttons) seem to have the problem.

That only leaves the subtitle button, which can use the same—toggle button—markup as the mute button for now. If you ever decide to add multiple subtitle support, this needs other markup (because it not only toggles, but selects a specific subtitle).

@sampotts
Copy link
Owner

sampotts commented Aug 6, 2015

Good suggestion @MichielBijl , I'll change them to toggle buttons which should resolve it for Safari. Oddly, Chrome on OSX reads the labels correctly.

@sampotts
Copy link
Owner

sampotts commented Aug 8, 2015

I've implemented the 'aria-pressed' attribute on toggle buttons and it seems to get picked up in Chrome on OSX as a toggle button (also reads selected state) using voiceover. NVDA in windows also works too and states they are toggle buttons.

I've also made some improvements so that captions are read by NVDA etc (using live regions).

If you find anything else please let me know. I'm keen to make sure everything is top notch a11y wise.

@ZoeBijl
Copy link

ZoeBijl commented Aug 8, 2015

Nice work!

@Veyfeyken
Copy link
Author

Hi, looks like all controls are read by most screenreader-browser-combinations now. Good job.

Unfortunately "aria-pressed" is not well supported. Using toggle buttons like the play-pauze button would be better. "Mute - unmute", "captions on - captions off". That 'll work for every screenreader in any context. Again, if you can do it without ARIA, that's always better.

Regarding captions with live regions. This is more of a problem then an enhancement. You're using aria-live="assertive" and captions are on by default. The "assertive" property interrupts screenreaders, regardless of what they're doing. Therefore you're forcing screenreader users to listen to the captions. Captions are primarily meant for people with hearing problems, not visually impaired users. I would recommend undoing this.

@ZoeBijl
Copy link

ZoeBijl commented Aug 11, 2015

@Veyfeyken do you have any sources for aria-pressed support? According to this post on TPG blog JAWS supports it. Here is a document that states it's supported as far back as JAWS 8 + IE8. It was also added to Bootstrap by @patrickhlauke.

If there any tests for this that I failed to find, could you post them here?

Edit: seems there are some resources that rebut the use of aria-pressed. Like this test by Paul J Adam. So we can rule out Safari for now. Still interested in test results with other browsers/AT. I should have know Safari has this issue, I filed a bug report on aria-pressed at WebKit…

Anyhow, I discussed this on the a11ySlackers Gitter Channel and opinions are divided; some agree aria-pressed should be implemented (as it seems only Safari has this issue, and it's the right way forward), others feel a button that changes its label is better.

@sampotts
Copy link
Owner

Hey @Veyfeyken, I've removed the live region now so captions should no longer be read. I thought the same after I'd released it that perhaps it wasn't really needed given the captions intended purpose and thought about removing that. 👍

@sampotts sampotts reopened this Aug 11, 2015
@sampotts
Copy link
Owner

Happy to make a button change at some point...

@Veyfeyken
Copy link
Author

@MichielBijl , our own tests. VoiceOver ignored it. Jaws and NVDA have support. My colleague tested with Supernova, popular in Belgium and the Netherlands. To my surprise, even Supernova supports it. So only VoiceOver (with Safari) is a problem. So I humbly take back my statement, support is better then I thought. We didn't test with older browser versions or mobile.

@ZoeBijl
Copy link

ZoeBijl commented Aug 11, 2015

@Veyfeyken thanks for the information :)

Here is a link to my conversation on a11ySlackers.

@sampotts
Copy link
Owner

Chrome and Opera read the buttons as toggle buttons in Voiceover. I guess I should be consistent and change the play/pause to be a toggle switch or do you think it's ok as is?

Thanks again for your help 👍

@ZoeBijl
Copy link

ZoeBijl commented Aug 11, 2015

@sampotts the play button label changes to “Pause”; it's not a toggle button. So that is fine as is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants