We should use this as a template for adjusting our exposed info: http://trac.webkit.org/browser/trunk/LayoutTests/platform/mac-mavericks/accessibility/roles-exposed-expected.txt

Alex, considering the big number of empty subroles in the ARIA map, does this justify a change this big? Or should I rather go with a sub role map in MozAccessible.mm, similar to what you did for the roleDescription, and add specific mappings for ARIA roles and the native stuff that's currently switch-cased there?

(In reply to Marco Zehe (:MarcoZ) from comment #1) > Created attachment 8584661 [details] [diff] [review] > WIP V1 > > Alex, considering the big number of empty subroles in the ARIA map, does > this justify a change this big? Or should I rather go with a sub role map in > MozAccessible.mm, similar to what you did for the roleDescription, and add > specific mappings for ARIA roles and the native stuff that's currently > switch-cased there? agree, 2nd approach looks nicer

This adds mappings for various ARIA roles besides landmarks which cannot be inferred from the role alone. In a second step, I plan to refactor the second switch...case statement to map roles to subroles and add more definitions if needed. Also, a few current role mappings will be corrected. But this is the first step I wanted to make sure is OK.

Comment on attachment 8585610 [details] [diff] [review] WIP Part 1: Add ARIA role mappings Review of attachment 8585610 [details] [diff] [review]: ----------------------------------------------------------------- ::: accessible/mac/mozAccessible.mm @@ +431,5 @@ > + const nsString& subRole; > +}; > + > +static const SubRoleMap sSubRoleMap[] = { > + { nsGkAtoms::alert, NS_LITERAL_STRING("AXApplicationAlert") }, I'm curious if it works because atoms are initialized at runtime iirc, I think that was a reason we used to keep pointers to nsGkAtoms @@ +473,5 @@ > + nsIAtom* ariaRole = *(mGeckoAccessible->ARIARoleMap()->roleAtom); > + size_t idx = 0; > + if (BinarySearchIf(sSubRoleMap, 0, ArrayLength(sSubRoleMap), > + SubRoleComparator(ariaRole), &idx)) { > + return sSubRoleMap[idx].subRole; they are different types, are they magically converted somehow? I think I would use const char* instead

(In reply to alexander :surkov from comment #4) > they are different types, are they magically converted somehow? I think I > would use const char* instead That's why I asked for feedback on the general approach, since that is the part I hadn't quite figured out yet. OK I'll just use hard-coded strings in the map, then. One question: Is there a utility method to convert these nsIAtom* thingies to strings? Since I get an nsIAtom back from the GeckoAccessible RoleMap.

I think I would try something like struct SubRoleMap { nsIAtom** role; const char* subRole; }; ... { &nsGkAtoms::region, "AXDocumentRegion" }, ... if (BinarySearchIf(sSubRoleMap, 0, ArrayLength(sSubRoleMap), SubRoleComparator(ariaRole), &idx)) { return [NSString stringWithUTF8String:sSubRoleMap[idx].subRole]; }

Note to self: Another source worth looking at for mappings is the test suite in Chromium: https://chromium.googlesource.com/chromium/src.git/+/master/content/test/data/accessibility/

Bug 1137748 - Adjust and correct exposed roles, subroles, and roledescriptions on OS X

Comment on attachment 8626754 [details] MozReview Request: Bug 1137748 - Adjust and correct exposed roles, subroles, and roledescriptions on OS X https://reviewboard.mozilla.org/r/12137/#review10621 ::: accessible/mac/mozAccessible.mm:733 (Diff revision 1) > - nsIAtom* landmark = accWrap->LandmarkRole(); > + if (accWrap->HasARIARole()) { LandmarkRole is may be provided by non ARIA content, so you need to keep it and put HasARIARole() thing separately

Bug 1137748 - Expose correct roles, subroles, and roledescriptions for various WAI-ARIA roles, r?surkov

Comment on attachment 8626838 [details] MozReview Request: Bug 1137748 - Expose correct roles, subroles, and roledescriptions for various WAI-ARIA roles, r?surkov https://reviewboard.mozilla.org/r/12165/#review10627 just curious if these two changeset can be merged into one?

https://reviewboard.mozilla.org/r/12135/#review10629 ok, looks good, r=me

(In reply to alexander :surkov from comment #11) > just curious if these two changeset can be merged into one? Yes, I will do that when rebasing my "branch", the bookmark, onto the latest inbound before pushing.

Comment on attachment 8626838 [details] MozReview Request: Bug 1137748 - Expose correct roles, subroles, and roledescriptions for various WAI-ARIA roles, r?surkov https://reviewboard.mozilla.org/r/12165/#review10631 Ship It!

url: https://hg.mozilla.org/integration/mozilla-inbound/rev/0d4f428037b5c4dbce6cdbbeea9a3afb43a2e882 changeset: 0d4f428037b5c4dbce6cdbbeea9a3afb43a2e882 user: Marco Zehe <mzehe@mozilla.com> date: Fri Jun 26 17:31:44 2015 -0700 description: Bug 1137748 - Expose correct roles, subroles, and roledescriptions for various WAI-ARIA roles on OS X, r=surkov