Bug 32688

Summary: MSAA: Accessibility role of list items is wrong
Product: WebKit Reporter: Jon Honeycutt <jhoneycutt>
Component: AccessibilityAssignee: Jon Honeycutt <jhoneycutt>
Status: RESOLVED FIXED    
Severity: Normal CC: aroben
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Windows XP   
Attachments:
Description Flags
patch aroben: review+, jhoneycutt: commit-queue-

Description Jon Honeycutt 2009-12-17 17:55:39 PST
List items should have the "list item" role, not the "grouping" role.
Comment 1 Jon Honeycutt 2009-12-18 13:47:43 PST
Created attachment 45185 [details]
patch
Comment 2 Adam Roben (:aroben) 2009-12-18 14:22:11 PST
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
Comment 3 Jon Honeycutt 2009-12-18 16:04:30 PST
(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!
Comment 4 Jon Honeycutt 2009-12-18 16:52:36 PST
Landed in r52351.