Bug 32688 – MSAA: Accessibility role of list items is wrong

• Component: Accessibility
• Status: RESOLVED FIXED

[Jon Honeycutt]

List items should have the "list item" role, not the "grouping" role.

[Jon Honeycutt]

Created attachment 45185 [details]
patch

[Adam Roben (:aroben)]

Comment on attachment 45185 [details]
patch


> +static AccessibilityRole msaaRoleForRenderer(const RenderObject* renderer)
> +{
> +    if (!renderer)
> +        return UnknownRole;
> +
> +    if (renderer->isText())
> +        return EditableTextRole;
> +
> +    if (renderer->isListItem())
> +        return ListItemRole;
> +
> +    return UnknownRole;
> +}
> +
>  AccessibilityRole AccessibilityRenderObject::roleValueForMSAA() const
>  {
>      if (m_roleForMSAA != UnknownRole)
>          return m_roleForMSAA;
>  
> -    if (m_renderer && m_renderer->isText())
> -        m_roleForMSAA = EditableTextRole;
> -    else
> +    m_roleForMSAA = msaaRoleForRenderer(m_renderer);
> +
> +    if (m_roleForMSAA == UnknownRole)
>          m_roleForMSAA = m_role;

It seems unfortunate that we have this m_roleForMSAA. I'd imagine the way things would work to be:

* WebCore has a master AccessibilityRole enum that has all the members needed by every supported accessibility platform
* The platform-specific integration with the system's accessibility layer would translate AccessibilityRoles into whatever the platform expects

Is there a reason why we don't do this?

r=me

[Jon Honeycutt]

(In reply to comment #2)
> (From update of attachment 45185 [details])
> 
> It seems unfortunate that we have this m_roleForMSAA. I'd imagine the way
> things would work to be:
> 
> * WebCore has a master AccessibilityRole enum that has all the members needed
> by every supported accessibility platform
> * The platform-specific integration with the system's accessibility layer would
> translate AccessibilityRoles into whatever the platform expects
> 
> Is there a reason why we don't do this?

No, there isn't. I'll post a patch to do that.

> 
> r=me

Thanks!

[Jon Honeycutt]

Landed in r52351.