bug 424161 - Tree invalidation screwed up badly, getting 6 failures on Mochitest for bug 368835. p=Alexander Surkov (surkov.alexander@gmail.com) r=me, r=Olli.Pettay sr=jonas a1.9=beltzner

accessible/src/base/nsRootAccessible.cpp

@@ -649,7 +649,7 @@ nsresult nsRootAccessible::HandleEventWithTarget(nsIDOMEvent* aEvent,
     nsCOMPtr<nsIContent> treeContent = do_QueryInterface(aTargetNode);
     nsAccEvent::PrepareForEvent(aTargetNode, PR_TRUE);
     return accService->InvalidateSubtreeFor(eventShell, treeContent,
-                                            nsIAccessibleEvent::EVENT_ASYNCH_SIGNIFICANT_CHANGE);
+                                            nsIAccessibleEvent::EVENT_DOM_SIGNIFICANT_CHANGE);
   }
 #endif
 

accessible/tests/mochitest/test_bug368835.xul

@@ -62,13 +62,21 @@
       performActionOnCell: function(aAction, aRow, aCol) {}
     };
 
+    var gTreeViewChangedCount = 0;
     var gTreeViewChanged = false;
     function TreeViewChangedHandler(aEvent)
     {
-      gTreeViewChanged = true;
+      gTreeViewChangedCount++;
 
-      // Tree view has been setted. We can continue tests for the tree.
+      // We get two systems 'treeViewChanged' event when tree is initialized
-      window.setTimeout(doTest2, 0);
+      // The third one is our when we change the tree view by
+      // nsITreeBoxObject::setTree.
+      if (gTreeViewChangedCount == 3) {
+        gTreeViewChanged = true;
+
+        // Tree view has been setted. We can continue tests for the tree.
+        window.setTimeout(doTest2, 500);
+      }
     }
 
     var gTreeRowCountChanged = false;
@@ -83,59 +91,50 @@
       is(count, 1, "Wrong 'count' data of 'treeRowCountChanged' event.");
     }
 
+    var gTreeColumnInvalidated = false;
+    var gTreeRowInvalidated = false;
+
     var gTreeInvalidatedCount = 0;
     function TreeInvalidatedHandler(aEvent)
     {
       gTreeInvalidatedCount++;
 
-      if (gTreeInvalidatedCount == 2)
+      switch (gTreeInvalidatedCount) {
-        TreeColumnInvalidatedHandler(aEvent);
+        case 1:
-      else if (gTreeInvalidatedCount == 5)
+          TreeInvalidatedHandlerHelper(aEvent, 0, 5, null, null,
-        TreeRowInvalidatedHandler(aEvent);
+                                       "nsITreeBoxObject::rowCountChanged");
+          break;
+        case 2:
+          TreeInvalidatedHandlerHelper(aEvent, null, null, 0, 0,
+                                       "nsITreeBoxObject::invalidateColumn");
+          gTreeColumnInvalidated = true;
+          break;
+        case 3:
+          TreeInvalidatedHandlerHelper(aEvent, 1, 1, null, null,
+                                       "nsITreeBoxObject::invalidateRow");
+          gTreeRowInvalidated = true;
+          break;
+        }
     }
 
-    var gTreeColumnInvalidated = false;
+    function TreeInvalidatedHandlerHelper(aEvent, aStartRow, aEndRow,
-    function TreeColumnInvalidatedHandler(aEvent)
+                                          aStartCol, aEndCol, aCauseMsg)
     {
       var startRow = aEvent.getData("startrow");
-      is(startRow, null,
+      is(startRow, aStartRow,
-         "Wrong 'startrow' of 'treeInvalidated' event on InvalidateRow().");
+         "Wrong 'startrow' of 'treeInvalidated' event on " + aCauseMsg);
 
       var endRow = aEvent.getData("endrow");
-      is(endRow, null,
+      is(endRow, aEndRow,
-         "Wrong 'endrow' of 'treeInvalidated' event on InvalidateRow().");
+         "Wrong 'endrow' of 'treeInvalidated' event on " + aCauseMsg);
 
       var startCol = aEvent.getData("startcolumn");
-      is(startCol, 0,
+      is(startCol, aStartCol,
-         "Wrong 'startcolumn' of 'treeInvalidated' event on InvalidateRow().");
+         "Wrong 'startcolumn' of 'treeInvalidated' event on " + aCauseMsg);
 
       var endCol = aEvent.getData("endcolumn");
-      is(endCol, 0,
+      is(endCol, aEndCol,
-         "Wrong 'endcolumn' of 'treeInvalidated' event on InvalidateRow().");
+         "Wrong 'endcolumn' of 'treeInvalidated' event on " + aCauseMsg);
-
-      gTreeColumnInvalidated = true;
-    }
-
-    var gTreeRowInvalidated = false;
-    function TreeRowInvalidatedHandler(aEvent)
-    {
-      var startRow = aEvent.getData("startrow");
-      is(startRow, 1,
-         "Wrong 'startrow' of 'treeInvalidated' event on InvalidateColumn().");
-
-      var endRow = aEvent.getData("endrow");
-      is(endRow, 1,
-         "Wrong 'endrow' of 'treeInvalidated' event on InvalidateColumn().");
-
-      var startCol = aEvent.getData("startcolumn");
-      is(startCol, null,
-         "Wrong 'startcolumn' of 'treeInvalidated' event on InvalidateColumn().");
-
-      var endCol = aEvent.getData("endcolumn");
-      is(endCol, null,
-         "Wrong 'endcolumn' of 'treeInvalidated' event on InvalidateColumn().");
-
-      gTreeRowInvalidated = true;
     }
 
     var gNameChangedOnTreeRowInvalidated = false;
@@ -155,6 +154,9 @@
 
         ++this.mCount;
 
+        // We should get first six 'name changed' events on
+        //  nsITreeBoxObject::invalidateColumn when we update 0th column
+        // containing six rows.
         if (this.mCount == 6) {
           gNameChangedOnTreeColumnInvalidated = true;
 
@@ -166,8 +168,7 @@
           gNameChangedOnTreeRowInvalidated = true;
       },
 
-      mCount: 0,
+      mCount: 0
-      mType: ""
     };
 
     function CheckEvents()
@@ -175,22 +176,26 @@
       // If these fail then it doesn't mean actually events are not fired,
       // possibly setTimeout was executed earlier than events have been fired.
 
-      // DOM events
+      // nsITreeBoxObject::view
       ok(gTreeViewChanged,
          "TreeViewChanged event should have been fired.");
+
+      // nsITreeBoxObject::rowCountChanged
       ok(gTreeRowCountChanged,
          "TreeRowCountChanged event should have been fired.");
+
+      // nsITreeBoxObject::invalidateColumn
       ok(gTreeColumnInvalidated,
          "TreeInvalidated event should have been fired for InvalidateColumn().");
-      ok(gTreeRowInvalidated,
-         "TreeInvalidated event should have been fired for InvalidateRow().");
-
-      // A11y events
-      ok(gNameChangedOnTreeRowInvalidated,
-         "Wrong NameChanged events number on tree row invalidation.");
       ok(gNameChangedOnTreeColumnInvalidated,
          "Wrong NameChanged events number on tree column invalidation.");
 
+      // nsITreeBoxObject::invalidateRow
+      ok(gTreeRowInvalidated,
+         "TreeInvalidated event should have been fired for InvalidateRow().");
+      ok(gNameChangedOnTreeRowInvalidated,
+         "Wrong NameChanged events number on tree row invalidation.");
+
       // Remove DOM event listeners
       document.removeEventListener("TreeViewChanged",
                                    TreeViewChangedHandler, true);
@@ -225,7 +230,6 @@
                          getService(Components.interfaces.nsIObserverService);
       gObserverService.addObserver(gA11yEventObserver, "accessible-event",
                                    false);
-
       // Add DOM event listeners
       document.addEventListener("TreeViewChanged",
                                 TreeViewChangedHandler, true);

layout/xul/base/src/tree/src/nsTreeBodyFrame.cpp

@@ -742,12 +742,6 @@ nsTreeBodyFrame::InvalidateRange(PRInt32 aStart, PRInt32 aEnd)
   if (aStart == aEnd)
     return InvalidateRow(aStart);
 
-#ifdef ACCESSIBILITY
-  nsIPresShell *presShell = PresContext()->PresShell();
-  if (presShell->IsAccessibilityActive())
-    FireInvalidateEvent(aStart, aEnd, nsnull, nsnull);
-#endif
-
   PRInt32 last = GetLastVisibleRow();
   if (aStart > aEnd || aEnd < mTopRowIndex || aStart > last)
     return NS_OK;
@@ -758,6 +752,15 @@ nsTreeBodyFrame::InvalidateRange(PRInt32 aStart, PRInt32 aEnd)
   if (aEnd > last)
     aEnd = last;
 
+#ifdef ACCESSIBILITY
+  nsIPresShell *presShell = PresContext()->PresShell();
+  if (presShell->IsAccessibilityActive()) {
+    PRInt32 end =
+      mRowCount > 0 ? ((mRowCount <= aEnd) ? mRowCount - 1 : aEnd) : 0;
+    FireInvalidateEvent(aStart, end, nsnull, nsnull);
+  }
+#endif
+
   nsRect rangeRect(mInnerBox.x, mInnerBox.y+mRowHeight*(aStart-mTopRowIndex), mInnerBox.width, mRowHeight*(aEnd-aStart+1));
   nsIFrame::Invalidate(rangeRect, PR_FALSE);
 
@@ -777,12 +780,6 @@ nsTreeBodyFrame::InvalidateColumnRange(PRInt32 aStart, PRInt32 aEnd, nsITreeColu
   if (aStart == aEnd)
     return InvalidateCell(aStart, col);
 
-#ifdef ACCESSIBILITY
-  nsIPresShell *presShell = PresContext()->PresShell();
-  if (presShell->IsAccessibilityActive())
-    FireInvalidateEvent(aStart, aEnd, aCol, aCol);
-#endif
-
   PRInt32 last = GetLastVisibleRow();
   if (aStart > aEnd || aEnd < mTopRowIndex || aStart > last)
     return NS_OK;
@@ -793,6 +790,15 @@ nsTreeBodyFrame::InvalidateColumnRange(PRInt32 aStart, PRInt32 aEnd, nsITreeColu
   if (aEnd > last)
     aEnd = last;
 
+#ifdef ACCESSIBILITY
+  nsIPresShell *presShell = PresContext()->PresShell();
+  if (presShell->IsAccessibilityActive()) {
+    PRInt32 end =
+      mRowCount > 0 ? ((mRowCount <= aEnd) ? mRowCount - 1 : aEnd) : 0;
+    FireInvalidateEvent(aStart, end, aCol, aCol);
+  }
+#endif
+
   nsRect rangeRect;
   nsresult rv = col->GetRect(this, 
                              mInnerBox.y+mRowHeight*(aStart-mTopRowIndex),