Fix skiplink behavior for Webkit browsers

Can you explain what parts of the patch are necessary to fix the problem?

Any chance it's possible to take out browser sniffing from the patch? if it works for opera and chrome, shouldn't hurt firefox, IE and the rest either?

(edit) also on opera I can't get the skip link to show up at all.

(edit2) It would be better to get the target element from the skiplink href as well, currently it's hardcoded to main-content.

@dcmouyard - any eta on that new patch?

Closed #1823308: Should the "skip to main content" link place cursor focus in the main content area? as a duplicate of this issue.

It works! Patch applies nicely. I can verify that this fixes the problem nicely in Chrome in Linux.

I don't seen anything wrong with going from href="#main-content" to href="#main" but wanted to note it. Will be better for mobile & low bandwidth.

It does simplify stuff to hit an ID rather than a link. I think we were targeting a link in D7 because it was required for some browser (but not sure which).

I'm totally in favor of adding role="main". It's outside the discussion of skip links, but it is right there and cleans things up.

The Themers are going to need some documentation on this. Good job!

Thanks @dcmouyard!

Are all these markup changes really necessary (renaming IDs and such)? I just tested and this problem occurs in D7 as well. Can we just get the minimum change that is necessary to resolve this bug, so that we can backport this fix without breaking themes? Other changes (like role="main" and probably renaming the divs) are out of scope for this issue and should be filed as separate issues. Thanks!

re-rolled. Mostly just removing aria landmarks.

@xjm - I do think the changes to ID's are necessary for consistency. Also cuts down the unneeded code associated with the skip links.

This patch fixes functionality for webkit browsers.

Comments #4 & #5 touch on a larger issue for Opera, & FF:
"also on opera I can't get the skip link to show up at all."

If you hit tab in Opera or FF, the focus goes to the first form element. This seems to be a "feature" of these browsers.
NOTE: Apparently, some people are getting the skip link to show up in FF. But, after playing around with FF settings I still can't get the link to show.

At the very least, this patch fixes the issue for Webkit browsers.

Thumbs up!

Talked this over more in person and this is not quite ready yet.

1) It's weird we're adding a JS library to modules/system. More often this gets added to misc/ directory. It sounds like this might've already been vetted above, but that's unclear.

2) It's definitely weird though to add it with scripts[] rather than with hook_library(). As a result of doing it that way, jquery.js isn't loaded for anonymous users, so it's broken for browsers.

adjusted.

#15: skiplink-1529814-13.patch queued for re-testing.

re-roll.

#20: skiplink-1529814-20.patch queued for re-testing.

Re-rolled to remove markup changes that have already been committed (probably as a result of #10)

Patch now only adds the system-skiplink.js file to core/misc and adds library in system.module.

Re-rolled for library version number.

Thanks for the reroll ibullock! However; it looks like we lost the skip-link markup in core/themes/seven/templates/maintenance-page.tpl.php and core/modules/system/templates/maintenance-page.tpl.php!

Also, we need a newline character at the end of the js file.

+++ b/core/misc/system.skiplink.jsundefined
@@ -0,0 +1,21 @@
\ No newline at end of file

Looks to me like every instance of the skip-link markup was removed. Is this no longer an issue then?

Tested with SimplyTest.me on the Mac. It works fine in FF, but seems to fail in Safari & Chrome.

And ya, this exists nicely in Bartik and where it matters most in Seven <a id="main-content"></a>:
core/themes//bartik/templates/maintenance-page.tpl.php: <a id="main-content"></a>
core/themes//bartik/templates/page.tpl.php: <a id="main-content"></a>
core/themes//seven/templates/page.tpl.php: <div class="element-invisible"><a id="main-content"></a></div>

But is missing in:
core/themes/seven/templates/maintenance-page.tpl.php

I do think this should be brought up in another topic issue though.

The skip-link HTML added to several files in dcmouyard's patch #8 seemed to be added when I rerolled, but has since been removed, or I was otherwise mistaken. Just looking for clarification on what this issue is actually trying to achieve. The issue seems to be about these links not functioning, rather than them not existing.

Example of one such missing link:

+  <div id="skip-link">
+    <a href="#main" class="element-invisible element-focusable"><?php print t('Skip to main content'); ?></a>
+  </div>

Edit: Realizing now that the patch was probably primarily meant to add the wrapping divs?

Yes, the problem we were trying to fix here is that for Webkit browsers the skip links are there, but just don't work. They don't take you to the #main ID on the page.

I do think it is worth adding the missing link (as you described in #30 that was in the patch in #8) here:
core/modules/system/templates/maintenance-page.tpl.php
core/themes/seven/templates/maintenance-page.tpl.php

But think that should be done in a new issue. Let's get the JS set up so that the behavior works as planned in all browsers.

#25: skiplink-1529814-25.patch queued for re-testing.

#25: skiplink-1529814-25.patch queued for re-testing.

I cannot reproduce the bug from another webkit browser, web(ex epiphany), it works there.

Can we make sure if this can be tested on last opera.

Also a minor note, it can be argued that skiplink is used mostly on browser with no javascript enabled, so maybe a javascript solution is not a good idea.

[Note: I have not applied the patch]

I can't reproduce this error on Chromium in Ubuntu. All focusable elements do receive focus as I tab through the page.

I *can* reproduce this on Opera 12.15 on Ubuntu 12.04. The only elements on the page that get focus are the two input fields (Username and Password) and the Log in submit button. None of the anchor tags get focus; it's not just the Skip to main content link. The expected behavior is that tabbing from the top of the page through the bottom, the following elements should get focus: Skip to main content, User account, the logo, the site title (drupal 8), the Home menu item, the node title, the image in the node, Read more, Log in, register, the next node title, Read more, the rss feed, Username, Password, Log in button, Create new account, Request new password, Contact, the Drupal in "Powered by Drupal".

The attached screen shot is from FF.

I am going to save the source as a static HTML page and remove any styling to see what happens.

I have tested this by saving the page as HTML and stripping out all style sheets. Opera still doesn't tab through the anchor tags but does tab through the input fields and the submit button.

Chromium and FireFox do tab through the anchor tags as well as the input fields and the submit button.

Opera will allow me to tab through all the anchor tags (but not the input fields or the submit button) if I use Ctrl-arrow down or Ctrl-arrow up. This works on both the actual web page as well as the modified HTML file.

Chromium and FireFox do not respond to the Ctrl-arrow useage.

Using Shift+arrow keys in Opera gives a different navigation; you can then navigate through the anchor tags and the input fields and submit button.

I came up with the idea to test the Ctrl and Shift sequences after reading: http://www.accessibleculture.org/articles/2010/05/in-page-links/

After doing the above research and discussing this with @marvil07 we have decided that it doesn't make sense to add javascript to work around a behavior that Opera has decided to implement. Users of Opera will need to be aware that this is how Opera chooses to do things and that it provides other options for navigating the elements on the page.

I verified that here http://simplytest.me/project/drupal/8.x

The last patch doesn't apply either, but that's to be expected.

Is this the right approach to go?

EDIT: Also see list of WebKit browsers http://en.wikipedia.org/wiki/List_of_web_browsers#WebKit-based

When I add tabindex="-1" to the div in my theme that has the id="main-contnet", it fixes the tabbing behavior in Chrome. Perhaps the bug has been fixed in webkit and we just need to implement the tabindex to make the div element focusable?

Indeed, testing this on vanilla Drupal 7 with Bartik, the skip link functionality is broken out of the box, but works if you add a tabindex="-1" to the #main-content anchor. Tested this in Chrome and not any other browser.

Whoops. I posted a D7-only patch. :-p

Here's the patch for D8.

I've updated the issue summary to note we should also update the drupal.org docs at https://drupal.org/node/467976

Ok, that fixes Chrome, and FF still works! Maybe that's enough to mark this patch RTBC and open up new ones.

Safari & Opera are both still broken though.

With Safari 6.0.5 the "Skip to main content" link drops down, but focus doesn't go anywhere.

With Opera 12.16, the "Skip to main content" link just doesn't show up.

I'm not certain that both of these are still WebKit-based, but would be good to get this fixed. Certainly for Safari.

With Safari 6.0.5 the "Skip to main content" link drops down, but focus doesn't go anywhere.

Ugh. You are right. Chrome is fixed with the patch, but not Safari. :-p

It looks like this is a broader problem with these browsers. Using a link with a #id target simply doesn't move the focus as it should. While we can fix this for Drupal sites with some JavaScript, that only fixes the Drupal site. The browser is still broken for keyboard navigation users on all the other sites. The only way they can work around the bug is to use a different browser or install a browser plug-in like https://chrome.google.com/webstore/detail/keyboard-navigation/abcekjakje...

I'm strongly opposed to adding a JS on every page for this problem given that a complete fix is beyond our abilities.

:-\

Fixing it in Chrome & FF may be good enough. Certainly don't want to add JS for this at this point.

I'm looking around for other solutions for Safari, but hopefully it's something that Apple can fix.

Can you see any reason not to implement the patch in #44?

I do think that D8 does need to have a bigger review in Safari though. The admin stuff does look weird, but that's another issue.

#44: 1529814-44-skip-link.patch queued for re-testing.

re-roll.

no longer applies.

Thanks @mark.labrecque - it isn't clear by looking at the code, but the headers were removed in #2218039: Render the maintenance/install page like any other HTML page

It was decided for the maintenance page & start page that the skip links weren't needed (as there really wasn't any content to skip).

I just deleted the core/modules/system/templates/maintenance-page.html.twig code from your patch

no longer applies.

Re-rolled patch.

Looks good. I tested the behavior in Safari & Chrome on the Mac. Also Chrome on Windows. Works well. Opera 12.16 is just well, different... Outside of scope at this point I think.

58: 1529814-44-skip-link-58.patch queued for re-testing.

bot error.

Committed 8e07c22 and pushed to 8.x. Thanks!

This is tagged for backport.

Patch for D7. This one adds the tabindex to Garland too.
Tested it with Chromium 34 on Linux Mint 16 KDE.

I updated the specimen code in the skip-links handbook page at https://www.drupal.org/node/467976

Also tested in Chrome on a Mac - Version 38.0.2083.0 canary (64-bit).

Looks good!

Probably this should be rolled out on a production site though first. I'll try to do that soon.

Sorry, this contradicts what I said in #60, but Safari still has the same busted behavior. As with my comments in #46 & #47 this isn't resolved. It's still better since it works well with Chrome now.. I'm putting in a request with Apple though.

I now can't seem to get to the toolbar using the keyboard in D7. If I just tab over the "Skip to main content" I go right to the breadcrumb in Seven. Tabbing forward & back I can't seem to get into the toolbar. I think the tabindex="-1" is messing up Drupal 7's toolbar.

Apple encouraged me to download, test, report directly using the webkit browser http://webkit.org/

It works fine with that browser however, which leads back to those who forked it...

minimizing number of tags...

This issue hasn't been looked at for a while. We should do some manual testing to see what the current situation is with a variety of WebKit- and Blink-based browsers.

I've run into this happening when the skip link's destination is a div instead of an anchor, since divs aren't naturally focusable elements.

Thanks @helenasue - so the solution you talked about at TCDrupal was this same one discussed here & add a tabindex="-1", right?

Hi @helenasue - Can you explain what you mean about the skip-link's destination being a div? They wouldn't normally be focusable, but in all D7 core templates the destination is an anchor element.

The patch in #67 adds the tabindex=-1 attribute, and still applies cleanly to the latest 7.x branch.

@mgifford: I'm interested in the problems you mentioned in #72 and #74. I don't have a mac handy for testing in Safari, and I can't replicate the problem with the toolbar. Are these still an issue currently?

If you review the patch in comment #1 on line 123, you'll see where that was a div at one point. As I followed the thread, I saw the destinations all become anchors over time. I was wondering if those replacements are in now, or if those are just proposed changes on this thread.

Sorry if I'm asking really dumb questions here - I'm newly trying to get involved with the issue queue and this format is somewhat confusing to me because I'm used to GitHub and not sure how to tell what's been merged or not.

I haven't seen this mentioned, but I know that skip links in general have some problems on Edge.

This might be outside the scope of this discussion, but if this is a problem for skip links, it will be a problem with all in-page links. I'm thinking specifically for things like tables of contents, which might also break. It's worth exploring whether a more general solution is feasible.

(I think Drupal ate an earlier version of this comment, so I apologize if this is posted twice).

I know skip links are also a problem in Edge, so this will benefit more than just Webkit. WebAIM considers it a best practice to use scripting to move focus explicitly (http://webaim.org/techniques/skipnav/#quirks).

Another thought I had: If this is a problem for skip links, it's also a problem for all internal links. I'm thinking specifically of tables of contents. I don't know if it's feasible to imagine a general solution for page links (rather than a specific solution for just skip links), but I would love to think about it. Skip links just seem to be a particular instance of a more general case.