Evaluate CSS of #skip-link, .element-focusable, and upcoming "disable overlay" links for their impact on contrib/custom themes

I do like this idea. Because the patch failed and because I thought that 'Skip link' wasn't descriptive enough I added a bit of text and re-rolled it.

arg, forgot to set the bots on it.

I'm not always all that good at finding the right text. I just not sure how many people outside of the #a11y community would know what it refers to. I think something about accessibility has to be in the text so that administrators have some idea what this would do. It's not like many of the other items which are pretty transparent. It's going to be more important when it's only visible on focus as I think most people will never even realize that this text is there.

It did seem odd that your patch failed. Applied nicely. But think it had windows carriage returns.

The setting works in all themes. But it only appears as an option here /admin/appearance/settings

It would be nice if it could be enabled/disabled with the other options. Not sure how that didn't propagate to any of the specific theme configuration pages (/admin/appearance/settings/garland & /admin/appearance/settings/seven).

I figure that most folks don't use Garland for a default theme and that generally folks will be modifying the page.tpl.php file and if they want to remove something will do it first there. I'm totally fine with both of these being theme configurable options. Just not sure how to best implement it.

+1 for this idea of allowing users to hide/show it. The code for the patch in #4 looks good, and it works successfully for me.

Note: The setting is added to the Global Settings (/admin/appearance/settings) but not to individual theme settings (eg, /admin/appearance/settings/seven). If it's easy to do, you may want to also add it to the latter so that users could hide it in some themes while showing it in others.

Trying to get around the bot's line break concerns.

arg, go bot go!

@Jeff:

Is your proposed design that "checked" means the the Skip Navigation is visible at all times and "unchecked" means it is invisible at all times (regardless of keyboard focus)? That's what it seemed like when I tested it.

I ask because there are really more options for the Skip links than just "show" or "hide" them. I can think of at least 4 settings people might want: A) it is visible for everyone at all times; B) it is invisible until it appears on keyboard focus; C) it is invisible at all times; D) it is completely gone from the markup. So this patch lets people toggle between A and C. I'm fine with that because some of the other options (like D) aren't best practices. I just want to make sure I'm understanding.

Ok, I like the patch in #14 except that we've got outstanding issues about how to describe it going back to #6. Would really like to have a clearer way to describe this.

It works just great in Seven, but Garland seems to behave the same way with the skip to main becoming visible on focus. So right now it doesn't seem to behaving just as it is described in #18.

Although I do think this is the behavior we want.

Essentially it's always on for people using screen readers and optionally visible for others. That might be a better way to describe it. Although with this patch here - http://drupal.org/node/386462#comment-2297094

It's not going to be visible anywhere without it being tabbed to and most folks will only know it's there if it's helpful to them.

Jeff, can you re-roll the patch in #14 to include the special casing for Garland & Seven & I'll mark it RTBC after testing in conjunction with the patch in this related issue http://drupal.org/node/386462#comment-2297094 which will address @Bojhan's usability concerns.

#14: toggle-skip-link_639460_v3.2.patch queued for re-testing.

Looking at the CSS rules in seven and garland's style.css files for handling #skip-link makes my eyes bleed, so I am in favor of a theme setting that lets you toggle visual display of that link without requiring every theme to have that cruft. But:

+++ modules/system/html.tpl.php	22 Nov 2009 23:21:22 -0000
@@ -45,7 +45,7 @@
-  <div id="skip-link">
+  <div id="skip-link" class="<?php print $toggle_skip_link; ?>">
     <a href="#main-content"><?php print t('Skip to main content'); ?></a>
   </div>
...
+++ includes/theme.inc	22 Nov 2009 23:21:28 -0000
@@ -2273,6 +2274,9 @@ function template_preprocess_html(&$vari
+  // Toggle the display of the Skip link.
+  $variables['toggle_skip_link'] = theme_get_setting('toggle_skip_link') ? 'show-skip-link' : 'element-invisible';

In system-behavior.css, the comment above .element-invisible says that this class "must not be used for focusable elements (such as links and form elements) as this causes issues for keyboard only or voice recognition users". So, is it safe to use this class on a "div" that wraps a link? I'd like an accessibility/css expert to comment on that.

This review is powered by Dreditor.

The stuff we used in Garland and Seven (especially) is a bit crufty, but after long debate over what constituted best practice it was decided showing on focus was it.

Given that, and given #17, I don't like it as a checkbox within "toggle display". All the other checkboxes there control whether that thingy is in the markup or not, and I don't like the idea of creating an inconsistency in that regard.

What do you think of splitting this into two issues: make this one about creating additional classes in system-behavior.css, perhaps ".link-visible", ".link-invisible", and ".link-visible-on-focus" (just suggestions, feel free to use alternate names if there are CSS best practices for how to name these kinds of classes). Assuming the CSS for these three could be in system-behavior.css and not depend too delicately on the theme CSS, then theme developers can have the behavior they want by setting a class in html.tpl.php.

If that works, a second issue could be opened to create a new settings group. Just like we have "Shortcut Icon Settings", we could have "Skip Link Settings" or "Accessibility Settings", and within that group, create a radio group, so that an administrator could toggle between these 3 options without needing to modify html.tpl.php.

@effulgentsia on your suggestion to add new CSS, I don't think we can re-open this issue in D7. It's a great topic for D8 though!

For the long history of this issue see: http://drupal.org/node/473396

Could someone summarize the remaining problems here?

1. If a theme wants to always have the "skip-link" DIV in html.tpl.php be invisible, but accessible to screen readers, can it add the "element-invisible" class to it? The comment above .element-invisible in system-behavior.css seems to say no, but Jeff believes that comment is wrong (#25).

2. If the answer to above is no, can we add a class such as "link-invisible" with appropriate CSS rules in system-behavior.css, so that a theme can achieve this without doing something wrong like adding "#skip-link {display:none}" to its style.css file? mgifford says this is out of scope for D7 (#27).

3. If either #1 or #2 is do-able, can we add another class such as "element-visible-on-focus" or "link-visible-on-focus" with appropriate CSS rules in system-behavior.css, so that a theme that wants to implement the "Skip to main content" link with the same functionality that both Garland and Seven use can do so without duplicating arcane CSS magic in its style.css file; just look at the rules for #skip-link in garland's and seven's style.css file: do we want that being duplicated in every theme?

4. If the above is answered in such a way that results in the visibility of the "Skip to main content" link (always visible, always invisible, or visible on focus) being controlled through which class is added to the "skip-link" DIV, then do we want a theme setting that lets an administrator control which option is used, or are we ok with the theme hard-coding the class in html.tpl.php?

@effulgentsia

4. If the above is answered in such a way that results in the visibility of the "Skip to main content" link (always visible, always invisible, or visible
on focus) being controlled through which class is added to the "skip-link" DIV, then do we want a theme setting that lets an administrator control which
option is used, or are we ok with the theme hard-coding the class in html.tpl.php?

I would like to see the ability for a site's administrator to be able to choose between the three options. I believe that this will provide the greatest balance between accessibility, and an administrators freedom to design their site as they see fit.

#6:

breadcrumb is the other one which should be a theme setting also, but that's another issue

indeed it is, but one that hasn't gotten much attention: #222569: Breadcrumb display should be a theme setting

Anyone monitoring the "markup" queue have thoughts on #29 (items 1-3)? If we can get that far, I can help with integrating the theme setting: that's the easy part, it's the CSS questions that are difficult (at least for me).

Give me a test plan and I'm in for testing!

Subscribe. I think a setting for this is a great idea.

A setting would be a "feature" which is too late for the D7 cycle.

As for consolidating the CSS so theme author's don't have to duplicate the rules in Garland and Seven… hmm… Reducing code duplication is an optimization, right? :-)

I would like to see the ability for a site's administrator to be able to choose between the three options. I believe that this will provide the greatest balance between accessibility, and an administrators freedom to design their site as they see fit.

So a site admin form would be a feature. I'm not opposed to (but also not crazy about) adding such a feature to D8. However, what if we just had a "show-link-on-focus" class (actual name TBD) that contained the appropriate CSS to make the link show on focus. So the themer (but not the site admin) would have these choices:

1. Use core's default html.tpl.php which does not have the "show-link-on-focus" class. The skip link would be shown at all times.
2. Override the html.tpl.php to apply the "show-link-on-focus" class to the skip link. The skip link would only be shown on focus.

Would that be sufficient choice? and for the right people?

Thanks for giving this attention, John! Please see #26 and #29. Let's start with the CSS class(es) (I don't know if 'show-link-on-focus' is sufficient, or if we need all three suggested in #26). Such a patch should also change system/html.tpl.php to put a variable class on the needed element. Then a themer can choose what to do using a simple THEME_preprocess_html() function. And a module/theme can add a UI to expose the choice to the administrator, if we can't get such a UI into core. At any rate, I agree with you that we should start with the CSS classes, then open a separate issue for whether adding a setting to core is ok for D7. Unfortunately, I have no idea how to write the CSS, so I'm relying on you or Jeff Burnz or someone else following this issue to take the next step.

Ok, so we're looking at splitting this issue thread into two pieces.

1) D8 feature to change the Skip Navigation display as an option for admins

2) D7 optimization of existing Skip Navigation CSS.

There is some discussion about the two different methods used here:
http://drupal.org/node/790192#comment-2929532

What's clear from the initial implementation of Corolla is that more documentation is needed and that there is confusion about what is needed and what the best practice is. #7 & #9 are both relevant in that thread.

So, who wants to step forward & split the thread & push ahead with optimizing/documenting the CSS for skip to main content links?

Bumping to D8. Neither of the issues here needs to be completed for D7.

Hmm. #897638: Make .element-invisible work for focusable elements like links landed 3 weeks ago, so I think some improvements should still be on the table for D7 here. Bartik, Seven, and Garland all still have a bunch of CSS rules that start with #skip-link, and within html.tpl.php, neither the "skip-link" DIV nor the anchor tag inside it have the "element-invisible" or "element-focusable" CSS class, despite that other issue that landed recently. Question for the CSS gurus here: are we still making it too hard on contrib themes to implement the skip link accessibly, and if so, can we use the new "element-focusable" class to make it easier?

As far as whether we should support "always visible", "always invisible", or "visible on focus", according to #896364-217: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay, the middle option is probably poor practice, so I don't think we should have it. But I do think we should make it possible for a theme to toggle between "always visible" and "visible on focus" with either a simple CSS rule, or a 1 line THEME_preprocess_html() function that sets an appropriate 'classes' variables.

I'm considering this "major" unless the CSS gurus out there respond saying that the CSS structure already in HEAD is such that we're not foreseeing that more than half of contrib themes will fail to implement the "Skip to main content" link accessibly, in which case, this can be downgraded to "normal".

[EDIT: making it easy to toggle "always visible" and "visible on focus" is less of a priority, and is just a nice to have if it comes cheap from fixing the main issue, which is to make a "visible on focus" skip link reasonably easy to implement for a contrib theme.]

I'm escalating this to critical, because of its relationship to two other critical issues: #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay and #841184: "Skip to main content" link doesn't work correctly in the overlay. We need our CSS experts looking at these issues in relationship to each other, so I think having this in the critical queue will assist with that. I'm fine if after this gets looked at, if it gets demoted to "major" or bumped to D8; I just want to make sure it gets adequately looked at first.

@Jeff Burnz: Thank you for the work you've already contributed to these issues, and if you can share more wisdom, that would be awesome. But I'm guessing you've been swamped with other work, so I'm hoping we can bring in more eyes from the community on this.

For anyone new to this issue, a couple summaries are on #29 and #40, but basically, it boils down to: if we're going to have 3 "visible on focus" links (the "skip to main content" one, and the two being added in #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay), and if we're learning all sorts of complicated stuff about how to make these properly accessible for all screen readers, and for keyboard users, let's make sure as much of the magic as possible is captured in our system CSS files, and not in our theme CSS; otherwise, people will all too easily fail to implement these accessibly in contrib and custom themes.

.

@Jeff wrote:

2. Therefore we have a dilemma - showing skip nav by default is considered a best practice, but element-invisible will hide it by default (in Stark). For many reasons I think this would actually be better. In my experience themers resort to inaccessible techniques such as display: none; purely out of ignorance.

I am quite content either way. But, it is nice to show people the 'right way' to hide the link. Since it is more desirable for them to learn the right way than to use display: none or remove the markup altogether.

Duplicate post.

I'm a bit late to the party on this, but in response to the issues in #44

1. I think adding the classes in core's html.tpl.php is fine

2. I don't think this is a problem at all. If the link is visible on :focus then screen readers and non-mouse users should be able to find it. It also has the added benefit that you mentions of showing to themer's an example of how to hide it properly in their own themes.

3. I'm a proponent of less being more so I would avoid adding any more styling here.

#4: toggle-skip-link_639460_v2.patch queued for re-testing.

I agree with #47. Here's a patch to add the classes in a way that still lets themes easily remove those classes if they want to implement the link as always visible. Please add to this patch any changes to CSS files that this allows (i.e., does this allow us to remove anything from core theme css files?).

subscribing

I think the patch in #49 is a good way of doing this. What else needs to be done for this to be marked fixed? Do all the core themes need to be reviewed to bring them into conformity with this approach for html.tpl.php?

I agree the patch in #49 is a great way of doing this too, but I think the following still needs to happen:

1. $skip_link_classes needs documentation in html.tpl.php: I think the classes should be mentioned in the docs, and the default behavior explained.
2. Since showing this link is a best practice, it should be noted that the $skip_link_classes variable can simply be omitted if the themer needs the link to display by default.
3. This is sorta not directly related, but I think it should also be noted that <a href="#main-content"> refers to the <div id="main-content"> which lives in page.tpl.php. This file is one of the first things most themers change, and since this link cannot be in page.tpl.php, we run a big risk of having this link display properly, but be totally broken because the theme didn't use <div id="main-content">. This is probably better as a comment in page.tpl.php I guess.

Basically, I think we need to go a little out of our way to educate themers on this, to ensure this actually remains in most themes, and functions properly.

Do all the core themes need to be reviewed to bring them into conformity with this approach for html.tpl.php?

Yes. I'm going to take a stab at this, and the docs now.

Ok, here's a patch and a bunch of screenshots. Note that $skip_link_classes are on the <a>. Previous patch had them on the <div>, which was wrong because <div>'s are not focusable.

Oh, and disregard #53-3. Not sure why I was thinking a div was used for that, but it's an anchor so it's all good.

Looks good to me on visual inspection, but I am not familiar enough with the CSS of the themes to give a good review to the patch.

Ok, I've added it here: http://drupal7.dev.openconcept.ca/

Cleared the cache. Still looks fine. CSS seems simpler and seems to work fine. I've tested this in Garland, Seven & Bartik.

What do we need to get this marked RTBC? Do we need a CSS review? The consolidation looks good.

Thanks Jacine, mgifford, and Jeff! Great to see the CSS masters reviewing this issue! These CSS changes are exactly what I was hoping for: to me, the new CSS looks like something that won't scare away contrib theme developers or cause them to remove the link entirely. In #841184-44: "Skip to main content" link doesn't work correctly in the overlay, I asked to get some people who've been testing that issue to test this one too, since both issues require the same cross-browser evaluation. Assuming that testing confirms that there isn't a regression, I'm +1 for this being RTBC.

This patch only changes the documentation in html.tpl.php. Please feel free to post feedback on it.

You know... On second thought maybe it's best to forget about the classes array for this. It forces us to have to explain it in an unusual way, and telling themers to create a preprocess function just to follow what's considered a best practice is pretty risk and probably counter-productive. 99% of the time I'd be all for moving classes to arrays in preprocess functions, but when it comes to these one-off situations, maybe it'd best that we didn't overcomplicate things. That's my 2 cents, but I'd be ok with whatever you guys decide.

BTW, I tested this in IE6, IE7, IE8, Firefox (2, 3.6 & 4), Safari, Chrome and Opera before posting the patch. I don't have IE9 installed.

The only difference from the screenshots is in IE6 for Garland because IE6 (a) ignores !important and (b) doesn't support "fixed". The attached screenshot shows the difference for IE6. IE7 has bugs with position:fixed as well, and apparently there are some z-index issues as IE7 doesn't show the skip link at all. IMO, we should just do Garland like the others and forget about position:fixed, rather than wrestle with IE for this. Thoughts?

Note about testing: I have been unable to test this in Firefox and Opera by tabbing. I'm not finding any settings that will allow me to do this, so the way I tested this in those browsers was by including this code in page.tpl.php:

<script type="text/javascript" charset="utf-8">
  jQuery(document).ready(function($) {
    $('#skip-link a').focus();
  });
</script>

On second thought maybe it's best to forget about the classes array for this...

The documentation in html.tpl.php might be too much, or just need improvement, and it's true that usually where we use $classes and $classes_array, it's for semantic classes, not for behavioral classes, and both of those are good reasons to question the idea of a $skip_link_classes/$skip_link_classes_array approach, and instead, hard-code class="element-invisible element-focusable" into html.tpl.php. But, my thinking behind using the variables is that:

1) If there's no other reason for a theme to override html.tpl.php, then I don't like the idea of it being overridden just to make that link always visible.

2) I'm envisioning a contrib module (whether stand-alone, or part of Edge) that adds a checkbox to the theme settings form for making the link always visible, as per #40 and earlier discussion. But, we can't add documentation to html.tpl.php about a contrib module, especially one that doesn't exist yet. Of course, if Dries or webchick give the go-ahead to add that checkbox in core, I'm happy to submit the patch to do that. It's very easy from a code standpoint, the hardest part will be reaching consensus on the text.

I did some cross browser testing for this using the browser's listed in #59

IE 6
IE 7 - FAIL (See Garland screen)
IE 8
IE 9 beta -
FF 3.6
FF 4 beta 6
Opera 10.x
Chrome 5+
Safari 5+

In response to testing difficulties in #61, I only had trouble tabbing in Opera.

See this note about firefox: http://support.mozilla.com/en-US/kb/Pressing+Tab+key+does+not+select+men...

@effulgentsia Yeah. TBH, I don't think the whole separation of page.tpl.php and html.tpl.php makes a lot of sense when this stuff is in html.tpl.php, and I think it complicates these sorts of issues much more than they need to be. I would have preferred everything that is inside the <body> be inside page.tpl.php, but that's moot at this point. ;)

I would definitely prefer a theme setting for this, but I think the chances of that happening are next to none, so we should probably stick with the proposed implementation work on improving the documentation.

+ * - $skip_link_classes: String of classes that can be used to style the "Skip
+ *   to main content" link. This link enables screen reader users to instruct
+ *   the screen reader software to skip reading block content and go straight to
+ *   the main content. The classes can be manipulated through the variable

Technically these classes are not meant to be "styled" against at all. Styling should happen using the #skip-link id. .element-invisible and .element-focusable are strictly for behavior purposes only. It's also not only for screen reader users. Any capable browser with keyboard navigation enabled will be able to see and use this link to skip to the main content area.

+ *   To make this link visible to all users, implement a preprocess function to
+ *   remove these classes from the 'skip_link_classes_array' variable, or
+ *   enable a module that does this.

I don't think we should point people to enable any module for this in core documentation, as it's also likely that themes will offer a theme setting for it. Maybe we should leave this out instead of potentially confusing things further and document this in a handbook page?

Setting to needs work for documentation, and the fact that we'll need to figure out what to do about Garland since it's broken in IE7.

@dcrocks Holy crap, thank you :D That works!!

I've rerolled the patch with a fix for IE7's z-index issue. I've already tested this across the browsers I listed in my last comment, barring IE6, so I'm rather confident it doesn't have any seriously negative repercussions. A real test would be great if anyone is still setup to do so. I still need to setup a proper IE6 VM image so I can swap and test IE7 and IE6 without removing and reinstalling IE7.

I also took the liberty of adding outline: 0; to Seven as mentioned in #59. I also found the outline very distracting on the browsers that support border radius.

@james.elliot I can't seem to apply this correctly. It stops at Seven's CSS file and gives me this complaint.

patching file includes/theme.inc
patching file modules/system/html.tpl.php
patching file themes/bartik/css/style.css
patching file themes/garland/style.css
patching file themes/seven/style.css
patch: **** malformed patch at line 190: -moz-border-radius: 0 0 10px 10px;

Is there any chance you can re-roll? I'd be happy to test :D

I tested #60 with the patch from #841184-41: "Skip to main content" link doesn't work correctly in the overlay, and the only problem I found was that in IE, the dummy element introduced in #841184 needs outline: 0. It looks like #67 will fix that (when it gets rerolled). So unless there are objections to outline: 0, let's leave it in.

New and improved. Sorry it took so long. Damned be the line endings in Windows ruining my patches!

A plea to the test gods

@james.elliot Thanks! Nice job. :)

The outline you added needs to be on :hover, :focus, :active, not :link, :visited, so this patch changes that, and also makes the outline: 0 styles consistent across themes.

I think this in pretty good shape from a code standpoint, we just need more testing and to tweak the docs a bit as mentioned in #65. Anyone willing to help with that?

This has no change relative to #71 with respect to the style.css files.

Based on Jacine's feedback, this takes away the use of variables for html.tpl.php, and simply hard-codes the classes. This is more consistent with the rest of core templates, where variable classes are for semantics, and classes like 'clearfix' are applied in the template file. Also, webchick made it clear to me in IRC that it is not D7 core's job to deal with the "always visible" use-case (we will revisit this in D8, since it is considered best practice to make it always visible).

This doesn't make it impossible for theme developers to make the link always visible. They can override the template file and remove the classes. This isn't documented in this patch, but also, it's not like we have explicit documentation letting people know they can override a template file and remove a "clearfix" class. I think the assumption is that theme developers can read markup and figure those kinds of things out for themselves. But, if we think documentation is needed here, I'm fine with us adding some.

This also doesn't make it impossible for a base theme or contrib module to create an "always visible" theme setting. That base theme or contrib module can ship with an html.tpl.php file that uses variables.

#74 was x-post, so this merges my changes with #73.

Ok, that sounds fine. I'm happy with this patch. The docs would be better in the theming guide anyway. That would be much cleaner and can cover all the use cases in one place.

We need screen reader tests per Jeff on #59:

- Jaws 10 (9 also?)
- Windows Eyes
- NVDA
- Apple VO

:)

@Jeff Burnz I agree it's pretty ugly, but don't have a preference either way. I think it's up to you. The other technique does have cleaner code IMO and you are the maintainer, so I'd say go for it (quickly, while we wait for screen reader tests, so we don't end up holding things up) if you want to.

@Jeff Burnz, I think we need to go darker, like Bartik. Check out the screenshots.

Ok, so the patch in #81 was giving me the #skip-link behind the navigation. I added a z-index to #skip-link and it's fine now. Patch for that is attached. All is well, and it looks much better in Garland. :)

I re-tested in: IE6, IE7, IE8, Firefox (2, 3.6 & 4), Safari, Chrome and Opera.

Thanks all! I haven't done cross-browser testing on this myself, but from reading the comments above, I feel satisfied that this has been well tested. Therefore, RTBC.

Summary: this patch does two things:

1) Brings Garland's styling of the skip link up to date with standards that we used for Seven and Bartik (see #77) (side note: reading #78, Jeff: want to be added to the MAINTAINERS.txt file for Garland?)

2) Removes cruft from Seven's and Bartik's styling of the skip link, that was either not necessary to begin with, or became unnecessary once #897638: Make .element-invisible work for focusable elements like links was resolved, giving us proper encapsulation of the "visible on focus" behavior.

As a (IMO: good) side-effect of item 2, the skip link has visible on focus, rather than always visible, behavior on the Stark theme as well.

As another (IMO: good) side-effect of item 2, you can now override html.tpl.php, remove the classes on the skip link, and viola, the link is always visible on all 4 core themes without needing to make a single CSS change.

Looking over the new CSS in Garland, Seven, and Bartik, I feel much better that contrib theme developers won't run away in fear, trying to figure out how to style the skip link for their theme (or give up on it, and simply display:none it, which was one of Jeff's initial concerns at the top of this issue). All of the theme CSS is relatively easy to understand (even for me, a total CSS amateur), and is entirely about making it look right. No more magic top:-10000px. No more magic height:1px or overflow:.

And proving that we can do it here, makes me feel much more confident that we'll be able to do the same for similar links being added in #896364: Screen reader users and some keyboard only users need a clear, quick way to disable the overlay.

As per #74, we're not using a variable or theme setting to make it completely easy to remove the classes being added in html.tpl.php, but overriding a template file isn't hard, and there will surely be a contrib module and/or base theme that makes it even easier by exposing a setting.

In #59, Jeff suggested this should be tested in screen readers, and while that wouldn't hurt, I don't think it needs to hold this issue up. I don't see why any of the CSS changes here would affect a screen reader.

I agree this is good to go. CSS changes can affect screenreaders, but these shouldn't. Just to make sure, I did a reality check using NVDA and the latest JAWS, and everything was fine. I also tested in JAWS and NVDA with the latest skip-link patch at #841184: "Skip to main content" link doesn't work correctly in the overlay applied on top of this. Everything is fine there with both screenreaders, and the IE problem I mentioned in #70 is fixed.

I applied this to my sandbox http://drupal7.dev.openconcept.ca/

Looks fine to me. I haven't tested it with VoiceOver, but don't expect this change will cause a problem.

I'm happy for the consolidation and glad this is marked RTBC.

Great -- like it. Committed to CVS HEAD. Thanks.