762898 - implement UIA_AriaRolePropertyId

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[alexander :surkov (:asurkov)]

it's analogue of xml-roles object attributes, see Accessible::GetAttributes(nsIPersistentProperties **aAttributes)

1) add a case uiaRawElmProvider::GetPropertyValue
2) obtain ARIA role attribute
3) return its value as BSTR

Comment 1

[Andrew Quartey [:drexler]]

Attachment: patch

This builds locally. Unsure of how to proceed in testing this...pointers more than welcomed.

Comment 2

[Trevor Saunders (:tbsaunde)]

so, first thing you should use GetAttributes() here and get the xml-roles attributes since that will give you hte full aria role string not just the stringification of the enumerated role, which isn't correct with something like role="list, grid" where xml-roles will be correct. see http://msdn.microsoft.com/en-us/library/windows/desktop/ee684013%28v=vs.85%29.aspx#ARIARoleMapped or section 5.3 and 5.4 of the aria implementors guide.

Comment 3

[Trevor Saunders (:tbsaunde)]

Comment on attachment 640898 [details] [diff] [review]
patch

>-  // UI Automation will attempt to get the property from the host
>-  //window provider.
>+  
>   aPropertyValue->vt = VT_EMPTY;

nit, trailing spaces on the blank line

>+  if (mAcc) {

that's always true, but you should return CO_E_OBJECTNOTCONNECTED if mAcc is defunct.

Comment 4

[Andrew Quartey [:drexler]]

Attachment: patch

made changes per previous review comment.

Comment 5

[Andrew Quartey [:drexler]]

Attachment: patch

Ooops. refreshed patch.

Comment 6

[Trevor Saunders (:tbsaunde)]

Comment on attachment 642672 [details] [diff] [review]
patch

>+      if(!xmlRoles.IsEmpty()) {
>+        aPropertyValue->vt = VT_BSTR;
>+        aPropertyValue->bstrVal = ::SysAllocString(xmlRoles.get());
>+      }
>+      return S_OK;

nit, blank line after }

note you'll need to merge in the changes in bug 762806 (which I just pushed).

Comment 7

[Andrew Quartey [:drexler]]

Attachment: patch_for_checkin

merged and rebased

Comment 8

[Andrew Quartey [:drexler]]

Attachment: patch_for_checkin

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2394b87d167

Comment 10

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #2)
> so, first thing you should use GetAttributes() here and get the xml-roles
> attributes since that will give you hte full aria role string not just the
> stringification of the enumerated role, which isn't correct with something
> like role="list, grid" where xml-roles will be correct. see
> http://msdn.microsoft.com/en-us/library/windows/desktop/ee684013%28v=vs.
> 85%29.aspx#ARIARoleMapped or section 5.3 and 5.4 of the aria implementors
> guide.

the problem is that xml-roles is ARIA plus extra stuff, the spec says us to expose ARIA. 

Btw, the spec (http://msdn.microsoft.com/en-us/library/dd757479%28v=vs.85%29.aspx) doesn't say that "list grid" is incorrect value, see "Identifies the AriaRole property, which is a string containing the Accessible Rich Internet Application (ARIA) role information for the automation element."

Btw, the link you provided (http://msdn.microsoft.com/en-us/library/windows/desktop/ee684013%28v=vs.85%29.aspx#ARIARoleMapped) says "User agents may also offer secondary roles in the AriaRole property by using space as a separator, as defined in the ARIA W3C standard" what makes "list grid" value valid.

Comment 11

[Trevor Saunders (:tbsaunde)]

(In reply to alexander :surkov from comment #10)
> (In reply to Trevor Saunders (:tbsaunde) from comment #2)
> > so, first thing you should use GetAttributes() here and get the xml-roles
> > attributes since that will give you hte full aria role string not just the
> > stringification of the enumerated role, which isn't correct with something
> > like role="list, grid" where xml-roles will be correct. see
> > http://msdn.microsoft.com/en-us/library/windows/desktop/ee684013%28v=vs.
> > 85%29.aspx#ARIARoleMapped or section 5.3 and 5.4 of the aria implementors
> > guide.
> 
> the problem is that xml-roles is ARIA plus extra stuff, the spec says us to
> expose ARIA. 

it looks like it was originally your suggestion in the description.  I wouldn't really mind just mAcc->GetNode()->GetAttribute() but iirc xml-roles is pretty close so maybe ok.

> Btw, the spec
> (http://msdn.microsoft.com/en-us/library/dd757479%28v=vs.85%29.aspx) doesn't
> say that "list grid" is incorrect value, see "Identifies the AriaRole
> property, which is a string containing the Accessible Rich Internet
> Application (ARIA) role information for the automation element."
> 
> Btw, the link you provided
> (http://msdn.microsoft.com/en-us/library/windows/desktop/ee684013%28v=vs.
> 85%29.aspx#ARIARoleMapped) says "User agents may also offer secondary roles
> in the AriaRole property by using space as a separator, as defined in the
> ARIA W3C standard" what makes "list grid" value valid.

I don't disagree, my point was that  the string version of ARIARole() was not correct.

Comment 12

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #11)
> > the problem is that xml-roles is ARIA plus extra stuff, the spec says us to
> > expose ARIA. 
> 
> it looks like it was originally your suggestion in the description.

I said analogue ;)

>  I
> wouldn't really mind just mAcc->GetNode()->GetAttribute()

it should be what we need

> I don't disagree, my point was that  the string version of ARIARole() was
> not correct.

k, I see.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/d2394b87d167