search

dimsemenov / Magnific-Popup

Light and responsive lightbox script with focus on performance.
http://dimsemenov.com/plugins/magnific-popup/
MIT License
11.39k stars 3.48k forks source link

Multiple problems with focus and tab navigation #426

Open peterkuma opened 10 years ago

peterkuma commented 10 years ago

When the focus setting is set to a selector which does not yield any elements, focus is not set correctly in _setFocus, and tab navigation is allowed to break out of the popup. This can be seen in the Modal popup example, which currently sets focus to #username (probably by mistake), which is not present in the dialog. _setFocus should check if mfp.content.find(mfp.st.focus) is not empty and only then set focus (fall back to mfp.wrap otherwise).

If focus settting is present, calling _setFocus from _onFocusIn in order to contain focus within the popup prevents elements occuring before the focus element from being accessible by Tab. This is because when focus is removed from the last element of the popup (and to be moved to a page element), the _onFocusIn method gives focus to the focus element, instead of the first tab-focusable (tabbable) element of the popup. In practice, this prevents the Close button from being accessible by Tab when the focus setting is used.

There is also a question whether focus should ever be given to mfp.wrap as a fallback. I think it is better to give focus to the first focusable element in the popup. Esp. from WAI-ARIA 1.0 Authoring Practices:

When the dialog box is opened, focus should be set to the first tab focusable element within the dialog. If there is no tab focusable element within the dialog box contents, set the focus to the item that is used to cancel or close the dialog.

Finding the first tabbable or focusable element is a bit problematic (see this stackoverflow question). jQuery UI provides :tabbable and :focusable selectors which are supposed to do that, but they are not available to Magnific-Popup. There is a jquerry.tabbale package which should be able to do that, but I'm not sure Magnific-Popup would want to have such a dependency. There is also the possibility to integrate the code from jQuery UI or jQuery.tabbable, but it is mildly complicated. One can also come up with a simplified CSS selector to find focusable or tabbable elements with a few lines of code (but without handling some special cases).

I can provide a patch implementing the last mentioned solution.

peterkuma commented 10 years ago

Finding the first focuable element seems far too complicated to get right (see HTML Standard: 8 User interaction, so I produced a patch which just sets focus to mfp.wrap when wrapping tab navigation, and as a fall back when mfp.st.focus is not set or does not exist. This fixes both of the problems mentioned, but does not follow the ARIA guidelines.

With ChromeVox, the browser just reads the entire content of the popup (incl. the close button and arrows) when it is initially loaded. I don't know how bad this is compared to focusing the first focusable element, but I guess at least getting right the tab cycling and tab navigation confinement is an improvement.