Bug 1342433 - don't recreate a tree on onclick changes, r=yzen
☠☠ backed out by 76614caf4ddc ☠ ☠
authorAlexander Surkov <surkov.alexander@gmail.com>
Tue, 14 Mar 2017 14:29:08 -0400
changeset 347844 7c05090a2cd01ad71df1d3840ca17818d8b6cdba
parent 347843 26ded80b0df562e15af98cdd3a15d5adac3ef426
child 347845 4f2cd3f715a218dc3bca55e89720b6aa1040d35c
push id38951
push userkwierso@gmail.com
push dateWed, 15 Mar 2017 21:29:40 +0000
treeherderautoland@0b51e2a9d781 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen
bugs1342433
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1342433 - don't recreate a tree on onclick changes, r=yzen
accessible/base/nsAccessibilityService.cpp
accessible/tests/mochitest/treeupdate/test_bug1175913.html
accessible/tests/mochitest/treeupdate/test_textleaf.html
--- a/accessible/base/nsAccessibilityService.cpp
+++ b/accessible/base/nsAccessibilityService.cpp
@@ -312,26 +312,23 @@ nsAccessibilityService::ListenersChanged
           listenerName != nsGkAtoms::onmousedown &&
           listenerName != nsGkAtoms::onmouseup) {
         continue;
       }
 
       nsIDocument* ownerDoc = node->OwnerDoc();
       DocAccessible* document = GetExistingDocAccessible(ownerDoc);
 
-      // Always recreate for onclick changes.
-      if (document) {
-        if (nsCoreUtils::HasClickListener(node)) {
-          if (!document->GetAccessible(node)) {
-            document->RecreateAccessible(node);
-          }
-        } else {
-          if (document->GetAccessible(node)) {
-            document->RecreateAccessible(node);
-          }
+      // Create an accessible for a inaccessible element having click event
+      // handler.
+      if (document && !document->HasAccessible(node) &&
+          nsCoreUtils::HasClickListener(node)) {
+        nsIContent* parentEl = node->GetFlattenedTreeParent();
+        if (parentEl) {
+          document->ContentInserted(parentEl, node, node->GetNextSibling());
         }
         break;
       }
     }
   }
   return NS_OK;
 }
 
--- a/accessible/tests/mochitest/treeupdate/test_bug1175913.html
+++ b/accessible/tests/mochitest/treeupdate/test_bug1175913.html
@@ -42,28 +42,30 @@
       {
         return "Test that show event is sent when click listener is added";
       }
     }
 
     function testRemoveListener()
     {
       this.eventSeq = [
-        new invokerChecker(EVENT_HIDE, getNode("parent")),
+        new unexpectedInvokerChecker(EVENT_HIDE, getNode("parent")),
       ];
 
       this.invoke = function testRemoveListener_invoke()
       {
         getNode("parent").removeEventListener("click", dummyListener);
       }
 
       this.finalCheck = function testRemoveListener_finalCheck()
       {
-        is(getAccessible("parent", null, null, DONOTFAIL_IF_NO_ACC), null, "Check that parent is not accessible.");
-        is(getAccessible("child", null, null, DONOTFAIL_IF_NO_ACC), null, "Check that child is not accessible.");
+        ok(getAccessible("parent", null, null, DONOTFAIL_IF_NO_ACC),
+           "Parent stays accessible after click event listener is removed");
+        ok(!getAccessible("child", null, null, DONOTFAIL_IF_NO_ACC),
+           "Child stays inaccessible");
       }
 
       this.getID = function testRemoveListener_getID()
       {
         return "Test that hide event is sent when click listener is removed";
       }
     }
 
--- a/accessible/tests/mochitest/treeupdate/test_textleaf.html
+++ b/accessible/tests/mochitest/treeupdate/test_textleaf.html
@@ -38,32 +38,26 @@
       }
     }
 
     function setOnClickAttr(aID)
     {
       var node = getNode(aID);
       node.setAttribute("onclick", "alert(3);");
       var textLeaf = getAccessible(node).firstChild;
-      is(textLeaf.actionCount, 1, "Wrong action numbers!");
+      is(textLeaf.actionCount, 1, "setOnClickAttr: wrong action numbers!");
     }
 
     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";
-      }
+      let node = getNode(aID);
+      node.removeAttribute("onclick");
+      let textLeaf = getAccessible(node).firstChild;
+      is(textLeaf.actionCount, 0,
+         "removeOnClickAttr: wrong action numbers!");
     }
 
     function setOnClickNRoleAttrs(aID)
     {
       this.__proto__ = new textLeafUpdate(aID, true);
 
       this.invoke = function setOnClickAttr_invoke()
       {
@@ -124,20 +118,21 @@
     //gA11yEventDumpToConsole = true;
 
     var gQueue = null;
 
     function doTest()
     {
       // adds onclick on element, text leaf should inherit its action
       setOnClickAttr("div");
+      // remove onclick attribute, text leaf shouldn't have any action
+      removeOnClickAttr("div");
+
       // Call rest of event tests.
       gQueue = new eventQueue();
-      // 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"));
 
       // text data removal of text node should remove its text accessible
       gQueue.push(new removeTextData("p", ROLE_PARAGRAPH));
       gQueue.push(new removeTextData("pre", ROLE_TEXT_CONTAINER));