676267 - expose stale state on accessibles unattached from tree

Status: RESOLVED FIXED

Description

[alexander :surkov]

Comment 1

[alexander :surkov]

Created attachment 550414 [details] [diff] [review]
patch

Comment 2

[Trevor Saunders (:tbsaunde)]

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]

(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]

landed on inbound http://hg.mozilla.org/integration/mozilla-inbound/rev/dd38dafe931b

Comment 5

[alexander :surkov]

landed http://hg.mozilla.org/mozilla-central/rev/dd38dafe931b