Page MenuHomePhabricator

OOjs UI: Do not let me tab outside a dialog box in Chrome/Safari
Closed, ResolvedPublic

Description

This is related to T69153: VisualEditor: Tab order or behavior is bad in the Save dialog, but specific to Chrome/Safari:

Whatever the dialog box, once you've tabbed through whatever links and buttons are inside that dialog (and on the tab list, which isn't always all of them), then pressing tab takes you outside the dialog box to these items:

  1. the browser's URL bar
  2. the main Wikipedia search box
  3. the Cancel button (on VisualEditor's toolbar)
  4. the Save button (on VisualEditor's toolbar)
  5. the top of the article
  6. the slug at the top of the article, where it gets stuck forever.

and never lets you get back to the dialog box. This means that if you press tab one time more than you meant to, you cannot use or close the dialog box without taking your hands off the keyboard.

See Also: T69162: VisualEditor: Can't tab from Caption to Alt text fields in image media dialog box, in Safari only

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:27 AM
bzimport added projects: OOUI, Accessibility.
bzimport set Reference to bz67156.

Krenair, is this something that we do as part of the aria-role stuff (and is it something we should do)?

I don't think we do this deliberately. This happens in Chrome for me... Are you sure this is Safari-specific, Sherry?

Firefox lets me leave the dialog box, but it eventually tabs back into it. Safari gets stuck. I haven't checked Chrome; Safari and Chrome often have the same problems, so it might be anything WebKit/not Mozilla.

In that case this feels like an upstream issue

So after some confusion about this I looked into it again, and I believe that there's two issues here:

  • We perhaps shouldn't let tab take the focus out of the dialog box at all
  • Tab definitely should not be getting lost and become unable to return to the dialog

Different browsers appear to do different things:

  • Chrome on Ubuntu lets it out of the dialog, but it properly goes back and doesn't get stuck.
  • Chrome and Safari on Windows just let the tabbing leave the dialog, and then it gets stuck and doesn't return to the beginning of the page
  • Firefox (on Ubuntu or Windows) does not let it out of the dialog

Change 159902 had a related patch set uploaded by Alex Monk:
Try to stop user from tabbing outside of open dialog box

https://gerrit.wikimedia.org/r/159902

Change 159902 merged by jenkins-bot:
Try to stop user from tabbing outside of open dialog box

https://gerrit.wikimedia.org/r/159902

Merged. Hopefully this doesn't break anything else.

The applied patch throws an exception, see gerrit.

Patch was reverted, so what's the status of this? We need to do the same again, but without :focusable?

I looked at the source code and :focusable can be reasonably approximated using input:enabled:not([tabindex='-1']), select:enabled:not([tabindex='-1']), textarea:enabled:not([tabindex='-1']), button:enabled:not([tabindex='-1']), a[href]:not([tabindex='-1']), [tabindex]:not([tabindex='-1']), or even just *:enabled:not([tabindex='-1']), a[href]:not([tabindex='-1']), [tabindex]:not([tabindex='-1']). If that's too nasty, then we might also do [tabindex]:not([tabindex='-1']) and probably no one will notice the difference (OOUI focusable elements generally have tabindex=0 set, even if that's not strictly necessary).

Can we do it using observe and fixup, e.g. listen to focusin on the body, and if the event originates form outside the dialog, move it back inside the dialog.

The reverted patch did, in fact, do something like that. But we still need to find the place inside the dialog where we want to move the focus, and it used :focusable for this.

I did some googling and found http://accessibility.oit.ncsu.edu/training/aria/modal-window/version-2/, which uses a[href], area[href], input:not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled]), iframe, object, embed, *[tabindex], *[contenteditable]. Sadly, I think they're wrong in both using [disabled] rather than :disabled and not checking [tabindex='-1'].

Note also that :focusable implied :visible, we'd need to add this filter ourselves.

if jQuery UI can add a selector focusable and 'tabbable' for this. Can't we mix those in ourselves ? We'd have to account for the situation where both jQueryUI and OOjs UI are loaded, but we can do that with prefixes I think...

Some more notes:

  • The previously suggested focus trap only works for tab, not for shift-tab
  • a focus trap only fixes keyboard navigation, not screenreader accessibility
    • You can use aria-hidden to hide everything outside the dialog, opacity does not hide things....

Suggested reading:

Turns out that we have a method to find focusable elements now (6ff7e5c7ae384038b314dc90036f0a8468e7e65a), but it's a bit broken (T107343). Let's try to fix it and use it.

Change 237505 had a related patch set uploaded (by Esanders):
Re-attempt I31ab2bace4: Try to stop user from tabbing outside of open dialog box

https://gerrit.wikimedia.org/r/237505

Esanders set Security to None.
Esanders updated the task description. (Show Details)

Change 237480 had a related patch set uploaded (by Esanders):
Rewrite isFocusableElement for performance and correctness

https://gerrit.wikimedia.org/r/237480

Change 237480 merged by jenkins-bot:
Rewrite isFocusableElement for performance and correctness

https://gerrit.wikimedia.org/r/237480

Change 237505 merged by jenkins-bot:
Re-attempt I31ab2bace4: Try to stop user from tabbing outside of open dialog box

https://gerrit.wikimedia.org/r/237505