search

sampotts / plyr

A simple HTML5, YouTube and Vimeo player
https://plyr.io
MIT License
26.61k stars 2.93k forks source link

[Accessibility] Multiple Issues making player invalid for WCAG 2.0 AA compliance #905

Open fleps opened 6 years ago

fleps commented 6 years ago

Updated and more detailed issues reported on https://github.com/sampotts/plyr/issues/905#issuecomment-389909727

Original comment:

I'm working on a project that is being converted to meet WCAG 2.0 AA level compliance with the help of a specialized company to do an extensive QA on the project and report all issues found. The team found some issue on the Plyr plugin to a point that we will probably need to ditch it and develop our custom player on top of the HTML5 video tag (unless I use the non-minified version and try to fix myself)

I don't expect this issues be fixed soon but I'm reporting them here in order to help, because many accessibility requirements are really hard to understand and we as developer tend to miss-understand some details, so this experience of having a consultant specialist working with us is being really important to learn a few things.

Here are the issues found (PS: I don't know the exact version of the plugin, the min JS doesn't have the version, but for sure is before v3.)

Slider

Semantic information about the timeline and volume slider control is communicated visually but the current value or the min/max value is not available to all screen reader users.

Modification Recommended

Keyboard Access and Visibility

1 - The video Play, Mute/unmute and full screen button SVG elements are keyboard accessible by default when using Internet Explorer. This may confuse keyboard and screen reader users, as there are two-tab stops for each button.

Modification Recommended

2 - The volume slider has no visible focus indicator. Mouse users get visible cues that a page element is actionable (mouse pointer change, rollover color change, etc.) that are not available to keyboard-only users.

Modification Recommended

Findable Added Content

The Video’s percent loaded (buffering) function is wrapped in <progress> HTML5 tag which, in some browser/screen reader combinations behaves like an ARIA live region. This interrupts screen reader users as they are trying to listen to the content play. There are no means to turn off this feature, so it would continue to interrupt until the video was completely loaded and since the user is not actively doing anything (because they’re listening to the video) the entire video might be constantly interrupted by the load progress updates.

Modification Recommended

Hope it helps

friday commented 6 years ago

Nice write-up!

I'm guessing you're in a better position than anyone to make these changes and have them reviewed. The elements are being created in controls.js using methods in utils.js. See for example createRange, createProgress. It seems to be mostly about adding properties, which is really not that hard. At least compared to creating your own alternative to plyr.

sampotts commented 6 years ago

This is great feedback and as they look like small fixes, it shouldn’t take me long. I want to make sure it’s as accessible as possible so I’ll jump onto these straightaway.

sampotts commented 6 years ago

OK I've fixed the majority of these issues in this commit: https://github.com/sampotts/plyr/commit/a97b08e8ea0022646eae005ce64a7edf8b26fb29#diff-502df26753b20c330d95456a4c8db867

Some notes:

fleps commented 6 years ago

Hi @sampotts , great to know that you are giving accessibility this importance.

In the meantime I will also ask to the Accessibility Consultant to do a second pass on the player to see if there's any other issue they didn't get, and update here if anything new is found.

Thanks!

sampotts commented 6 years ago

Awesome. I really appreciate your time reviewing.

sampotts commented 6 years ago

With the <progress> would aria-hidden="true" not do anything or perhaps role="presentation"?

fleps commented 6 years ago

Hi @sampotts I think what the team meant with "a button to turn it off" is to have a button that removes / stop updating the element all together, maybe even deleting it from the DOM.

I'm not sure the aria-hidden will completely overwrite the screen readers behavior, if I'm not mistaken I saw an issue with some screen readers for this specific element in the past.

I'll check with the team for a confirmation.

sampotts commented 6 years ago

Hey @fleps , v3.2.1 has the majority of the fixes for these issues in, only thing missing is the toggle for <progress> pending confirmation from your team. Let me know 👍

sampotts commented 6 years ago

Hey @fleps , any more feedback?

fleps commented 6 years ago

Hi @sampotts Sorry for the delay on this. I'm still waiting approval from the client to extend the accessibility consultant contract for this, as it appears is out of the original scope. Business, you know. It will happen, I'm just not sure yet when, but I'll let you know as soon we have the analysis.

fleps commented 6 years ago

Hi @sampotts

Sorry for the delay but I took a different approach with the team review, and we actually asked them to evaluate the player on your demo page as it's the last version.

This turned out to be a more detailed issue report from accessibility stand point, but also more complex than I was expecting as he listed 10 issues on the current version of the player.

Do you prefer that I update the initial post on this topic, add the issues on a new comment, or close this and open a new one?

Let me know.

sampotts commented 6 years ago

Hey,

No worries about the delay. I'm thankful for your time testing. Feel free to use this issue and I'll action the changes required.

fleps commented 6 years ago

Hi @sampotts

As mentioned our consultant team did an extensive testing on the player and here are the issues found and the recommendations.

1) Frames

The imasdk.googleapis.com iframe is used as a shim, or to capture events, or for data transport, but it is not ignored by screen readers. Screen reader users may be confused by information that should be hidden.

Recommended Modification Hide the iframe as follows: Use CSS display:none or visibility:hidden on the iframe element. --- OR --- Add role="presentation" and tabindex="-1" to the iframe element. Use the attribute title="empty" to indicate the iframe contains no user information.

2) Buttons definitions

The Play/Pause, Mute/Unmute, Disable/Enable Captions, and Enter/Exit Full Screen buttons are defined as toggle buttons but they do not indicate the state correctly because the name of the buttons changes when activated. Screen reader users will not understand the current state of the controls.

Recommended Modification

The aria-pressed attribute should not be used on buttons where the button name changes to indicate the state. This causes incorrect information to be communicated to screen reader users. For example, the Play button is announced as "Play toggle button not pressed" which is correct, but when the button is activated it is announced as "Pause toggle button pressed" which communicates that the video is paused, but it is not.

Note: The full screen button is not displayed in IE.

3) Sliders

The Seek and Volume sliders are semantically defined but use multiple methods to label the control. There is a label element with a for attribute and aria-labelledby referencing the label element. This may complicate screen reader interaction with these controls. In addition, the slider values are not properly communicated to screen readers.

Recommended Modification

Note: In IE, the PageUp and PageDown keys do not work on the sliders as they do in Firefox and Chrome. While this interaction is optional, it allows keyboard users the ability to move the slide in larger increments which increases the usability.

4) Menus Definition

The Settings menu uses dropdown menus that do not define the proper role or state information for the expanded menus. The expanded menus are instead defined as a tab control with incorrect roles and properties defined on the tablist child elements. Screen reader users will not understand this menu and will have difficulty interacting with it.

Recommended Modification

See http://www.w3.org/TR/wai-aria-practices/#menubutton for more information and examples.

5) Settings Keyboard Access

The expanded Settings menus do not have the keyboard interaction users expect. Screen reader users are told to use the arrow keys to navigate the menus but this does not work properly. In addition, when navigating the radio submenu items, a selection is automatically made and the menu is collapsed. Keyboard and screen reader users will have difficulty with the menus.

Recommended Modification

See http://www.w3.org/TR/wai-aria-practices-1.1/#menu for details.

6) Buttons Keyboard Access

The buttons elements cannot be used with the keyboard consistently across different browsers and Assistive Technology/browser combinations. Content is not accessible to screen reader or keyboard users when mouse actions are required.

Recommended Modification

7 ) Focus visibility

a) (IE Only) The video controls, including the menus, have no visible focus indicators in IE. Mouse users get visible cues that a page element is actionable (mouse pointer change, rollover color change, etc.) that are not available to keyboard-only users.

Recommended Modification Make sure that there is a clear visual indicator when an object receives focus and when the mouse pointer moves over the object. For example, you could add an outline or change of background color.

b) Keyboard focus indicators are used in Firefox and Chrome but are difficult to distinguish against the colors used for the buttons and background for the large Play button overlaid on the video and for the seek and volume sliders. Keyboard-only users and low-vision users may not be able to see where they are on the page.

Recommended Modification Change the focus indicator to be more visible. If a custom focus indicator is used, it should have at least a 3:1 color contrast ratio between the color used for the focus indicator and the background it is used on.

8) Tab Order

The non-interactive video container is in the tab order. Screen reader and keyboard-only users expect only interactive elements to be in the tab order. In some browsers/screen readers, this will be announced as "clickable" which will be confusing to screen reader users.

Recommended Modification

9) Contrast on focus

Image icons used for the video controls have insufficient contrast (less than 3:1) when they have keyboard focus. Users who are color blind or cannot perceive color may not perceive the content.

Recommended Modification Increase color contrast to have 3:1 color contrast ratio when the buttons have focus.

Note: This issue is has low priority because it is not currently a requirement under WCAG 2.0 however, it will be a requirement in the upcoming WCAG 2.1.

10) Valid Markup

The video player contains coding errors that may prevent assistive technologies (AT) from interpreting content correctly. AT users may be unable to perceive or interact with page content and forms.

Recommended Modification Fix the validation errors. image image

That's it Sam.

As I mentioned, this became more complex than I was expecting, so let me know what are you thoughts on this, and also if you need clarifications on any of the issues reported.

If you decide to fix this, when everything is completed I'll ask the consultant to verify the player again and see if everything is ok.

Hope it helps!

sampotts commented 6 years ago

This is epic. Awesome feedback and I will action it ASAP and keep you updated :-)

On 18 May 2018, at 1:34 am, Felipe Spengler notifications@github.com wrote:

Hi @sampotts

As mentioned our consultant team did an extensive testing on the player and here are the issues found and the recommendations.

1) Frames

The imasdk.googleapis.com iframe is used as a shim, or to capture events, or for data transport, but it is not ignored by screen readers. Screen reader users may be confused by information that should be hidden.

Recommended Modification Hide the iframe as follows: Use CSS display:none or visibility:hidden on the iframe element. --- OR --- Add role="presentation" and tabindex="-1" to the iframe element. Use the attribute title="empty" to indicate the iframe contains no user information.

2) Buttons definitions

The Play/Pause, Mute/Unmute, Disable/Enable Captions, and Enter/Exit Full Screen buttons are defined as toggle buttons but they do not indicate the state correctly because the name of the buttons changes when activated. Screen reader users will not understand the current state of the controls.

Recommended Modification

The aria-pressed attribute should not be used on buttons where the button name changes to indicate the state. This causes incorrect information to be communicated to screen reader users. For example, the Play button is announced as "Play toggle button not pressed" which is correct, but when the button is activated it is announced as "Pause toggle button pressed" which communicates that the video is paused, but it is not.

Either remove the aria-pressed attributes from the buttons OR keep aria-pressed and do not change the name of the button to reflect its current state. Also remove role="tooltip" from the elements used for the button names, this role is not appropriate for this content. Note: The full screen button is not displayed in IE.

3) Sliders

The Seek and Volume sliders are semantically defined but use multiple methods to label the control. There is a label element with a for attribute and aria-labelledby referencing the label element. This may complicate screen reader interaction with these controls. In addition, the slider values are not properly communicated to screen readers.

Recommended Modification

Remove the for attribute from the label elements and keep aria-labelledby attribute. The seek slider value should be communicated as the time shown visually. Currently aria-valuemax="100" and the current time (aria-valuenow) is a percentage and not the actual time. Set aria-valuemax="183" (total minutes) and use aria-valuetext to communicate the actual time (e.g. aria-valuetext="00:23 of 03:03"). The aria-valuetext attribute should be used when alphanumeric values should be indicated to screen reader users The volume slider current value is not communicated properly. When the volume is at 100 the value is communicated as 1. Use aria-valuetext to communicate the volume percent (e.g. aria-valuetext="100%"). Note: In IE, the PageUp and PageDown keys do not work on the sliders as they do in Firefox and Chrome. While this interaction is optional, it allows keyboard users the ability to move the slide in larger increments which increases the usability.

4) Menus Definition

The Settings menu uses dropdown menus that do not define the proper role or state information for the expanded menus. The expanded menus are instead defined as a tab control with incorrect roles and properties defined on the tablist child elements. Screen reader users will not understand this menu and will have difficulty interacting with it.

Recommended Modification

Remove role="tooltip" from the with the button name for the Settings button. Remove all of the tablist, tabpanel, and tab roles. Remove all instances of aria-labelled-by (the attribute is also misspelled, but this is not required on these elements). Define the menu using WAI-ARIA: Use role="menu" on the

    elements and add role="presentation" on the
  • elements for the lists in the expanded menus. Use role="menuitem" on the buttons in the expanded menus. Userole="menuitemradio" and the aria-checked attribute for the submenus with radio buttons (remove the radio input and label elements). In the submenus, the buttons at the top (Captions, Quality, Speed) are defined as menu buttons but they do not open a menu, they navigate back to the previous menu. Remove aria-haspopup and aria-expanded from the buttons and add visually-hidden text "go back to previous menu" so screen readers understand the action of these elements. See http:www.w3.org/TR/wai-aria-practices/#menubutton for more information and examples.

    5) Settings Keyboard Access

    The expanded Settings menus do not have the keyboard interaction users expect. Screen reader users are told to use the arrow keys to navigate the menus but this does not work properly. In addition, when navigating the radio submenu items, a selection is automatically made and the menu is collapsed. Keyboard and screen reader users will have difficulty with the menus.

    Recommended Modification

    When keyboard focus is on the Settings menu button, pressing space or enter opens or closes the menu. When the menu is opened, keyboard focus should be moved to the first element in the expanded menu. From within the expanded menus: Space/Enter opens a submenu if present and keyboard focus should be moved to the first item or activates the menu item and closes the menu. Up-arrow should move keyboard focus to the previous menu item or wraps to the last. Down-arrow should move keyboard focus to the next menu item or wraps to the first. Left-arrow should move keyboard focus back to the previous menu. Right-arrow should open the submenu and keyboard focus should be moved to the first item, if no submenu moves to the next menu item. See http://www.w3.org/TR/wai-aria-practices-1.1/#menu for details.

    6) Buttons Keyboard Access

    The buttons elements cannot be used with the keyboard consistently across different browsers and Assistive Technology/browser combinations. Content is not accessible to screen reader or keyboard users when mouse actions are required.

    Recommended Modification

    Do not auto-hide the control bar when a control is activated. Before triggering the auto-hide, wait for a period of inactivity. Keyboard only: In FF, The Play/Pause button cannot be activated with the keyboard, it changes briefly but then switches back to the previous state. For the other buttons, the button with focus activates that button but also activates the Play/Pause button and hides the control bar. In IE, once the video starts, the Play/Pause button cannot be activated with the keyboard or mouse. In Chrome, the spacebar only activates the Play/Pause button regardless of which button has focus. With a screen reader running: The buttons cannot be activated using NVDA/Firefox. The Play/Pause button cannot be activated using JAWS/IE. 7 ) Focus visibility

    a) (IE Only) The video controls, including the menus, have no visible focus indicators in IE. Mouse users get visible cues that a page element is actionable (mouse pointer change, rollover color change, etc.) that are not available to keyboard-only users.

    Recommended Modification Make sure that there is a clear visual indicator when an object receives focus and when the mouse pointer moves over the object. For example, you could add an outline or change of background color.

    b) Keyboard focus indicators are used in Firefox and Chrome but are difficult to distinguish against the colors used for the buttons and background for the large Play button overlaid on the video and for the seek and volume sliders. Keyboard-only users and low-vision users may not be able to see where they are on the page.

    Recommended Modification Change the focus indicator to be more visible. If a custom focus indicator is used, it should have at least a 3:1 color contrast ratio between the color used for the focus indicator and the background it is used on.

    8) Tab Order

    The non-interactive video container is in the tab order. Screen reader and keyboard-only users expect only interactive elements to be in the tab order. In some browsers/screen readers, this will be announced as "clickable" which will be confusing to screen reader users.

    Recommended Modification

    Remove the tabindex="0" from all non-interactive elements. In addition, remove the aria-label attribute, its use is not valid in this context:

    9) Contrast on focus Image icons used for the video controls have insufficient contrast (less than 3:1) when they have keyboard focus. Users who are color blind or cannot perceive color may not perceive the content. Recommended Modification Increase color contrast to have 3:1 color contrast ratio when the buttons have focus. Note: This issue is has low priority because it is not currently a requirement under WCAG 2.0 however, it will be a requirement in the upcoming WCAG 2.1. 10) Valid Markup The video player contains coding errors that may prevent assistive technologies (AT) from interpreting content correctly. AT users may be unable to perceive or interact with page content and forms. Recommended Modification Fix the validation errors. That's it Sam. As I mentioned, this became more complex than I was expecting, so let me know what are you thoughts on this, and also if you need clarifications on any of the issues reported. If you decide to fix this, when everything is completed I'll ask the consultant to verify the player again and see if everything is ok. Hope it helps! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.
fleps commented 6 years ago

Hi @sampotts

My time to ask hehe, do you have any update on this?

Thanks!

sampotts commented 6 years ago

Hey @fleps , sorry it's still work in progress. Trying to get back to it ASAP. Will keep you updated 👍

fleps commented 6 years ago

Hey @sampotts Np, thanks for replying.

Whenever you have an ETA please let me know if possible.

Thanks!

fleps commented 6 years ago

Hi @sampotts

Just checking in if you have any updated on this or need any additional help / support.

Thanks!

fleps commented 6 years ago

Hey @sampotts, how it's going with these fixes? I saw there's some commits so I imagine some are already fixed?

Is there any particular issue that is harder and I may help?

sampotts commented 6 years ago

Hey @fleps,

Sorry I've been trying to get to it. I've got some time now so getting back to it. It's the re-working of the menu that's taken me the most time and getting my head around the new structure. I'm not sure there's anything that I'm struggling with per-se. I'll report back as I make progress.

sampotts commented 6 years ago

One part around the focus styles was going to involve using this: https://github.com/WICG/focus-visible But it looks like it's not possible to confine it's use to one element: https://github.com/WICG/focus-visible/issues/126

So that may have to wait for now and I'll look at why the current setup doesn't highlight focus in IE11. It should but IE is a right PITA as we all know.

fleps commented 6 years ago

Hey @sampotts

About the focus on iE11, I didn't take a look on the code but maybe check what CSS properties you are using to write the outline. I know that iE doesn't support outline-offset for example so in some elements we had to add a 1 or 2px transparent border so the outline is correctly visible.

Or if the element already use borders with colors, then we add a 2px padding on the element parent/container, so the outline has enough space and doesn't get cut by the parent element.

Hope that helps.

Edit: sorry about the close/open, misclicked the button -_-

sampotts commented 6 years ago

Thanks for that - I'll revisit. From memory we're using box-shadow - I think it just needs to be less subtle. Sorry, it's the designer in me.

Apologies for the delay on it. I want to get it out ASAP myself too but want to make sure it's spot on before asking you to check it again.

fleps commented 6 years ago

Hey @sampotts , how it's going with this? Any ETA?

Thanks

sampotts commented 6 years ago

Hey @fleps, I've just raised a PR for @friday to take a look at. Once that's in develop then I'll push it to a beta test page so you can take a look. I'll also push a beta tagged version to npm.

sampotts commented 6 years ago

I have pushed the improvements to hopefully rectify the issues outlined in v3.4.0-beta.1 which is now on npm and also live here: https://plyr.io/beta. If you want to take a look and let me know. I'll also double check myself tomorrow.

fleps commented 6 years ago

Hi @sampotts

Great job done on the plugin guys!

I've tested the beta version here and I found some issues considering the report:

3) Sliders

4) Menu Definitions

5) Settings Keyboard Access

Extra: Also noted there's a debugging console enabled, is this intended / to be configurable?

I'll still do some more testing here but that's what I got so far.

sampotts commented 6 years ago

Thanks for that. I'll try and clear those up tonight if I get time. It's my top priority to get this wrapped up.

sampotts commented 6 years ago

Sorry, I've been out of action with a cold/flu 🤒 . I've just pushed 3.4.0-beta.2 to npm (and updated https://plyr.io/beta) which contains fixes for all of those bar the IE11 (I'm really surprised IE11 has issues 😂) and NVDA issues which I'll need to do more testing with.

brennanyoung commented 6 years ago

Really excellent work guys! We chose plyr because of a11y, and because it is light and elegant. We've been disappointed with other tech chosen on the basis of a11y promises that didn't deliver.

Very impressed with keyboard operation on this thing. Superb. You made my life easier and gave us something I can solidly recommend to colleagues and peers.

As mentioned by fleps, still some issues with the settings menu. I can't seem to focus on (and thereby select) the lang radiobuttons with keyboard, apart from the one that's already selected.

(I agree with fleps, the up/down arrow keys should move the focus between items when the settings menu is open. This behavior is recommended by the W3C ARIA patterns document. They operate the volume control atm.)

Hope your cold is better @sampotts :)

sampotts commented 6 years ago

I'm still working on the remaining issues which are:

I'll look at these two next. The rest has been released in v3.4.0. I won't close this until it's fully resolved.

fleps commented 6 years ago

Great @sampotts I'll wait for these last issues to do a full review then.

Thanks!

fleps commented 6 years ago

@sampotts it's been quite some time since the last changes, any updates on the remaining issues?

sampotts commented 6 years ago

Sorry - I've been away on vacation. I'll try and get to the remaining issues this week.

Also, with regards to the ARIA menus, I saw this: https://www.marcozehe.de/2018/09/22/wai-aria-menus-and-why-you-should-generally-avoid-using-them/ Do you have any feedback on that?

Veyfeyken commented 6 years ago

Marco is correct. ARIA menus do more harm than good.

brennanyoung commented 6 years ago

Interesting article. But my takeaway is not "don't do it", but rather "don't just add the role and imagine that it is enough".

In other words, if you use the aria menu roles, you should go the whole hog, and ensure all the associated behaviour is in place. If not, don't use those roles.

I've asked on the web-a11y slack for their comments about the article.

fleps commented 6 years ago

Hi @sampotts

I believe the article is more concerned about the overusing of the aria menus when not necessary, like a simple website menu that can be entirely solved with simple html semantics using ul > li > ul.

The player settings menu is pretty complex and I imagine that for this solution they believed that the aria menu was the way to go, as the consultant team take in consideration the structure that was already on the player and also to the specific user case they are testing for.

sampotts commented 6 years ago

Cool thanks for the feedback. I'll leave that as-is then! 👍

fleps commented 6 years ago

@sampotts any updates on this? It's been quite some time with basically the iE11 and NVDA issues open as far I remember.

sampotts commented 6 years ago

Sorry I'm really struggling to debug on IE11. It's such a drainer and time waster. I'll get there though.

Team-Download commented 4 years ago

Hi @sampotts,

Keyboard Disabled is not working on fly. Can anybody help me on this.

I have to implement the functionality in which i have to disable keyboard event on plyr.

I am setting below property false on fly but these changes are not reflecting on player.

this.player.config.keyboard.focused = false; this.player.config.keyboard.global = false;

Although same property is working during first time when player is being instantiatiing: like : var player = new Plyr('.js-player', { controls, captions: { active: true }, keyboard: { focused: false, global:false, });

Can you please help me to sort out this issues. So that i can change keyboard enabled/disabled functionality on fly.

Team-Download commented 4 years ago

@sampotts : any updates? Its really very urgent..

brennanyoung commented 4 years ago

Something odd with keyboard focus when you use the keyboard to operate the fullscreen button. Plyr does go full screen, and screen reader announces that ESC will take you out again, which it does. All good.

BUT when you enter full screen mode, the focus indicator is still on the full screen button (displaying the 'pressed' state), although the button is not operable, unless you tab away, and then back again.

woutthielemans commented 1 year ago

Bumping this issue: is development still being done on the items listed above? I haven't checked every single one of them, but landed here whilst trying to work out/fix two specific issues we had on a project:

sampotts commented 1 year ago

Hey, happy to get those changes in. Looking at the above, all that was left was IE11 focus changes (but I'm going to ignore IE11 like Microsoft has) and an issue with Firefox/NVDA and the menu which I'll need to retest after all this time.

woutthielemans commented 1 year ago

Hi @sampotts, great!

role="timer" looks like the perfect solution here, this was also proposed by an accessibility audit we received for the project we're using Plyr in 👍

sampotts commented 1 year ago

v3.7.7 has these two fixes in, plus a change to leverage :focus-visible instead of the old custom solution I'd put in. https://github.com/sampotts/plyr/releases/tag/v3.7.7