The 'Search this site' form should be preceded by a header for accessibility. D7 has the search_theme_form enabled by default in the left sidebar but with no header. The block version of the search form, search_block_form, already gets a visible h2 header called "Search". But the block one is not enabled by default. The search_theme_form is the one used by default and it is less accessible.

The only way for a screenreader user to find the search_theme_form is to find a text field and submit button. It is better to provide an h2 header so screenreader users could jump to or skip over the site search just like they do with the navigation bars. Although it is not required by WCAG 2.0, the site search form should have a preceding h2 header because it is a key navigational tool just like navigation bars. Many sites add such a header above search as a best practice.

ARIA also provides a landmark role for search.

References:
* http://www.w3.org/TR/UNDERSTANDING-WCAG20/navigation-mechanisms-skip.html
* http://www.w3.org/TR/2008/WD-WCAG20-TECHS-20080430/H42.html

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bowersox’s picture

Status: Active » Needs review
FileSize
798 bytes

Please review the patch attached.

In search.module search_box():

+  if ($form_id == 'search_theme_form') {
+    $form[$form_id]['#prefix'] = '<h2 class="element-invisible">' . t('Search') . '</h2>';
+  }

This code adds an invisible h2 preceding the text field and submit button. I chose to use the invisible header so there is no impact on the visual display of D7 and Garland out of the box. I chose to make the text just "Search" because "Search this site" is the label on the form element that users will find once they navigate to this form.

I'm curious to gather feedback.

Everett Zufelt’s picture

+1

The search form / block is definitely a navigational block that would benefit from being proceeded by a heading. I would normally caution against the use of .element-invisible, but this looks to me to be a reasonable use (1. the test "search" would be visually repeated if not hidden).

mgifford’s picture

+1

That's a nice simple solution. Great to have the element-invisible class in core!

johnbarclay’s picture

Thanks. This is a good approach. This will save me a lot of work. I always have to theme this in to pass accessibility tests on sites I would normally not theme.

Everett Zufelt’s picture

Status: Needs review » Reviewed & tested by the community

This seems like a trivial addition to core, but it will be a noticable improvement in accessibility. Adding a heading before the search form will make the search far easier to find for screen-reader users, particularly when on a page with other form elements.

Dries’s picture

Status: Reviewed & tested by the community » Needs work

Let's add a code comment explaining why this is needed, with a reference to the official guidelines and recommendations. Thanks.

bowersox’s picture

@Dries

Thanks for the feedback. I've added a comment with explanation and references. Please let me know if this is what you wanted or if there's anything else I can do. Thanks!

bowersox’s picture

Status: Needs work » Needs review

This is ready for review. Let me know if there are any other improvements so we can make Drupal 7 the most accessible CMS.

Everett Zufelt’s picture

Version: 7.x-dev » 6.x-dev
Component: search.module » simpletest.module

@brandonojc

Consider using H42: Using h1-h6 to identify headings | Techniques for WCAG 2.0 as the resource in the docs

Everett Zufelt’s picture

Version: 6.x-dev » 7.x-dev
Component: simpletest.module » search.module

No idea how the changes in te above comment happened.

bowersox’s picture

Can we get some more feedback before RTBC? Lots of people +1'd the idea and the approach, but review and testing of the patch would be great.

Status: Needs review » Needs work

The last submitted patch failed testing.

bowersox’s picture

Status: Needs work » Needs review

Let's try testing again. The patch still applies and passes the 'Drupal error handlers' tests without problems.

Everett Zufelt’s picture

Wondering if this can wait for after code freeze? Looked at the patch and it looks good to me, although the language in the comment could be cleaned up a bit. I would also recommend that where possible accessibility comments reference only w3.org as W3C is the authoritative body for accessibility.

mgifford’s picture

I think the approach is fine. I applied it here and seems to work well - http://drupal7.dev.openconcept.ca

I think that in general with the accessibility comments we should be putting them into a web page and putting a link into the code for the details. Inline code should be brief in my opinion. Seems that's not the direction that the community is going, so maybe just editing down so that it is shorter...

+ // Add an h2 heading to search_theme_form, not search_block_form which already
+ // get an h2 in block.tpl.php. Headings allow screen-reader and keyboard only
+ // users to navigate to or skip menus and content sections. This heading is
+ // invisible because the search form is visually apparent to sighted users.
+ // See http://www.webaim.org/techniques/css/invisiblecontent/ for information.
+ // Level h2 is appropriate (as opposed to h3 or h4) because, like navigation
+ // menus, the search often precedes or follows the page title or site title h1.
+ // See http://www.w3.org/TR/WCAG-TECHS/H69.html and H42.html for more info.

And although I think the code is RTBC, I do hope that this is something we can get in afterwards as a usability enhancement, but if we can get this done RTBC we don't have to gamble on that being the case.
http://openconcept.ca/blog/everett/final_stretch_help_needed_for_drupal_...

Everett Zufelt’s picture

@brandon

Tested this out and it is working well. Would agree that the comments could be shortened.

bowersox’s picture

Since Dries asked for a code comment I'll try to strike a balance and shorten it. I agree that short code comments with good online references are best. The testing bot seems to not be re-testing this so I will reroll. The code itself is truly RTBC so I hope we can simply get this done before Sept 1.

mgifford’s picture

Totally agree Brandon!

I'm willing to RTBC it with the shorter docs. The code has been reviewed and it's a very simple improvement to the interface!

Mike

bowersox’s picture

Please review this patch. The code is the same with more compact comments that reference our new Accessibility info in the Theming Guide. This should be ready for RTBC.

Cliff’s picture

+1.

Labeling the search form so people using screen readers can find it is basic accessibility. It's also overlooked on virtually every site I have tested.

Why is this patch so important? If you can see, imagine that every room you enter has no windows and has the light switch well hidden in random locations. You would have to stumble around each room guessing at its possible location, unable to find anything simply because the one tool that would shed light on the subject was not consistently placed and labeled.

I imagine that's what it's like to be blind and have to find the search form on most Web sites. The text input field is never labeled in a way that screen readers can recognize it. So if I were blind I would have to assume that the site search, if there is one, is the first form field on each page. And in the page layout, it might be — but that doesn't mean it's always first in the linearized code.

Clearly labeling the site search is not just basic accessibility, it's a basic courtesy. I see Drupal as a tool for a better Web, and putting this patch in core would be a giant stride in that direction.

Everett Zufelt’s picture

I think this is ready for RTBC, I only have two final questions.

1. Should the heading perhaps be "Search Form" and not just search? This is a bit nit picky.

2. It may be nice to provide a link in the comments to the W3C technique for using headings found in comment #9 above.

bowersox’s picture

@Everett

1. I would be happy to resubmit the patch with 'Search Form' if other people think that is better. Either way I would like to see this one committed in time.

2. The W3C page is a great reference. It is currently the first reference in the "Further reading about headings" section of the new Heading Accessibility Theme Guide page which is the link included in the comments of this patch. So basically if people follow our link they will get a whole slew of resources, including that W3C one.

Thanks again for all the feedback!

Everett Zufelt’s picture

@brandon

My only reason for using "Search form" is that it can be confusing to have multiple headings with the same text and Search Form is a little more specific, and less likely to be duplicated for any reason than "Search". If we can get this rerolled with Search Form I think we can set it to RTBC. It could even be RTBC without, but I think that this is a good idea.

Thanks for the info about the pages and the w3c link, sounds good.

mgifford’s picture

I've modified this form to say "Search form" and resubmitted it. Think it's RTBC now.

Everett Zufelt’s picture

Status: Needs review » Reviewed & tested by the community

This is great, it was already ready for RTBC and now is finalized.

This patch simply adds a heading level 2 "Search form" to the search form. The heading is styled with class="element-invisible".

bowersox’s picture

I agree. RTBC. Thanks!

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Hm. I don't really like this fix.

If we're only trying to affect the themed search form, then why not add this heading to search-block-form.tpl.php?

Cliff’s picture

Angie, I know a lot about accessibility but not as much as I would like to about Drupal. Is search-block-form.tpl.php in core, where it will affect all Drupal installations, or unique to each theme or, perhaps, module?

Labeling the search form is a fundamental and too-often-missed aspect of accessibility. For the many who must comply with WCAG (or something close to it) by law, it's important to know that this is a feature that is native to Drupal, not something that a themer can choose to use or not to use. So if that would make a difference in whether the patch goes where they've built it or where you suggest, please reconsider.

But I see a new wrinkle in this, too. At least I do if I, as a relative newbie to the back end of Drupal, am understanding references to "themed search form" and "block search form."

If these terms refer to the two search forms sometimes seen on d.o — the form that searches all of d.o and appears at upper right and then a second form that searches, say, all issues or all modules and appears in that respective block — then we have a labeling problem. If there can be two search forms on one page, then the label for each needs to indicate exactly what that form searches. So the main form could be labeled "Search this site," but the secondary form needs something like "Search only this subject." It would be ideal if site developers could customize those generic labels to something more specific, such as "Search drupal.org" and "Search all issues" or "Search for modules." This additional detail is somewhat available to the sighted from the page design, but people relying on screen readers don't have that contextual help as they scan the page for text-input fields. (Many do that simply because sites in general fail to label the search form, so they will just try whatever text-input fields they find in the order in which the fields occur in the html until they get one that seems to search the site.)

webchick’s picture

Hi, Cliff!

search-block-form.tpl.php is indeed in core, in modules/search. This is the file used by all Drupal 6+ sites to output this form unless it is explicitly overridden in someone's theme.

The best practice in Drupal is to move all XHTML markup to a place where it is "themeable". For example, it's possible that with certain layouts and configurations, h3 is actually the best tag here, rather than h2. It's also possible that someone wants to change the title of this from "Search form" to "Site-wide search form" or similar. The current patch encodes this information in the form definition, which would require a themer to have to call in a module developer wielding hook_form_alter() to adjust. This is therefore a bug with the current implemetation.

There are ways for a Drupal site to avoid getting this new code despite our best efforts, but this is true regardless of how we implement it; Drupal is a flexible system that is infinitely overridable and customizable. So let's implement it in the standard way we implement other markup in Drupal, and document this change in the upgrade guide so that people porting their D6 themes who may have overridden this template will get the new markup.

The extra search box on d.o is custom and not part of core, so we don't need to worry about adjusting for it. Search module provides only one "global" search box, it just can be displayed either in a sidebar block or as part of the theme, typically in the header. The latter is the usage what we're special-casing.

Hope that helps!

Cliff’s picture

That does. Thanks! So, bottom line, our valiant coders need to:

  1. Move the patch to search-block-form.tpl.php (which I realize is probably not like moving a sentence from paragraph 6 to paragraph 8).
  2. Preferably, change the default label to something like "Sitewide Search" or "Search Our Site."

Yeah, built-in accessibility is never undoable. By making sure that it's there without a user's having to build it in, we are taking care of the concerns of the folks who need to comply with ADA or who just want to comply with WCAG (which should be pretty much everybody, right?).

;-)

Everett Zufelt’s picture

@Webchick

The current search-block-form.tpl.php contains:

<div class="container-inline">
  <?php print $search_form; ?>
</div>

Are you suggesting a change like...

<div class="container-inline">
  <h2 class="element-invisible"><?php print t('Search form'); ?> </h2>
  <?php print $search_form; ?>
</div>
bowersox’s picture

@ Cliff

Regarding the name, after discussion I was leaning towards "Search" rather than "Search this site" because the text "Search this site" is the actual label on the input field. That appears right after the heading. And since this heading and form are usually available on every single page of the site, we felt it would be clear without making a longer heading title that could clutter up the ability to quickly scan headings. Do you think that is wise or should we consider the full label?

webchick’s picture

@Everett: That's correct (except you have an extra space after your ?> ;)). It's a bummer to lose all of that nice documentation in core, but let's make sure it's replicated in the handbook where we talk about best practices for module developers doing accessible markup.

Everett Zufelt’s picture

Status: Needs work » Needs review
FileSize
573 bytes

Rerolled patch to apply heading in modules/search/search-block-form.tpl.php as per @webchick in #27.

mgifford’s picture

Think we need both patch #34 & #24 in core. Patch #34 works if you add the search element as a block. Patch #24 works if you add the search element through the theme config.

I've messed with the output a bit to show a difference, but I've applied it here http://drupal7.dev.openconcept.ca/

Both still apply nicely to core.

bowersox’s picture

Wait, it's only the search-theme-form that needs a heading, not search-block-form. Search-block-form already has a heading applied by the block theming (every block has a h2 heading from its title by default). I think we got confused and search-block-form.tpl.php is not the place that needs the heading at all.

Is there another .tpl.php file that applies to the search-theme-form?

Everett Zufelt’s picture

@brandon

You are correct, I missed that when rolling the last patch.

The following patch adds the heading to modules/search/search-theme-form.tpl.php

mgifford’s picture

Ok, this applies well. We just need #37 then. This is now RTBC.

Cliff’s picture

@brandonojc:

If the screen reader will announce "Search; Search this site" as it reads linearly through the content, that's great.

And if I would hear "Search this site" on asking my screen reader to just announce the first form, that's great, too.

So whatever achieves that result is what we want. (Sorry if I misunderstood earlier.)

Owen Barton’s picture

Status: Needs review » Reviewed & tested by the community

Looks like this is all ready to go - code looks good, and Mike has tested the html is appearing in the right place.

bowersox’s picture

+1 for the RTBC patch in 37. I reviewed and tested in a variety of browsers and with all the core themes. The invisible 'Search form' heading is properly added to search-theme-form, and there is no impact on search-block-form. There is also no visual change with any theme's appearance.

Thanks, webchick, for the cleaner approach, and Everett, Mike, Owen and Cliff for the fast help.

Status: Reviewed & tested by the community » Needs work

The last submitted patch failed testing.

mgifford’s picture

I hate it when things go missing like this. So what happened to:
modules/search/search-theme-form.tpl.php

But maybe it doesn't matter any more. I was unable to add the search form from the theme in my testing.

If this feature's been eliminated than this issue is closed.

Dave Reid’s picture

Dave Reid’s picture

This would just need to be added to search-block-form.tpl.php instead.

Heine’s picture

Everett Zufelt’s picture

Status: Needs work » Closed (won't fix)

theme search form has been removed from d7. As long as block search form still has a heading we can set this to won't fix.

YaxBalamAhaw’s picture

Status: Closed (won't fix) » Active

By default, the search block doesn't have a block title, so there is no heading. So, would this be considered an issue now?

mgifford’s picture

Status: Active » Needs review
FileSize
573 bytes

There is no heading above the search block unless it is added to the search template.

I've tested this here - http://drupal7.dev.openconcept.ca/

I do think it is an issue again.

jhodgdon’s picture

Status: Needs review » Active

That is a good point:
http://api.drupal.org/api/function/search_block_view/7
returns just content, not a title for the block.

The form itself doesn't have any headers:
http://api.drupal.org/api/function/search_box/7
(search_box is the form generator function for drupal_get_form('search_block_form'), as per search_forms().)

And the theme function for the form also doesn't have any headers:
http://api.drupal.org/api/drupal/modules--search--search-block-form.tpl....

So I guess we are back to square one - and we need to decide where to put this.

Also, when you are on the Search page, does the search form there need a header? It doesn't look like there is one:
http://api.drupal.org/api/function/search_view/7

mgifford’s picture

It's getting back to @webchick's comments in #27 http://drupal.org/node/545610#comment-2029490

That was basically RTBC when search-theme-form.tpl.php existed.

Looking at the search page in my sandbox:
http://drupal7.dev.openconcept.ca/?q=search

I can't see why we'd need/want to insert an invisible header on that page. There's a heading for Search as it is..

jhodgdon’s picture

Status: Active » Needs review

Your sandbox link gives "access denied" to me, so that wasn't very helpful... but you are right, the Search page should have an H1 header on it saying "Search" (as the page title) anyway, and this is generally right before the search box, so that is probably sufficient.

So webchick's suggestion is to add it to the block form tpl.php file. Oh I see, mgifford and I cross-posted -- mgifford has a patch that I think looks correct, unless the testing bot complains.

I would like the opinion of an accessibility expert before I mark it RTBC though. (maybe mgifford you are one? sorry for not knowing)

mgifford’s picture

Damn permissions. I didn't check that before sending out the link, sorry. I've corrected them. I've had to keep blowing away the database and re-populating it unfortunately.

I do consider myself an accessibility expert, certainly with Drupal. However, I'd like to get Everett to look at it before had as he marked it as Won't Fix back in September (#47).

bowersox’s picture

+1 for this patch. And thanks to @YaxBalamAhaw for catching the need to re-open this issue.

I do have one suggestion for improvement. The current patch will add an invisible heading always. It will even add a heading if the Search block adds on its own heading. That happens if the user decides to set a block title in the block configuration (/admin/structure/block/manage/search/form/configure). That situation is bad because the block then has 2 headings.

Here is sample code if we want this patch to be smart and try to check for a block title to prevent a double title:

  <?php if (empty($variables['form']['#block']->title)) {
    print '<h2 class="element-invisible">' . t('Search form') . '</h2>';
  } ?>

This code would be added to search-block-form.tpl.php the same place as the current patch.

mgifford’s picture

I like this enhancement Brandon. Can you roll up a patch for it?

bowersox’s picture

FileSize
655 bytes

Please review this updated patch. It adds the same invisible heading as before, but it prevents that if the search block has a title. The title will be the visible H2 heading so we no longer need the invisible off-screen redundant heading. See comment #54 for further explanation.

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

I applied this patch and it works great in my sandbox: http://drupal7.dev.openconcept.ca/

The code looks clean & simple so I'm going to RTBC it.

@jhodgdon any other thoughts on this? I think we've addressed your concerns.

jhodgdon’s picture

mgifford: no other concerns, if the patch is working for you and addresses your accessibility concerns. It looks perfectly reasonable to me, as long as it works (only displays the new H2 if there is no block title).

mgifford’s picture

Yup, Just confirmed it acts as you've specified. Thanks!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)
Issue tags: -Accessibility

Automatically closed -- issue fixed for 2 weeks with no activity.