Bug 171180 – AX: ARIA "region" role which lacks an accessible name should not be treated as a landmark

• Component: Accessibility
• Status: RESOLVED FIXED

[Joanmarie Diggs]

The ARIA spec states, "Authors must give each element with role region a brief label that describes the purpose of the content in the region." https://rawgit.com/w3c/aria/master/aria/aria.html#region

The Core AAM states, "Special case: if the region does not have an accessible name, do not expose the element as a landmark. Use the native host language role of the element instead." https://rawgit.com/w3c/aria/master/core-aam/core-aam.html#role-map-region

[Radar WebKit Bug Importer]

<rdar://problem/31775830>

[Joanmarie Diggs]

Created attachment 308612 [details]
Patch

[chris fleizach]

Comment on attachment 308612 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308612&action=review

> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2128
> +    if (role == LandmarkRegionRole && !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr))

do we think this is exhaustive !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr)
or are there are ways we should verify it has a label?

[Joanmarie Diggs]

(In reply to chris fleizach from comment #3)
> Comment on attachment 308612 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=308612&action=review
> 
> > Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2128
> > +    if (role == LandmarkRegionRole && !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr))
> 
> do we think this is exhaustive !hasAttribute(aria_labelAttr) &&
> !hasAttribute(aria_labelledbyAttr)
> or are there are ways we should verify it has a label?

It's admittedly not exhaustive. How exhaustive do you think we should be? Add alt and title? Or something else?

[chris fleizach]

Comment on attachment 308612 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=308612&action=review

>>> Source/WebCore/accessibility/AccessibilityNodeObject.cpp:2128
>>> +    if (role == LandmarkRegionRole && !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr))
>> 
>> do we think this is exhaustive !hasAttribute(aria_labelAttr) && !hasAttribute(aria_labelledbyAttr)
>> or are there are ways we should verify it has a label?
> 
> It's admittedly not exhaustive. How exhaustive do you think we should be? Add alt and title? Or something else?

I don't know. I just wanted to bring it up to make sure we considered the other cases. 
I don't want to encourage devs to keep using title attribute though so maybe this is good enough

[Joanmarie Diggs]

(In reply to chris fleizach from comment #5)

> I don't know. I just wanted to bring it up to make sure we considered the
> other cases. 

I appreciate your doing so.

> I don't want to encourage devs to keep using title attribute though so maybe
> this is good enough

Agreed. The ARIA spec language tries to steer them in the direction of label/labelled-by:

<quote>
Authors must give each element with role region a brief label that describes the purpose of the content in the region. Authors should reference a visible label with aria-labelledby if a visible label is present. Authors should include the label inside of a heading whenever possible. The heading may be an instance of the standard host language heading element or an instance of an element with role heading.
</quote>

And if it's not good enough and a bug is filed, assign it to me, and I'll add the additional cases as appropriate.

Thanks for the review!

[WebKit Commit Bot]

Comment on attachment 308612 [details]
Patch

Clearing flags on attachment: 308612

Committed r215968: <http://trac.webkit.org/changeset/215968>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.