945907 - Support atk/at-spi ROLE_LANDMARK.

Status: RESOLVED DUPLICATE of bug 1501182

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

E.g. banner might now map to this.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[Trevor Saunders (:tbsaunde)]

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

>   if (!accWrap) {
>     ProxyAccessible* proxy = GetProxy(aAtkObj);
>     if (!proxy)
>       return ATK_ROLE_INVALID;
> 
>+    isLandmark = proxy->LandmarkRole();
>     role = proxy->Role();

it'd be better to get the landmark second so we don't do ipc that can change the world and then  poke at the proxy.

>+  if (isLandmark && IsAtkVersionAtLeast(2, 12)) {
>+    aAtkObj->role = ATK_ROLE_LANDMARK;

so, this'll mean we now map headings footers forms atk roles to landmarks I think that's kind of silly and joanie agrees.  We might also want to consider the proper role for application.

>+  } else {
> #define ROLE(geckoRole, stringRole, atkRole, macRole, \
>              msaaRole, ia2Role, nameRule) \
>   case roles::geckoRole: \
>     aAtkObj->role = atkRole; \
>     break;
> 
>   switch (role) {
> #include "RoleMap.h"
>     default:
>       MOZ_CRASH("Unknown role.");
>   };

needs reindent

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #2)

> >+    isLandmark = proxy->LandmarkRole();
> >     role = proxy->Role();
> 
> it'd be better to get the landmark second so we don't do ipc that can change
> the world and then  poke at the proxy.

just swap them? LandmarkRole can change the world? - I'm not sure on the same page.

> 
> >+  if (isLandmark && IsAtkVersionAtLeast(2, 12)) {
> >+    aAtkObj->role = ATK_ROLE_LANDMARK;
> 
> so, this'll mean we now map headings footers forms atk roles to landmarks I
> think that's kind of silly and joanie agrees.

so ATK has roles for headings, footers, etc? too bad :)

Comment 4

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 5

[Trevor Saunders (:tbsaunde)]

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

>+  if (landmarkRole && IsAtkVersionAtLeast(2, 12)) {
>+    nsRoleMapEntry* roleMap = aria::GetRoleMap(landmarkRole);
>+    if (roleMap && roleMap->IsOfType(eLandmark) &&
>+        roleMap->roleRule == kUseNativeRole) {

this seems kind of confused, I'm not really clear on what you are trying to do, but I guess you want to check if you should use the landmark role or a different one.  However roleRule is afaik about html role vs aria, and so has nothing to do with what role atk wants.

Comment 6

[alexander :surkov (:asurkov)]

(In reply to Trevor Saunders (:tbsaunde) from comment #5)
> Comment on attachment 8668505 [details] [diff] [review]
> patch
> 
> >+  if (landmarkRole && IsAtkVersionAtLeast(2, 12)) {
> >+    nsRoleMapEntry* roleMap = aria::GetRoleMap(landmarkRole);
> >+    if (roleMap && roleMap->IsOfType(eLandmark) &&
> >+        roleMap->roleRule == kUseNativeRole) {
> 
> this seems kind of confused, I'm not really clear on what you are trying to
> do, but I guess you want to check if you should use the landmark role or a
> different one.  However roleRule is afaik about html role vs aria, and so
> has nothing to do with what role atk wants.

even if it's not same (it'd be good to have an example where it doesn't work), then it's close match. The alg here is when we don't have role mapping for a landmark role, then we use ATK_LANDMARK.

Comment 7

[James Teh [:Jamie]]

This will happen as part of bug 1501182, since we'll now be doing this for all platforms instead of specifically for ATK.