Last Comment Bug 740725 - Add down-casting to nsXULTreeAccessible
: Add down-casting to nsXULTreeAccessible
Status: RESOLVED FIXED
[lang=c++]
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla14
Assigned To: Mark Capella [:capella]
:
Mentors:
https://wiki.mozilla.org/Gecko:DeCOMt...
Depends on: 739524
Blocks: dexpcoma11y
  Show dependency treegraph
 
Reported: 2012-03-30 00:50 PDT by Mark Capella [:capella]
Modified: 2012-04-03 10:57 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Patch (v1) (5.84 KB, patch)
2012-04-02 13:29 PDT, Mark Capella [:capella]
surkov.alexander: feedback+
Details | Diff | Review
Patch (v2) (6.31 KB, patch)
2012-04-02 22:18 PDT, Mark Capella [:capella]
tbsaunde+mozbugs: review+
Details | Diff | Review
Patch (v3) (7.19 KB, patch)
2012-04-02 23:35 PDT, Mark Capella [:capella]
no flags Details | Diff | Review

Description Mark Capella [:capella] 2012-03-30 00:50:09 PDT
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] 2012-03-30 05:22:21 PDT
bug#636945
Comment 2 Mark Capella [:capella] 2012-04-02 13:29:43 PDT
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).

test_focus_general.xul fails while running this one test:

   gQueue.push(new synthDownKey("radiosweater",
                                 new focusChecker("radiojacket")));
Comment 3 alexander :surkov 2012-04-02 20:32:34 PDT
(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 2012-04-02 20:34:28 PDT
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] 2012-04-02 22:18:34 PDT
Created attachment 611718 [details] [diff] [review]
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 2012-04-02 22:31:09 PDT
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) 2012-04-02 22:47:46 PDT
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] 2012-04-02 23:35:54 PDT
Created attachment 611732 [details] [diff] [review]
Patch (v3)
Comment 10 Matt Brubeck (:mbrubeck) 2012-04-03 10:57:25 PDT
https://hg.mozilla.org/mozilla-central/rev/8f85a5488ffb

Note You need to log in before you can comment on or make changes to this bug.