Bug 545465 - don't travese tree in Init, r=ginn.chen, r=davidb, a=blockingBetaN

accessible/src/base/nsAccessible.h

@@ -433,7 +433,7 @@ protected:
   /**
    * Set accessible parent and index in parent.
    */
-  void BindToParent(nsAccessible* aParent, PRUint32 aIndexInParent);
+  virtual void BindToParent(nsAccessible* aParent, PRUint32 aIndexInParent);
   void UnbindFromParent();
 
   /**

accessible/src/base/nsBaseWidgetAccessible.cpp

@@ -207,13 +207,6 @@ nsLinkableAccessible::GetKeyboardShortcut(nsAString& aKeyboardShortcut)
 ////////////////////////////////////////////////////////////////////////////////
 // nsLinkableAccessible. nsAccessNode
 
-PRBool
-nsLinkableAccessible::Init()
-{
-  CacheActionContent();
-  return nsAccessibleWrap::Init();
-}
-
 void
 nsLinkableAccessible::Shutdown()
 {
@@ -242,11 +235,19 @@ nsLinkableAccessible::GetAnchorURI(PRUint32 aAnchorIndex)
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// nsLinkableAccessible
+// nsLinkableAccessible: nsAccessible protected
 
 void
-nsLinkableAccessible::CacheActionContent()
+nsLinkableAccessible::BindToParent(nsAccessible* aParent,
+                                   PRUint32 aIndexInParent)
 {
+  nsAccessibleWrap::BindToParent(aParent, aIndexInParent);
+
+  // Cache action content.
+  mActionContent = nsnull;
+  mIsLink = PR_FALSE;
+  mIsOnclick = PR_FALSE;
+
   nsIContent* walkUpContent = mContent;
   PRBool isOnclick = nsCoreUtils::HasClickListener(walkUpContent);
 
@@ -276,6 +277,9 @@ nsLinkableAccessible::CacheActionContent()
   }
 }
 
+////////////////////////////////////////////////////////////////////////////////
+// nsLinkableAccessible: protected
+
 nsAccessible *
 nsLinkableAccessible::GetActionAccessible() const
 {

accessible/src/base/nsBaseWidgetAccessible.h

@@ -97,7 +97,6 @@ public:
   NS_IMETHOD GetKeyboardShortcut(nsAString& _retval);
 
   // nsAccessNode
-  virtual PRBool Init();
   virtual void Shutdown();
 
   // nsAccessible
@@ -107,16 +106,16 @@ public:
   virtual already_AddRefed<nsIURI> GetAnchorURI(PRUint32 aAnchorIndex);
 
 protected:
+  // nsAccessible
+  virtual void BindToParent(nsAccessible* aParent, PRUint32 aIndexInParent);
+
+  // nsLinkableAccessible
+
   /**
    * Return an accessible for cached action node.
    */
   nsAccessible *GetActionAccessible() const;
 
-  /**
-   * Cache action node.
-   */
-  virtual void CacheActionContent();
-
   nsCOMPtr<nsIContent> mActionContent;
   PRPackedBool mIsLink;
   PRPackedBool mIsOnclick;

accessible/src/base/nsDocAccessible.cpp

@@ -1336,6 +1336,7 @@ nsDocAccessible::BindToDocument(nsAccessible* aAccessible,
   if (!mAccessibleCache.Put(aAccessible->UniqueID(), aAccessible)) {
     if (aAccessible->IsPrimaryForNode())
       mNodeToAccessibleMap.Remove(aAccessible->GetNode());
+
     return false;
   }
 
@@ -1354,14 +1355,14 @@ nsDocAccessible::BindToDocument(nsAccessible* aAccessible,
 void
 nsDocAccessible::UnbindFromDocument(nsAccessible* aAccessible)
 {
-  // Remove an accessible from node to accessible map if it is presented there.
+  // Remove an accessible from node-to-accessible map if it exists there.
   if (aAccessible->IsPrimaryForNode() &&
       mNodeToAccessibleMap.Get(aAccessible->GetNode()) == aAccessible)
     mNodeToAccessibleMap.Remove(aAccessible->GetNode());
 
 #ifdef DEBUG
   NS_ASSERTION(mAccessibleCache.GetWeak(aAccessible->UniqueID()),
-               "Illegitimate illegitimated accessible!");
+               "Unbinding the unbound accessible!");
 #endif
 
   void* uniqueID = aAccessible->UniqueID();

accessible/src/base/nsDocAccessible.h

@@ -220,7 +220,7 @@ public:
   bool BindToDocument(nsAccessible* aAccessible, nsRoleMapEntry* aRoleMapEntry);
 
   /**
-   * Remove the existing accessible from document caches and shutdown it.
+   * Remove from document and shutdown the given accessible.
    */
   void UnbindFromDocument(nsAccessible* aAccessible);
 

accessible/tests/mochitest/treeupdate/Makefile.in

@@ -51,6 +51,7 @@ _TEST_FILES =\
 		test_list.html \
 		test_recreation.html \
 		test_tableinsubtree.html \
+		test_textleaf.html \
 		$(NULL)
 
 libs:: $(_TEST_FILES)

accessible/tests/mochitest/treeupdate/test_textleaf.html

@@ -0,0 +1,138 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+  <title>Test accessible recreation</title>
+
+  <link rel="stylesheet" type="text/css"
+        href="chrome://mochikit/content/tests/SimpleTest/test.css" />
+
+  <script type="application/javascript"
+          src="chrome://mochikit/content/MochiKit/packed.js"></script>
+  <script type="application/javascript"
+          src="chrome://mochikit/content/tests/SimpleTest/SimpleTest.js"></script>
+
+  <script type="application/javascript"
+          src="../common.js"></script>
+  <script type="application/javascript"
+          src="../role.js"></script>
+  <script type="application/javascript"
+          src="../events.js"></script>
+
+  <script type="application/javascript">
+
+    ////////////////////////////////////////////////////////////////////////////
+    // Invokers
+
+    function textLeafUpdate(aID, aIsTextLeafLinkable)
+    {
+      this.node = getNode(aID);
+
+      this.eventSeq = [
+        new invokerChecker(EVENT_REORDER, this.node.parentNode)
+      ];
+
+      this.finalCheck = function textLeafUpdate_finalCheck()
+      {
+        var textLeaf = getAccessible(this.node).firstChild;
+        is(textLeaf.numActions, (aIsTextLeafLinkable ? 1 : 0),
+           "Wrong action numbers!");
+      }
+    }
+
+    function setOnClickAttr(aID)
+    {
+      this.__proto__ = new textLeafUpdate(aID, true);
+
+      this.invoke = function setOnClickAttr_invoke()
+      {
+        this.node.setAttribute("onclick", "alert(3);");
+      }
+
+      this.getID = function setOnClickAttr_getID()
+      {
+        return "make " + prettyName(aID) + " linkable";
+      }
+    }
+
+    function removeOnClickAttr(aID)
+    {
+      this.__proto__ = new textLeafUpdate(aID, false);
+
+      this.invoke = function removeOnClickAttr_invoke()
+      {
+        this.node.removeAttribute("onclick");
+      }
+
+      this.getID = function removeOnClickAttr_getID()
+      {
+        return "unmake " + prettyName(aID) + " linkable";
+      }
+    }
+
+    function setOnClickNRoleAttrs(aID)
+    {
+      this.__proto__ = new textLeafUpdate(aID, true);
+
+      this.invoke = function setOnClickAttr_invoke()
+      {
+        this.node.setAttribute("role", "link");
+        this.node.setAttribute("onclick", "alert(3);");
+      }
+
+      this.getID = function setOnClickAttr_getID()
+      {
+        return "make " + prettyName(aID) + " linkable";
+      }
+    }
+
+    ////////////////////////////////////////////////////////////////////////////
+    // Test
+
+    //gA11yEventDumpID = "eventdump"; // debug stuff
+    //gA11yEventDumpToConsole = true;
+
+    var gQueue = null;
+
+    function doTest()
+    {
+      gQueue = new eventQueue();
+
+      // adds onclick on element, text leaf should inherit its action
+      gQueue.push(new setOnClickAttr("div"));
+
+      // remove onclick attribute, text leaf shouldn't have any action
+      gQueue.push(new removeOnClickAttr("div"));
+
+      // set onclick attribute making span accessible, it's inserted into tree
+      // and adopts text leaf accessible, text leaf should have an action
+      gQueue.push(new setOnClickNRoleAttrs("span"));
+
+      gQueue.invoke(); // SimpleTest.finish() will be called in the end
+    }
+
+    SimpleTest.waitForExplicitFinish();
+    addA11yLoadEvent(doTest);
+  </script>
+</head>
+<body>
+
+  <a target="_blank"
+     title="Clean up the code of accessible initialization and binding to the tree"
+     href="https://bugzilla.mozilla.org/show_bug.cgi?id=545465">
+    Mozilla Bug 545465
+  </a>
+
+  <p id="display"></p>
+  <div id="content" style="display: none"></div>
+  <pre id="test">
+  </pre>
+
+  <div id="container">
+    <div id="div">div</div>
+    <span id="span">span</span>
+  </div>
+
+  <div id="eventdump"></div>
+</body>
+</html>