Problem/Motivation

The status message area (#console .messages) is not identified in a way that makes it discoverable by assistive technology agents.

We learned during the accessibility studies at DrupalCamp Twin Cities that screen reader users are unaware of the information/status/error messages on the screen.

Proposed resolution

Indicate the messages error through an ARIA landmark. Announce error messages so that a user isn't required to find the messages error in situation (like an invalid form element) when remediation is required to complete a task.

Remaining tasks

Propose a patch.

User interface changes

Screen reader users will be aware that a message area exists on the page.

API changes

None.

#1797438: HTML5 validation is preventing form submit and not fully accessible

Original report by jessebeach

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jessebeach’s picture

Status: Active » Needs review
FileSize
1.85 KB

In this patch, an ARIA role of contentinfo is added to the messages wrapper. The hidden h2 title in the area is moved to an ARIA label to reduce the header clutter on the page.

In the case of an error, the message is wrapped in an inner div with the role alert. This will cause, in supporting user agents, the error message to be read to the user with an aria-live urgency of assertive.

With these changes, the status message becomes a landmark equal to navigation, main, complementary and form elements.

mparker17’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good.

The "Your version of Drupal is no longer supported" error message shows up as a landmark in Voiceover and JAWS, and is labelled as an error message in both browsers.

The "Article x has been created" status message shows up as a landmark in Voiceover and JAWS and is labelled as a status message in both browsers.

mparker17’s picture

I can't easily get a warning message to show up... there aren't many of them in Drupal Core and I can't seem to get it into a state where one would appear :S

Further, I can't seem to get the devel module to install on simplytest.me

jessebeach’s picture

You should get a warning if you rearrange a block or the order of fields in a node display.

mparker17’s picture

Ah, right! I'll test that immediately.

mparker17’s picture

When I rearrange the order of blocks and fields, I do get a warning message.

The warning message works perfectly in JAWS (as soon as I rearrange the blocks, JAWS announces "Alert: Star You have unsaved changes.").

However, Voiceover (in Chrome and Safari) doesn't seem to notice that the message has been added to the page. I suspect this is unrelated to this issue.

On the blocks and fields page, the warning message isn't marked as a landmark, so it doesn't show up in the web rotor (Voiceover) or region navigator (JAWS). This is unrelated to this issue.

Leaving as RTBC.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/includes/theme.incundefined
@@ -1629,10 +1629,14 @@ function theme_status_messages($variables) {
-    if (!empty($status_heading[$type])) {
-      $output .= '<h2 class="visually-hidden">' . $status_heading[$type] . "</h2>\n";
+    $output .= "<div class=\"messages messages--$type\" role=\"contentinfo\" aria-label=\"{$status_heading[$type]}\">\n";

It looks as the previous code ensured that $status_heading[$type] had a value... this suggests that we could end up with the following markup <div class="messages messages--someweirdtype" role="contentinfo" aria-label="">

jessebeach’s picture

Status: Needs work » Needs review
Issue tags: +sprint, +Spark
FileSize
2.04 KB

Fair point. An empty aria-label is not necessarily a bad thing, but we can avoid it easily. Rerolled with the original if statement back in place.

mgifford’s picture

There's an ongoing discussion at the moment as to whether <h2>'s can be removed in favour of landmarks at this stage. I'm going to post the summary shortly, but wanted to note that adding ARIA doesn't mean we can remove headings at this stage.

EDIT The summary of the twitter discussion on ARIA vs Headers - #2045039-6: Cleanup HTML heading structure

falcon03’s picture

Thank everyone for working on this. Please, as @mgifford said, let's add the aria-label, but let's not remove the heading.

mgifford’s picture

Patch with heading restored.

Wim Leers’s picture

mgifford’s picture

Glad to see it still passes the tests.

What do we need to do to get this RTBC?

Wim Leers’s picture

I think we need somebody with an accessibility background to RTBC it. Like you :)

Wim Leers’s picture

Status: Needs review » Needs work
Issue tags: +Accessibility, +sprint, +Spark, +TwinCities

The last submitted patch, landmark-status-message-2047175-11.patch, failed testing.

falcon03’s picture

Disclaimer -- I've started getting my hands dirty with the theming system only a few days ago, so forgive me if this question is stupid...

I'm wondering -- isn't there a way to add aria-related attributes to #attributes so that we can consolidate the two conditional cheks into a single one and avoid the extra

?

I see that theme_status_messages() is not converted to twig yet, so maybe this is something that we can look at after the conversion happens...

I'm just raising this point for personal curiosity -- this is not a blocker for this patch to get in for me.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.39 KB

Drat.. function theme_status_messages has been moved to twig. All for the best, but have to rewrite this patch.

I don't feel I inserted this right:
<div role="alert">

Status: Needs review » Needs work

The last submitted patch, landmark-status-message-2047175-17.patch, failed testing.

Wim Leers’s picture

Looks good to me. I just have one question: why do you need a separate <div> for the role="alert"? Why can't it be added to the <div class="messages" …> element?

Wim Leers’s picture

Status: Needs work » Needs review
Issue tags: -Accessibility, -sprint, -Spark, -TwinCities

Status: Needs review » Needs work

The last submitted patch, landmark-status-message-2047175-17.patch, failed testing.

mgifford’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +Accessibility, +sprint, +Spark, +TwinCities

The last submitted patch, landmark-status-message-2047175-17.patch, failed testing.

mgifford’s picture

$ git apply landmark-status-message-2047175-17.patch

Works nicely locally. Not sure re-testing it will help anything here though.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.39 KB

Status: Needs review » Needs work

The last submitted patch, landmark-status-message-2047175-26.patch, failed testing.

Wim Leers’s picture

Views test failure; definitely unrelated to the changes in the patch.

Wim Leers’s picture

Status: Needs work » Needs review
Issue tags: -Accessibility, -sprint, -Spark, -TwinCities

Status: Needs review » Needs work

The last submitted patch, landmark-status-message-2047175-26.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review
Issue tags: +Accessibility, +sprint, +Spark, +TwinCities
mgifford’s picture

Glad the bots like it today!

So what else do we need to do to mark this RTBC?

Wim Leers’s picture

Nothing! Just my one question from #20:

Looks good to me. I just have one question: why do you need a separate <div> for the role="alert"? Why can't it be added to the <div class="messages" …> element?

Once that's answered or fixed, this is RTBC.

mgifford’s picture

So rather doing it in two places like:
<div class="messages messages--{{ type }} role="contentinfo">
&

{% if status_headings[type] == 'error' %}
  <div role="alert">
{% endif %}

You're asking why it can't be just done in one like:

{% if status_headings[type] == 'error' %}
  <div class="messages messages--{{ type }} role="contentinfo alert">
{% else %}
  <div class="messages messages--{{ type }} role="contentinfo">
{% endif %}

If so I don't know if it's alright to have multiple attributes for the role.. Not clear to me from:
http://www.w3.org/TR/wai-aria/roles

Wim Leers’s picture

Status: Needs review » Reviewed & tested by the community

Okay, never mind, I thought it'd be trivial to answer but apparently it could trick some screenreaders. Let's not let this issue be held up by extreme nitpickery :)

mgifford’s picture

Status: Reviewed & tested by the community » Needs work
FileSize
1.26 KB

I was just thinking about this, and although multiples may not be allowed, I think we could probably just use:

{% if status_headings[type] == 'error' %}
  <div class="messages messages--{{ type }} role="alert">
{% else %}
  <div class="messages messages--{{ type }} role="contentinfo">
{% endif %}

Let me roll that patch as it would simplify the template. I'm not sure of a situation where this wouldn't suffice, but I will check in with others.

Wim Leers’s picture

Okay :)

mgifford’s picture

Status: Needs work » Needs review

Sorry, switching status.

Wim Leers’s picture

Status: Needs review » Needs work

"needs work" was actually correct.

Could you please move this forward, so that we can all get this off our plate? It's so close!

Shyamala’s picture

Applying patch http://simplytest.me/, throws error msg: An error occurred while patching the project.

Wim Leers’s picture

Status: Needs work » Needs review
Issue tags: -Accessibility, -sprint, -Spark, -TwinCities

Status: Needs review » Needs work

The last submitted patch, landmark-status-message-2047175-36.patch, failed testing.

jessebeach’s picture

Status: Needs work » Needs review
Issue tags: +Accessibility, +sprint, +Spark, +TwinCities
mgifford’s picture

@Shyamala - can you confirm you were using http://simplytest.me/project/drupal/8.x

It may fail on the alpha3 release but should work against the git version.

EDIT: I didn't find any problems with a STM install.

Shyamala’s picture

@mgifford yes patch worked with http://simplytest.me/project/drupal/8.x

Not sure if I got the required output. Tried to get errors on 2 pages:
admin/structure/types/manage/mycontent page & user/1/edit pages.

Result using ChromeVox:
When the forms were submitted I was not automatically read the error message or confirmation messages. Instead I was read out the h1 or title tag on the page.

Also noted that the role added was role=" contentinfo" ="" even when these was actually an error.

Error msg on create content type page
error on create content type page

Error msg on edit profile page
error on profile edit page

Wim Leers’s picture

Status: Needs review » Needs work

I don't get why we're doing reviews. This patch was RTBC in #35. Then mgifford decided in #36 that he wanted to make the patch ever so slightly better. That's fine. But now we're 13 days further (!) and we're still walking in circles, because we're reviewing a patch that shouldn't be reviewed.

mgifford, please either A) tell us you don't want to reroll anymore, or B) reroll according to your plans in #36.

mgifford’s picture

Status: Needs work » Needs review
FileSize
1.26 KB

We realized that there was a missing '"' from the patch, so are re-rolling it here.

@Wim, we're trying to make sure that this works well for screen readers.

We're just going to verify that this works properly. So far we're running into some strange stuff like:
1) when you go admin/structure/types/manage/article/fields/
2) and "Add new field" but don't select a field type you get the following with the included patch:

<div class="messages messages--error">
<h2 class="visually-hidden">Error message</h2>
Add new field: you need to select a field type.
</div>

It should be:

<div class="messages messages--error" role="alert">
<h2 class="visually-hidden">Error message</h2>
Add new field: you need to select a field type.
</div>

But role seems to be stripped out...

jessebeach’s picture

The role=alert ARIA attribute currently has very poor support: http://blog.paciellogroup.com/2012/06/html5-accessibility-chops-aria-rol...

I think the best we can do is have it present in a sane way in the HTML so that if support improves, we'll be ready.

I've changed the HTML back to the original nested version in #8. This is because we want the role=contentinfo wrapper to always be present. This provides a landmark. role=alert is not a landmark, it's just shorthand for aria-live=assertive.

I moved the aria-label from the heading to the contentinfo div, since this gives the landmark a name and removes the duplicated announcement of the heading-and-label when the message h2 is read.

This produces the sample following output with both an error message and status message:

<div id="console" class="clearfix">
<!-- THEME DEBUG -->
<!-- CALL: theme('status_messages') -->
<!-- BEGIN OUTPUT from 'core/modules/system/templates/status-messages.html.twig' -->
  <div class="messages messages--error" role="contentinfo" aria-label="Error message">
    <div role="alert">
      <h2 class="visually-hidden">Error message</h2>
      <em class="placeholder">Notice</em>: Undefined index: path in <em class="placeholder">Drupal\Core\Routing\RouteBuilder-&gt;rebuild()</em> (line <em class="placeholder">97</em> of <em class="placeholder">core/lib/Drupal/Core/Routing/RouteBuilder.php</em>).
    </div>
  </div>
  <div class="messages messages--status" role="contentinfo" aria-label="Status message">
    <h2 class="visually-hidden">Status message</h2>
    Caches cleared.
  </div>
<!-- END OUTPUT from 'core/modules/system/templates/status-messages.html.twig' -->
</div>

The landmark navigation looks like this:

Error message contentinfo
Status message contentinfo

I think this is the best we can do for the moment.

mgifford’s picture

Status: Needs review » Reviewed & tested by the community

Thanks for posting Steve Faulkner's blog post. ChromeVox seems to deal with it well when I tested this on SimplyTest.me.

The patch would also be more useful if this also got in #1493324: Inline form errors for accessibility and UX

I think this is as discoverable as we can make this at the moment. Thanks again Jesse, Wim & Shyamala!

webchick’s picture

Issue summary: View changes
Status: Reviewed & tested by the community » Fixed

Wow, thanks a lot for all the thorough research done here.

Committed and pushed to 8.x. Thanks!

Wim Leers’s picture

Issue tags: -sprint
tstoeckler’s picture

+++ b/core/modules/system/templates/status-messages.html.twig
@@ -21,18 +24,24 @@
+  <div class="messages messages--{{ type }}" role="contentinfo" aria-label="{{ status_headings[type] }}">
...
+      {% if status_headings[type] %}
+        <h2 class="visually-hidden">{{ status_headings[type] }}</h2>
+      {% endif %}

Sorry, if this was discussed above, but it seems odd that we use status_headings[type] without checking for its existence, and then check for its existence below.

Status: Fixed » Closed (fixed)

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