Dialog: Improve accessibilty #794

Merged
merged 43 commits into from Nov 26, 2012

Projects

None yet

4 participants

@jzaefferer
Member

add an aria-describedby attribute on the dialog if there is nothing yet in the dialog content.

@ezufelt could you give this a try? I've put together a test page for modal form dialog's with better markup then those in our demos:

http://view.jqueryui.com/dialog/tests/visual/dialog/form.html

That page has some info on what it provides and what we expect to happen based on the current implementation. Looking for input both on our assumptions as well as the implementation.

This change and a few already landed in master overlap with changes from this earlier pull request: https://github.com/jquery/jquery-ui/pull/704/files
Restoring focus to the trigger is implemented, setting the aria-describedby attribute is being added here, though only under the condition that there's no markup including an aria-describedby attribute yet.

I hope we can get away without the extra role="document" wrapper element. Base on my testing with current Firefox, NVDA and JAWS 12, it seems to work fine without it.

@jzaefferer
Member

@larowlan @ezufelt could you review and test this? Its supposed to replace #704.

@larowlan
Contributor
larowlan commented Nov 4, 2012

@jzaefferer have sent call-out to a11y folks involved with Drupal to help test

@mgifford
mgifford commented Nov 5, 2012

Well, I'm not much of a screen reader tester, but this is what I get in FF with VoiceOver:

Open Form Dialog Button - On focus

"dog" - content selected - "Your favorite animal" "Please share some personal information" "Please share some personal information" Text

Open Propt Dialog Button - On focus

Password Text

So it's better.

But It is really hard to understand the context that I'm getting dropped into with the Open Form Dialog Button & not sure why it repeats itself.

With the Open Propt Dialog Button I really don't know what I'm missing. I don't hear the other information on the dialog.

But I can open & close it pretty well & jump between the elements.

@larowlan
Contributor
larowlan commented Nov 6, 2012

Feedback from Vincenzo Rubano (falcon03 on drupal.org)

  1. FF 16 + Windows 7 + NVDA 2012.2.1 or Jaws 13: works as expected.
  2. IE 9 + Windows 7 + NVDA 2012.2.1: dialogs work as expected, even if all groups labels are announced and this could eventually create confusion for the end user.
  3. IE 9 + Windows 7 + Jaws 13: The first form dialog doesn't work as expected, since descriptions aren't announced. The second dialog works as expected, but the description is read only after the password field label.
  4. Safari + Mac OS X: . Dialog doesn't work as expected, since when it opens the cursor is moved to the first form field and the dialog title and dialog description aren't announced. If I want to read them, I have to move back to them and then come back to the first form field of the dialog. Not a great thing.
@jzaefferer
Member

Thanks @mgifford and @larowlan for the feedback. I just tested with VoiceOver on OSX 10.6 with both Firefox and Safari. In both cases VoiceOver doesn't read any of the aria-markedup descriptions. Consider that those work pretty well as intended for both JAWS and NVDA, I don't think we should try to work around VoiceOver's limitations with some hack like live-regions.

I'll also ping Hans Hillen about this, maybe he some more input.

@jzaefferer
Member

Merged the dialog-review branch. That improves focus handling, which is relevant here as well, and uses the button widget for the close button, which is more accessible as well (tooltip text, space-to-activate works).

@jzaefferer
Member

I've commented on the two dialog accesiblity tickets: http://bugs.jqueryui.com/ticket/7861#comment:6 and http://bugs.jqueryui.com/ticket/7862#comment:5
In short: Modal dialogs work really well, non-modals have issues.

I've tested, with VoicerOver the modified dialog by @hanshillen found here: http://hanshillen.github.com/jqtest/#goto_dialog
The result is the same: VoiceOver ignores aria-described and role=group elements, so text outside of the inputs can be read only using VO+cursor keys (ctrl+option+cursor up/down). Unhelpfully, VoiceOver then restricts that navigation to the surrounding group.

jzaefferer and others added some commits Oct 26, 2012
@jzaefferer jzaefferer Dialog: Improve accessibilty - add an aria-describedby attribute on t…
…he dialog if there is nothing yet in the dialog content. Partial fix for:
2a887e4
@jzaefferer jzaefferer Dialog: Keep focus inside modal dialog, by handling focus events on e…
…lements outside of it
8ee8046
@jzaefferer jzaefferer Dialog: move to top when opening again, and focus as if opened from s…
…cratch.
0a25b2c
@jzaefferer jzaefferer Dialog: Inline code review 2062a18
@jzaefferer jzaefferer Dialog: Fix yoda-if, remove unnecessary TODOs; add missing callbacks …
…to commons test
4632780
@jzaefferer jzaefferer Dialog: Remove deprecated disableSelection() usage. b6cefc7
@jzaefferer jzaefferer Dialog: Trigger focus event when dialog is moved to top. 4e03321
@jzaefferer jzaefferer Dialog: Use $.isEmptyObject() to check if there a button-option prope…
…rties
324d54d
@jzaefferer jzaefferer Dialog: Only add the new dialogClass, not the base classes when chang…
…ing the option.
f7d3a51
@jzaefferer jzaefferer Dialog: Remove attroperty workaround for title attribute. Make the de…
…fault null, as it should be. No back-compat, as the behaviour doesn't change.
6edce86
@jzaefferer jzaefferer Dialog: Focus tabbable only when dialog lost focus before. 0848040
@jzaefferer jzaefferer Dialog: Use button widget for close button (was already listed as dep…
…endency)
83a9f21
@jzaefferer jzaefferer Dialog: Refactor _setOption to call _super early. Move dialogClass up…
…date above that to access old option value.
fed2972
@jzaefferer jzaefferer Dialog: Have _createButtons access the buttons option directly. Start…
… refactoring _setOption, no need for switch.
7e964be
@jzaefferer jzaefferer Dialog: Finish refactoring _setOption, getting rid of unnecessary swi…
…tch (no fallthroughs).
16a79c1
@jzaefferer jzaefferer Dialog: Have _makeResizable look at options instead of passing handles. c8aef20
@jzaefferer jzaefferer Dialog: Remove outdated TODO 1001504
@jzaefferer jzaefferer Dialog: Refactor the mousedown-bind call to use _on, removing the nee…
…d for the uiDialog closure. Use this.uiDialog and remove the variable.
7a03535
@jzaefferer jzaefferer Dialog: Refactor uiDialogTitlebar variable, use this.uiDialogTitlebar…
… instead. Wasn't needed anymore (previous refactorings).
f0acaac
@jzaefferer jzaefferer Dialog: Refactor _create, extracting title bar creation in _createTit…
…lebar
1d6ce64
@jzaefferer jzaefferer Dialog: Extract button pane creation into _createButtonPane 4c9caa8
@jzaefferer jzaefferer Dialog: Extracting wrapper creation into _createWrapper. Merging the …
…two keydown handlers into one.
625a111
@jzaefferer jzaefferer Dialog: Remove useless tmp vars. 0ae6fc1
@jzaefferer jzaefferer Dialog: Button is now a fixed dependency, so remove the check 972f5c1
@jzaefferer jzaefferer Dialog: Remove busted ui-dialog-disabled class, shouldn't be there. R…
…emoved outdated TODOs.
0bc73b7
@jzaefferer jzaefferer Dialog: Remove uuid and getTitleId. Leftovers from 240b22b. e3dcaf2
@jzaefferer jzaefferer Dialog: Tiny code improvements d8b98ec
@jzaefferer jzaefferer Dialog: Pass through icons and showText (as 'text') options to button…
…. Fixes #6830 - Allow Icons to be specified for Dialog buttons.
9996173
@jzaefferer jzaefferer Dialog: Extend visual test to verify DOM position restore on destroy;…
… overhaul unit test for destroy method.
b694409
@jzaefferer jzaefferer Dialog: Cleanup in ticket tests: TODO to merge one test, fix whitespace 299681e
@jzaefferer jzaefferer Dialog: Extend autofocus, starting with [autofocus], then :tabbable c…
…ontent, then buttonpane, then close button, then dialog. Fixes #4731 - Dialog: Add option to set which element gains focus on open
b27db7e
@jzaefferer jzaefferer Dialog: Removed broken disabled option from dialog, defuse disable/en…
…able methods. Disabling dialogs is not supported.
0be97bf
@jzaefferer jzaefferer Dialog: Move array notation support for position option to backCompat…
… check. Fixes #8824 - Deprecate array notation for position option.
a0310eb
@jzaefferer jzaefferer Dialog: Refactor overlay handling into two instance methods. Remove u…
…naddressable TODOs.
41c2afd
@jzaefferer jzaefferer Dialog: Improve _destroy method, detaching dialog content from wrappe…
…r instead of appending to body.
32a8931
@jzaefferer jzaefferer Dialog: Add missing unit test for aria-describedby attribute 5aac8f5
@jzaefferer jzaefferer Dialog: Exclude deprecated units from phantomjs 73533d9
@jzaefferer jzaefferer Dialog: Update focus-tabbable test with a timer workaround to get IE8…
… to pass.
f3525af
@jzaefferer jzaefferer Dialog: Follow-up to 9fe3a62 - also deprecate string notation for pos…
…ition option.
a09f5c0
@jzaefferer jzaefferer Dialog: Follow-up to c77ca67 - exclude button options from properties…
… to create the button.
ec1f1bd
@kborchers @jzaefferer kborchers Dialog: Update position when size is changed. Fixes #8789 - Dialog do…
…es not close for first click on chrome.
d179cba
@jzaefferer jzaefferer Dialog: Don't focus dialog when mousedown is on close button. Fixes #…
…8838 - Dialog: Close icon does not work in dialog larger than the window in IE.
60486ac
@jzaefferer jzaefferer Dialog: Extract setting the title into a _title method, use .text() t…
…o prevent XSS. Fixes #6016 - Dialog: Title XSS Vulnerability.
7e9060c
@jzaefferer jzaefferer merged commit 7e9060c into master Nov 26, 2012
@jzaefferer
Member

Landed in master, closing six tickets.

@larowlan
Contributor
larowlan commented Dec 9, 2012

Great work guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment