implement UIA_AriaRolePropertyId

RESOLVED FIXED in mozilla17

Status

()

Core
Disability Access APIs
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: surkov, Assigned: drexler)

Tracking

(Blocks: 1 bug)

unspecified
mozilla17
x86
Windows 8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [good first bug][mentor=surkov.alexander@gmail.com][lang=c++])

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
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
(Assignee)

Comment 1

5 years ago
Created attachment 640898 [details] [diff] [review]
patch

This builds locally. Unsure of how to proceed in testing this...pointers more than welcomed.
Assignee: nobody → andrew.quartey
Status: NEW → ASSIGNED
Attachment #640898 - Flags: review?(surkov.alexander)
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 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.
Attachment #640898 - Flags: review?(surkov.alexander)
(Assignee)

Comment 4

5 years ago
Created attachment 642669 [details] [diff] [review]
patch

made changes per previous review comment.
Attachment #640898 - Attachment is obsolete: true
Attachment #642669 - Flags: review?(trev.saunders)
(Assignee)

Comment 5

5 years ago
Created attachment 642672 [details] [diff] [review]
patch

Ooops. refreshed patch.
Attachment #642669 - Attachment is obsolete: true
Attachment #642669 - Flags: review?(trev.saunders)
Attachment #642672 - Flags: review?(trev.saunders)
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).
Attachment #642672 - Flags: review?(trev.saunders) → review+
(Assignee)

Comment 7

5 years ago
Created attachment 644486 [details] [diff] [review]
patch_for_checkin

merged and rebased
(Assignee)

Comment 8

5 years ago
Created attachment 644501 [details] [diff] [review]
patch_for_checkin
Attachment #644486 - Attachment is obsolete: true
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2394b87d167
Flags: in-testsuite-
Keywords: checkin-needed
(Reporter)

Comment 10

5 years ago
(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.
(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.
(Reporter)

Comment 12

5 years ago
(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.
https://hg.mozilla.org/mozilla-central/rev/d2394b87d167
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.