Last Comment Bug 676267 - expose stale state on accessibles unattached from tree
: expose stale state on accessibles unattached from tree
Status: RESOLVED FIXED
: access
Product: Core
Classification: Components
Component: Disability Access APIs (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla8
Assigned To: alexander :surkov
:
Mentors:
Depends on:
Blocks: statea11y
  Show dependency treegraph
 
Reported: 2011-08-03 08:53 PDT by alexander :surkov
Modified: 2011-08-05 08:48 PDT (History)
0 users
surkov.alexander: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
patch (5.74 KB, patch)
2011-08-03 09:55 PDT, alexander :surkov
tbsaunde+mozbugs: review+
Details | Diff | Review

Description alexander :surkov 2011-08-03 08:53:22 PDT

    
Comment 1 alexander :surkov 2011-08-03 09:55:57 PDT
Created attachment 550414 [details] [diff] [review]
patch
Comment 2 Trevor Saunders (:tbsaunde) 2011-08-03 13:07:24 PDT
Comment on attachment 550414 [details] [diff] [review]
patch

>    */
>-  inline bool HasAccessible(nsINode* aNode)
>+  inline bool HasAccessible(nsINode* aNode) const
>   {
>     return GetAccessible(aNode);
>   }

nit, don't we usually use { return blah; } in cases like this?

> 
>   /**
>+   * Return true if the given accessible is in document.
>+   */
>+  inline bool IsInDocument(nsAccessible* aAccessible) const
>+  {
>+    nsAccessible* acc = aAccessible;

why do you need this local var? why not just mutate aAcc? 

>+    while (acc && !acc->IsPrimaryForNode())
>+      acc = acc->Parent();
>+
>+    return acc ? !!mNodeToAccessibleMap.Get(acc->GetNode()) : false;
>+  }

why do you need !! does some compiler complain?
Comment 3 alexander :surkov 2011-08-03 13:12:10 PDT
(In reply to comment #2)

> nit, don't we usually use { return blah; } in cases like this?

ok, fine with me

> >+  {
> >+    nsAccessible* acc = aAccessible;
> 
> why do you need this local var? why not just mutate aAcc? 

harder to debug, following the rule "keep in arguments unchanged"

> >+    return acc ? !!mNodeToAccessibleMap.Get(acc->GetNode()) : false;
> >+  }
> 
> why do you need !! does some compiler complain?

I didn't try, just this is usual way to convert pointer to boolean
Comment 4 alexander :surkov 2011-08-04 02:58:03 PDT
landed on inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/dd38dafe931b
Comment 5 alexander :surkov 2011-08-05 08:48:15 PDT
landed http://hg.mozilla.org/mozilla-central/rev/dd38dafe931b

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