151108 - Support nsIAccessibleAction in MAI (Mozilla Atk Implementation)

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Bolian Yin]

Comment 1

[Bolian Yin]

Attachment: patch_v1

support nsIAccessibleAction Interface. 
map the call from atk action interface to nsIAccessibleAction Interface.

Comment 2

[Kyle Yuan]

bolian, I suggest you call nsIAccessible::accDoAction() etc. directly rather
than call nsIAccessibleAction::doAction(), because I think there are a lot of
duplicate methods in those two interfaces and I'm not going to implement
nsIAccessibleAction in trunk.

Comment 3

[Bolian Yin]

Attachment: patch_v2 (call funcs in nsIAccessilbe other than nsIAccessibleAction)

Since Kyle said, there will be no sperate implementation of nsIAccessibleAction
in trunk, use nsIAccessilbe instead.

Comment 4

[Kyle Yuan]

Comment on attachment 87343 [details] [diff] [review]
patch_v2 (call funcs in nsIAccessilbe other than nsIAccessibleAction)


two comments:
1) you can use nsAccessible::GetAccKeyboardShortcut() in
MaiInterfaceAction::GetKeybinding()

2) +	//    nsIAccessibleAction *GetNSAccessibleAction();
this comment should be removed

other than above that is ok for me.
r=kyle

Comment 5

[Bolian Yin]

Kyle, GetAccKeyboardShortcut() is not suitable for action shortchut, there maybe
more than one action for nsAccessible.

Comment 6

[Bolian Yin]

Attachment: patch_v3 (move callback into C namespace)

Comment 7

[Kyle Yuan]

Comment on attachment 88430 [details] [diff] [review]
patch_v3 (move callback into C namespace)

r=kyle

Comment 8

[Bolian Yin]

Attachment: patch_v4

comments in bug 151133 hold here

Comment 9

[Aaron Leventhal]

If you want
+    nsresult rv = accessible->AccDoAction(aActionIndex);
+    return (NS_FAILED(rv)) ? FALSE : TRUE;
could be
+    return ! NS_FAILED(accessible->AccDoAction(eActionIndex));

However, your way is good too :-)

Put in a better comment than
+    /* ???? */
+

Avoid else after return. It is not necessary.
+    if (maiInterfaceAction)
+        return maiInterfaceAction->DoAction(aActionIndex);
+    else
+        return FALSE;

Comment 10

[Bolian Yin]

Attachment: patch_v5 (add aaronl's comment, thanks)

Comment 11

[Aaron Leventhal]

Comment on attachment 89245 [details] [diff] [review]
patch_v5 (add aaronl's comment, thanks)

r=aaronl

Comment 12

[Bolian Yin]

This bug has been checked in.
It is not in the default build and we need it to do later work. Further
comments, review, and super-review is still welcome, I will file new bug if
needed. Thanks for everyone.

Comment 13

[Johnny Stenback (:jst)]

Comment on attachment 89245 [details] [diff] [review]
patch_v5 (add aaronl's comment, thanks)

sr=jst