Bug 1342433 - onclick changes shouldn't recreate the tree. r=yzen, a=lizzard
☠☠ backed out by 4d660d9d4255 ☠ ☠
authorAlexander Surkov <surkov.alexander@gmail.com>
Wed, 15 Mar 2017 15:19:26 -0400
changeset 379473 35bf6122572ace2295ed0d0a13c3ba6edd0a3e40
parent 379472 9e3e0759726b21e37afe2e55acf53f13a94f3918
child 379474 d2a680d1fbd8dbe6b143adae1b74379b40a6ad21
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersyzen, lizzard
bugs1342433
milestone53.0
Bug 1342433 - onclick changes shouldn't recreate the tree. r=yzen, a=lizzard
accessible/base/nsAccessibilityService.cpp
accessible/tests/browser/e10s/browser_treeupdate_listener.js
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/browser/e10s/browser_treeupdate_listener.js
+++ b/accessible/tests/browser/e10s/browser_treeupdate_listener.js
@@ -21,23 +21,9 @@ addAccessibleTask('<span id="parent"><sp
       content.window.dummyListener = () => {};
       content.document.getElementById('parent').addEventListener(
         'click', content.window.dummyListener);
     });
     yield onReorder;
 
     let tree = { TEXT: [] };
     testAccessibleTree(findAccessibleChildByID(accDoc, 'parent'), tree);
-
-    onReorder = waitForEvent(EVENT_REORDER, 'body');
-    // Remove an event listener from parent.
-    yield ContentTask.spawn(browser, {}, () => {
-      content.document.getElementById('parent').removeEventListener(
-        'click', content.window.dummyListener);
-      delete content.window.dummyListener;
-    });
-    yield onReorder;
-
-    is(findAccessibleChildByID(accDoc, 'parent'), null,
-      'Check that parent is not accessible.');
-    is(findAccessibleChildByID(accDoc, 'child'), null,
-      'Check that child is not accessible.');
   });
--- 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";
-      }
+      var node = getNode(aID);
+      node.removeAttribute("onclick");
+      var 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));