740725 - Add down-casting to nsXULTreeAccessible

Status: RESOLVED FIXED

(Core :: Disability Access APIs, defect)

Description

[Mark Capella [:capella]]

Add down-casting to nsXULTreeAccessible class similar to work done for nsRootAccessible in bug#636845 to support DeCOMtamination effort.

https://wiki.mozilla.org/Gecko:DeCOMtamination

Comment 1

[Mark Capella [:capella]]

bug#636945

Comment 2

[Mark Capella [:capella]]

Attachment: Patch (v1)

Ok, first try at this patch ... builds ok, but fails during testing. Can you see anything? I've narrowed it to the following (it works if I comment it out).

test_focus_general.xul fails while running this one test:

   gQueue.push(new synthDownKey("radiosweater",
                                 new focusChecker("radiojacket")));

Comment 3

[alexander :surkov (:asurkov)]

(In reply to Mark Capella [:capella] from comment #2)
> Created attachment 611569 [details] [diff] [review]
> Patch (v1)
> 
> Ok, first try at this patch ... builds ok, but fails during testing. Can you
> see anything? I've narrowed it to the following (it works if I comment it
> out).

it looks correct

> 
> test_focus_general.xul fails while running this one test:
> 
>    gQueue.push(new synthDownKey("radiosweater",
>                                  new focusChecker("radiojacket")));

no XUL tree at all here, no known intermittent failures for test_focus_general.xul and no XUL tree tests there. Can you make sure you build this patch against trunk and no other parches are applied

Comment 4

[alexander :surkov (:asurkov)]

Comment on attachment 611569 [details] [diff] [review]
Patch (v1)

Review of attachment 611569 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/src/base/nsRootAccessible.cpp
@@ +394,1 @@
>      if (treeAcc) {

you don't need to check NodeInfo, just do
nsXULTreeAccessible* treeAcc = accessible->AsXULTree();
if (treeAcc) {

Comment 5

[Mark Capella [:capella]]

Attachment: Patch (v2)

It looks like I had the patch re-based after all. Alexs suggestion seems have been the missing piece ... builds and tests out now just fine !

Comment 6

[alexander :surkov (:asurkov)]

Mark, could you please mark patches as obsolete when you upload new one and please move '?' flags (like review requests) to new patch?

Comment 7

[Trevor Saunders (:tbsaunde)]

Comment on attachment 611718 [details] [diff] [review]
Patch (v2)

>+    if (eventType.EqualsLiteral("TreeRowCountChanged")) {
>+      HandleTreeRowCountChangedEvent(aDOMEvent, treeAcc);
>+      return;
>+    }
>+    if (eventType.EqualsLiteral("TreeInvalidated")) {

nit, blank line between ifs would be nice

>+inline nsXULTreeAccessible*
>+nsAccessible::AsXULTree()
>+{
>+  return mFlags & eXULTreeAccessible ?

you could use IsXULTree() to be sure we are consistant.

btw I don't see changes to nsAccessibilityService.cpp in this patch, so you missed one place where we use do_QueryObject ot get an nsXULTreeAccessible.  If you wnat to be sure you got all of them I'm pretty sure you could rm the class ID xpcom macros from nsXULTreeAccessible.

Comment 8

[Mark Capella [:capella]]

Attachment: Patch (v3)

Comment 9

[alexander :surkov (:asurkov)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/8f85a5488ffb

Comment 10

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/8f85a5488ffb