"Skip to main content" link doesn't work correctly in the overlay

So could we just write a jQuery script to remove the 'Skip to main' in Overlay & redirect the URL in the first 'Skip to main' so that it bounces right into the first form in Overlay?

This would be the best behavior I think.

Latest patch in #716612: Overlay is not accessible to screen reader users adds tabindex="-1" to the parent skip-link as long as the overlay is open.

Subscribe

Changing to major as per tag.

Tag update

As reported in #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay I've encountered this in IE7 and yoroy in FF on Mac. The problem afaict is that focus is not given to the overlay page strait away in those browsers, it does work just fine in other browsers.

I think this is blocking #896364, because its totally unreliable with this problem - I doubt we even have any idea just how unreliable, what we do know is that IE7 and FF on Mac are problematic.

Yep, postponing #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay on this. We need a reliable way for overlay pages to have tab focus.

What needs testing in supported browsers:
When entering an admin page in the overlay, hit the TAB key. Which part of the page gets focus? Can you get the 'Skip to content' link to show up?

Sorry, but lets escalate this for a bit then :\

Fixing tag.

Heres a screen-shot to show the issue clearly, this is taken in IE9, while it might appear the skip nav is taking focus, its not, thats Bartiks skip link showing through from underneath the overlay:

I believe this is the comment and patch referred to in #2.
So I ripped the guts out, and here's the gist of the attached patch, as far as I understand it:

• removes event-based Drupal.overlay.eventhandlerRestrictKeyboardNavigation(), since it doesn't work
• replaces it with state-aware logic that applies ARIA role=presentation attributes to elements intended to be disabled during overlay
• implements functionality to force JAWS's virtual buffer to refresh itself after updating ARIA attributes

casey can probably explain it more thoroughly and accurately, and hopefully will point any outstanding issues.

I applied the patch in #14 and tested in IE9. When the overlay opens the skip link is selected/focused by default. After tabbing through all the focusable elements on the page the browser took the focus and then coming back into the page focus went to the underlying page (Bartik).

I guess this needs to be worked on by someone who wants to debug javascript in IE.

On the vanishingly small chance that someone else is thinking about working on this today, I'm assigning it to myself.

So it turns out that jQuery UI's :tabbable filter is only an approximation of what's actually tabbable on the page. I switched to using a straight list of element types that can accept focus, which speeds things up as well. I also removed the ARIA-related parts of the patch, since we moved away from trying to use ARIA roles. It seems to work in IE8 and IE9, plus FF3.6/Ubuntu and Chrome. Would love an IE7 test.

The part of this that bothers me is that we're setting focus on the skip link every time overlay opens. That link isn't even intended to be visible if you're not a keyboard user. I'm thinking we'd do better to add a dummy element right before the skip link and set focus there, so the skip link shows up on the first tab. But I'm out of time for the evening, so I'm posting what I have.

will test this NOW :D!

The skip to conent link is always visible when loading a webpage in overlay.
Is that intentional? Cause thats rly confusing for sighted users.

I always thought only tabbing could trigger that link.

Did some IE7 testing, except for the visual issue, this is working.

Agreed, although we should show the skip to content link at all times, we have decided as a community to only show it on focus. So, we should not be showing it by default in Overlay. Focus should start out before the skipt to content link.

Right - that's what I said in the second paragraph of #19.

I'm not sure when I'll have time to come back and fix the skip link, so I'll unassign myself in case someone else gets a chance to work on it.

Srry ksenzee missed that part, and I was thinking the same about the possible solution.

Related #939014: Overlay doesn't get focus in Opera

It sounds like we all agree with @ksenzee's proposal to "add a dummy element" that first receives focus. See #19.

Later today I can test in IE7 too.

Just looked at patch #19 in Safari 5.0.2, OSX 10.5.8:

• "Skip to Content" link gets initial focus, but after tabbing away, I cannot tab back to it (neither by shift-tabbing or cycling all the way through)
• None of the admin toolbar links are reachable via tabbing

This is presumably not a webkit issue, because the same bugs do not manifest themselves in Chrome 7.0.517.41 on the same system.

Finally, in general, it seems weird to set focus anywhere but the element with the lowest tab index. Forgive me if this is a UI/UX-ignorant question, or if this has been covered already:

• Instead of adding a dummy element for tabbability's sake, would not a better approach be to re-order the tab indexes of the tabbable elements?

+++ modules/overlay/overlay-parent.js
@@ -813,6 +787,50 @@ Drupal.overlay.getDisplacement = function (region) {
+  $('*', context || document.body).each(function () {
...
+  $('*[tabindex]', context || document.body).each(function () {

This line definitely kills any browser that does not happen to run on a QuadCore WhatSoEver.

I'd love to implement the solution, but I forgot Overlay's implementation details in the meantime, so I can only point to the solution:
#336924: Massive page rendering slowdown with many links

1) Whatever key you press, a browser fires an event (a.k.a. 'keypress').

2) Events bubble up.

3) We can catch any event on the top-level document/body.

The effective code is visible in http://drupalcode.org/viewvc/drupal/contributions/modules/google_analyti... - critical credits fly out to quicksketch for that - but then again, we only need the first 2 lines from within the actual .ready() code... ;)

Powered by Dreditor.

• "Skip to Content" link gets initial focus, but after tabbing away, I cannot tab back to it (neither by shift-tabbing or cycling all the way through)
• None of the admin toolbar links are reachable via tabbing

These issues are specific to Safari "universal access" settings, please ignore.

I'm not sure the approach sun suggests in #29 will work with screenreaders, and I don't think the performance impact of walking the DOM is quite as bad as he fears, but I'll do some screenreader testing and see if I can make it work.

Before I throw out the walking-the-DOM approach, here's an optimized patch for comparison's sake. It adds the dummy element I mentioned in #19, and it optimizes the DOM iterator code. In FF3.6, this patch adds about 550ms when there are 200 links on the underlying page whose tabindex we need to change. When you add admin_menu into the mix (which adds a whole bunch of links whose tabindex we're not changing), it adds ~1500ms. That's about the worst real-life case I can picture. So if capturing keypress doesn't work out, we can fall back to this patch. And to be honest, I'm not too hopeful about capturing keypress; as it happens, we have an existing keypress handler that's intended to control taborder, and this patch rips it out because it's not working. :(

In other words: I think this patch is good to go, in all aspects except performance on pages with a gazillion links. Meanwhile, I'll try to get the existing keypress handler to work per sun's suggestion, because it would definitely be more performant.

I got a working patch cobbled together using the keypress handler approach. It unfortunately wasn't much faster than #32 (for reasons that I won't go into unless someone actually cares). I was about ready to admit defeat when it occurred to me that in real life, hardly anyone actually sets tabindexes on anything. So by checking for the tabindex attribute on everything outside the overlay, then only recording tabindex for those elements, we can speed things up a lot. I tried it and it was *much* faster - 115ms in FF for a page with 200 links plus admin_menu enabled. Whew.

I've tested this in FF and Chrome, and it works quickly and well. More cross-browser testing would be great.

Patch #33 works great for me in Chrome and Safari on OSX.

#33: 841184-33-fix-skip-link.patch queued for re-testing.

#33 Works for me in firefox chrome and Safari.

I tested in IE6 and IE7 and it threw up. I haven't quite finished fixing everything, but I'm out of time for today, so here's what I have so far. This patch quits trying to make _recordTabindex and _restoreTabindex into functions usable as anything but a jQuery.each callback, and adds iframes to the list of elements that are able to receive tab focus, so we don't make the entire overlay untabbable (oops!).

Setting focus to the placeholder element is still failing in IE7 at the jQuery insertBefore() call. Will track that down as soon as I can.

Patch in #38 fixes #939014: Overlay doesn't get focus in Opera.

So it turns out that in IE6-7, jQuery.fn.insertBefore() is broken with iframes, and tabindex attribute CRUD is horribly broken as well. This patch does a browser check (I know, I know...) to work around all that brokenness. I've tested it in every browser I have access to (IE6, IE7, IE8/XP, IE8/Win7, IE9 beta, FF3.6/Ubuntu, Chrome/Ubuntu). If @aaronbauman or others would retest it on a Mac that would be fabulous.

I tested this in IE7, IE8, IE9, chrome and ff.
Works great.

I rerolled cause there was a newline missing between an @param and @return block.
Nothing special.

tested 41 on OSX FF, Safari, Chrome - works great!

Awesome

Great job, all! For everyone who did cross-browser testing on this, can you also test the patch in #639460-55: Evaluate CSS of #skip-link, .element-focusable, and upcoming "disable overlay" links for their impact on contrib/custom themes applied in conjunction with this to make sure the styling looks right in all core themes across all browsers we care about, both in and out of the overlay? Jacine did some great CSS cleanup of the skip link in that patch: would be great to get some confirmation that there's no regression introduced by it. Please post feedback about it in that issue. Thanks!

Do we have any sense of the performance impact? Iterating over the entire document might be expensive. We expressed some fears about that above but haven't seen anyone done a comparison.

(If the overlay can be disabled easily, is this still required for accessibility?)

ksenzee posted some performance info in #33.

@Dries: The overlay can't be disabled easily unless this patch goes in. That's why it got marked critical in the first place.

I ran performance tests on all the browsers I have. In modern browsers the impact is negligible - between 10 and 150 ms, but ~50ms in most cases. IE6 and IE7 however are predictably quite slow with this. Without admin_menu enabled, they add a couple of seconds, which is bad but not insurmountable. However, if you enable admin_menu or otherwise add a ton of links to the page, it's slow enough to trigger the "do you want to abort this script?" prompt. And to be honest I don't have any ideas on how to fix it. The time-consuming part is determining what has a tabindex attribute, and there is no fast way to do that in those browsers. And without manipulating tabindex values, we can't limit keyboard access to the underlying page.

So the only thing I can think of is to quit trying to limit access to the underlying page in IE6 and IE7. I'm attaching a patch that does that. The problem Jeff Burnz points up in #16 will still apply in IE6 and IE7: "When the overlay opens the skip link is selected/focused by default. After tabbing through all the focusable elements on the page the browser took the focus and then coming back into the page focus went to the underlying page (Bartik)."

Oh, and this is back to needs review.

I don't know how many blind people use IE6 or IE7. I don't know how many people who have disabilities that prevent them from using a mouse use IE6 or IE7. But while we should do everything in our power to make the web as friendly as possible to people who can't easily fix their eyes or limbs, I don't think it's critical for us to bend over backwards to fix problems that are the responsibility of browser manufacturers to fix, especially when the browser manufacturer has fixed those problems and released that fix for free as IE8. I'm assuming disabilities don't prevent someone from upgrading to a working browser.

So, personally, I'm ok with #47.

I completely agree with #49

I agree that we should drop IE{ancient} support. But not because of screenreader users. If I understand correctly, there are old versions of JAWS that only work with IE6. That means some JAWS users are stuck using IE6 until they shell out $1000 for a new screenreader. They're the only IE6 users I give a rip about, and if I could make overlay accessible for them I would. But we've already given up on that. All we're trying to do here is

a) make the skip-link appear at the top of the page for all users;
b) make the overlay usable for sighted keyboard-only users.

So screenreader users are already out of luck. :( All we can do for them is a), and we've accomplished that. It's the sighted keyboard-only users that will run into problems with IE6 and IE7, and for whom we're trying to do b). *Those* are the people for whom we're dropping IE6/IE7 support. And yes, those people can just darn well upgrade their browser.

It's late so I will leave a brief comment now, and * may * expand it in the morning. To be an equitable community, something that we are good at, I think we need to provide the same degree of support to IE 6 / IE 7 for sighted keyboard only users as we do for any other users. If the suggested solution is consistent with this then I don't have too big of a problem, if it represents inconsistency then I would have a problem.

Of course, we are also looking at there not actually being a good solution, so we might rather say that while we wish we could make this work perfectly for all users in IE6 / IE7 it really just isn't technically possible. I would be content with that approach. It certainly isn't for lack of trying.

Again, thanks to all who have battled with this and related issues.

Patch in #47 does not work for me in Mac Firefox 3.6. It works perfectly on Mac Safari 5.0.2 and Chrome 7.0. (So I hope the Firefox is me doing something wrong).

The code itself looks good and well-organized. Lots of great work on this, folks!

@Everett: You're right, it's not for lack of trying. I have spent a lot of time, both Acquia-sponsored and on my own, specifically on IE6 and IE7 support. I believe the patch in #41 represents the absolute best that is technically possible, and it's so slow that in some cases it would make the site unusable. The technology just plain isn't there. We are not discriminating against any group of users here; we're doing the best we can for everyone.

I would love to see another test on FF3.6/Mac, since @brandonojc reports a possible problem. Any takers?

The way I understand it is this:

1. Overlay is a pretty new technology (not just our implementation in Drupal, but modal dialogs in general are relatively new for the web).
2. Some people believe it provides a UI improvement, and therefore, should be available to people who have the technology to support it (a sufficiently modern browser, a sufficiently modern screen reader).
3. Some people will visit a Drupal site in a really old browser, one for which jQuery isn't supported at all, and for those people, overlays won't be displayed. But that's ok, they'll still be able to interact with all of the administrative pages, just not in an overlay (and not with any of the other JavaScript enhancements like drag-and-drop as a way to set weights).
4. Some people will use a screen reader that's capable of reading all Overlay content and otherwise interacting with Overlay. Yay!
5. Some people will use a screen reader that's incapable of reading all Overlay content, or otherwise is unable to fully support all Overlay interaction. These people will be able to disable the Overlay once #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay is solved, and once they do so, will have full access to all administrative pages, just not in an Overlay.
6. Some keyboard-only users will use a modern browser, for which, after this issue is solved, tabbing will work correctly in the Overlay. Yay!
7. Some keyboard-only users will use IE6 or IE7, for which tabbing within the Overlay won't work quite right, as described in the last sentence of #47. Just as with item 5, these people will be able to disable the Overlay once #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay is solved, and once they do so, will have full access to all administrative pages, just not in an Overlay.

That we have already achieved #4 above, that this issue achieves #6, and that the other issue will achieve #5 and #7, is an enormous testament to the commitment the Drupal community has to accessibility. This kind of dedication to inclusiveness makes me proud to be a part of it.

I'm tempted to RTBC, but #54 indicates that a little more testing might be in order.

@effulgentsia

OT: #4 isn't solved, just as an FYI. We will work on solving #4 for D8. At present I (we) weren't sufficiently comfortable that any of the million attempts to solve the problem would a. improve experience for modern screen-redaers and not b. degrade experience for others (including making it difficult to disable overlay). Again, the hundereds / thousands of hours that were put into attempting to solve 4 was an amazing testament to our community's resolve to ensure that our product is accessible to all. If you want an overview in more detail of the above msg me and I'll give more background.

I applied the patch works and it works in FF3.6 for Mac

But, is the position to where this link jumps ok? Seems like toolbar/shortcuts are not accounted for and cover things up. I realize this is specific to the visual representation, screenreaders are not affected but:

In overlay, the top admin category titles on configuration page are hidden from view:

(And in Bartik a node title gets covered up: http://skitch.com/yoroy/d77em/skippedtocontent)

Is this something this patch even changes? I'm fine with a followup for it. Even think there might already be an issue for it but a quick search found me nothing.

Tempted to RTBC but this needs verification I think.

Repeat: the patch works with FF3.6 on Mac though :)

@yoroy: Yeah, that sucks. It's going to be especially confusing when we get the overlay toggle link in, and people follow it to their profile page, where they will see absolutely nothing relevant, because the toolbar covers up the fieldset they went there to see. :( But that is a totally separate issue, not changed by this patch at all, and it belongs to the toolbar module.

If this weren't my own patch I'd RTBC it. It's ready.

For anyone interested, the relevant toolbar module issue is #546126: Toolbar interferes with anchor links. It looks like webchick marked it as critical when it was first reported in August 2009 - in fact, it was on her alpha hit list - but it was demoted to normal this February for no reason that I can see, which is unfortunate. If it had stayed in the critical queue it might be fixed by now.

Thank you. I should have checked how it works before applying the patch.

RTBC! Summary in #55

OK, ok. Committed to CVS HEAD. :)