Bug 1113389 - loading google creates accessibles without firing show events, r=tbsaunde

accessible/generic/Accessible-inl.h

@@ -67,14 +67,6 @@ Accessible::ScrollTo(uint32_t aHow) const
     nsCoreUtils::ScrollTo(mDoc->PresShell(), mContent, aHow);
 }
 
-inline bool
-Accessible::UpdateChildren()
-{
-  AutoTreeMutation mut(this);
-  InvalidateChildren();
-  return EnsureChildren();
-}
-
 } // namespace a11y
 } // namespace mozilla
 

accessible/generic/Accessible.cpp

@@ -2012,8 +2012,7 @@ Accessible::InvalidateChildren()
 {
   int32_t childCount = mChildren.Length();
   for (int32_t childIdx = 0; childIdx < childCount; childIdx++) {
-    Accessible* child = mChildren.ElementAt(childIdx);
-    child->UnbindFromParent();
+    mChildren.ElementAt(childIdx)->UnbindFromParent();
   }
 
   mEmbeddedObjCollector = nullptr;
@@ -2446,23 +2445,17 @@ Accessible::TestChildCache(Accessible* aCachedChild) const
 #endif
 }
 
-// Accessible public
-bool
+void
 Accessible::EnsureChildren()
 {
-  if (IsDefunct()) {
-    SetChildrenFlag(eChildrenUninitialized);
-    return true;
-  }
+  NS_ASSERTION(!IsDefunct(), "Caching children for defunct accessible!");
 
   if (!IsChildrenFlag(eChildrenUninitialized))
-    return false;
+    return;
 
   // State is embedded children until text leaf accessible is appended.
   SetChildrenFlag(eEmbeddedChildren); // Prevent reentry
-
   CacheChildren();
-  return false;
 }
 
 Accessible*

accessible/generic/Accessible.h

@@ -366,14 +366,9 @@ public:
     { mRoleMapEntry = aRoleMapEntry; }
 
   /**
-   * Update the children cache.
+   * Cache children if necessary.
    */
-  bool UpdateChildren();
-
-  /**
-   * Cache children if necessary. Return true if the accessible is defunct.
-   */
-  bool EnsureChildren();
+  void EnsureChildren();
 
   /**
    * Set the child count to -1 (unknown) and null out cached child pointers.
@@ -587,6 +582,7 @@ public:
   HyperTextAccessible* AsHyperText();
 
   bool IsHTMLBr() const { return mType == eHTMLBRType; }
+  bool IsHTMLCombobox() const { return mType == eHTMLComboboxType; }
   bool IsHTMLFileInput() const { return mType == eHTMLFileInputType; }
 
   bool IsHTMLListItem() const { return mType == eHTMLLiType; }
@@ -870,6 +866,19 @@ public:
   bool NeedsDOMUIEvent() const
     { return !(mStateFlags & eIgnoreDOMUIEvent); }
 
+  /**
+   * Get/set survivingInUpdate bit on child indicating that parent recollects
+   * its children.
+   */
+  bool IsSurvivingInUpdate() const { return mStateFlags & eSurvivingInUpdate; }
+  void SetSurvivingInUpdate(bool aIsSurviving)
+  {
+    if (aIsSurviving)
+      mStateFlags |= eSurvivingInUpdate;
+    else
+      mStateFlags &= ~eSurvivingInUpdate;
+  }
+
   /**
    * Return true if this accessible has a parent whose name depends on this
    * accessible.
@@ -953,8 +962,9 @@ protected:
     eGroupInfoDirty = 1 << 5, // accessible needs to update group info
     eSubtreeMutating = 1 << 6, // subtree is being mutated
     eIgnoreDOMUIEvent = 1 << 7, // don't process DOM UI events for a11y events
+    eSurvivingInUpdate = 1 << 8, // parent drops children to recollect them
 
-    eLastStateFlag = eIgnoreDOMUIEvent
+    eLastStateFlag = eSurvivingInUpdate
   };
 
   /**
@@ -1068,7 +1078,7 @@ protected:
   int32_t mIndexInParent;
 
   static const uint8_t kChildrenFlagsBits = 2;
-  static const uint8_t kStateFlagsBits = 8;
+  static const uint8_t kStateFlagsBits = 9;
   static const uint8_t kContextFlagsBits = 1;
   static const uint8_t kTypeBits = 6;
   static const uint8_t kGenericTypesBits = 13;

accessible/generic/DocAccessible.cpp

@@ -1308,19 +1308,10 @@ DocAccessible::ProcessInvalidationList()
   // children are recached.
   for (uint32_t idx = 0; idx < mInvalidationList.Length(); idx++) {
     nsIContent* content = mInvalidationList[idx];
-    Accessible* accessible = GetAccessible(content);
-    if (!accessible) {
+    if (!HasAccessible(content)) {
       Accessible* container = GetContainerAccessible(content);
-      if (container) {
-        container->UpdateChildren();
-        accessible = GetAccessible(content);
-      }
-    }
-
-    // Make sure the subtree is created.
-    if (accessible) {
-      AutoTreeMutation mut(accessible);
-      CacheChildrenInSubtree(accessible);
+      if (container)
+        UpdateTreeOnInsertion(container);
     }
   }
 
@@ -1633,8 +1624,6 @@ DocAccessible::ProcessContentInserted(Accessible* aContainer,
   if (!HasAccessible(aContainer->GetNode()))
     return;
 
-  bool containerNotUpdated = true;
-
   for (uint32_t idx = 0; idx < aInsertedContent->Length(); idx++) {
     // The container might be changed, for example, because of the subsequent
     // overlapping content insertion (i.e. other content was inserted between
@@ -1644,15 +1633,12 @@ DocAccessible::ProcessContentInserted(Accessible* aContainer,
     // Note, the inserted content might be not in tree at all at this point what
     // means there's no container. Ignore the insertion too.
 
-    Accessible* presentContainer =
+    Accessible* container =
       GetContainerAccessible(aInsertedContent->ElementAt(idx));
-    if (presentContainer != aContainer)
+    if (container != aContainer)
       continue;
 
-    if (containerNotUpdated) {
-      containerNotUpdated = false;
-
-      if (aContainer == this) {
+    if (container == this) {
       // If new root content has been inserted then update it.
       nsIContent* rootContent = nsCoreUtils::GetRoleContent(mDocumentNode);
       if (rootContent != mContent) {
@@ -1665,34 +1651,93 @@ DocAccessible::ProcessContentInserted(Accessible* aContainer,
       // there is no HTML body element.
     }
 
-      // XXX: Invalidate parent-child relations for container accessible and its
-      // children because there's no good way to find insertion point of new child
-      // accessibles into accessible tree. We need to invalidate children even
-      // there's no inserted accessibles in the end because accessible children
-      // are created while parent recaches child accessibles.
-      // XXX Group invalidation here may be redundant with invalidation in
-      // UpdateTree.
-      AutoTreeMutation mut(aContainer);
-      aContainer->InvalidateChildren();
-      CacheChildrenInSubtree(aContainer);
-    }
+    // HTML comboboxes have no-content list accessible as an intermidiate
+    // containing all options.
+    if (container->IsHTMLCombobox())
+      container = container->FirstChild();
 
-    UpdateTree(aContainer, aInsertedContent->ElementAt(idx), true);
+    // We have a DOM/layout change under the container accessible, and its tree
+    // might need an update. Since DOM/layout change of the element may affect
+    // on the accessibleness of adjacent elements (for example, insertion of
+    // extra HTML:body make the old body accessible) then we have to recache
+    // children of the container, and then fire show/hide events for a change.
+    UpdateTreeOnInsertion(container);
+    break;
   }
 }
 
 void
-DocAccessible::UpdateTree(Accessible* aContainer, nsIContent* aChildNode,
-                          bool aIsInsert)
+DocAccessible::UpdateTreeOnInsertion(Accessible* aContainer)
 {
-  uint32_t updateFlags = eNoAccessible;
+  for (uint32_t idx = 0; idx < aContainer->ContentChildCount(); idx++) {
+    Accessible* child = aContainer->ContentChildAt(idx);
+    child->SetSurvivingInUpdate(true);
+   }
 
+  AutoTreeMutation mut(aContainer);
+  aContainer->InvalidateChildren();
+  aContainer->EnsureChildren();
+
+  nsRefPtr<AccReorderEvent> reorderEvent = new AccReorderEvent(aContainer);
+
+  uint32_t updateFlags = eNoAccessible;
+  for (uint32_t idx = 0; idx < aContainer->ContentChildCount(); idx++) {
+    Accessible* child = aContainer->ContentChildAt(idx);
+    if (child->IsSurvivingInUpdate()) {
+      child->SetSurvivingInUpdate(false);
+      continue;
+    }
+
+    // A new child has been created, update its tree.
+#ifdef A11Y_LOG
+    if (logging::IsEnabled(logging::eTree)) {
+      logging::MsgBegin("TREE", "process content insertion");
+      logging::Node("container", aContainer->GetNode());
+      logging::Node("child", child->GetContent());
+      logging::Address("child", child);
+      logging::MsgEnd();
+    }
+#endif
+
+    updateFlags |= UpdateTreeInternal(child, true, reorderEvent);
+  }
+
+  // Content insertion/removal is not cause of accessible tree change.
+  if (updateFlags == eNoAccessible)
+    return;
+
+  // Check to see if change occurred inside an alert, and fire an EVENT_ALERT
+  // if it did.
+  if (!(updateFlags & eAlertAccessible)) {
+    // XXX: tree traversal is perf issue, accessible should know if they are
+    // children of alert accessible to avoid this.
+    Accessible* ancestor = aContainer;
+    while (ancestor) {
+      if (ancestor->ARIARole() == roles::ALERT) {
+        FireDelayedEvent(nsIAccessibleEvent::EVENT_ALERT, ancestor);
+        break;
+      }
+
+      // Don't climb above this document.
+      if (ancestor == this)
+        break;
+
+      ancestor = ancestor->Parent();
+    }
+  }
+
+  MaybeNotifyOfValueChange(aContainer);
+  FireDelayedEvent(reorderEvent);
+}
+
+void
+DocAccessible::UpdateTreeOnRemoval(Accessible* aContainer, nsIContent* aChildNode)
+{
   // If child node is not accessible then look for its accessible children.
   Accessible* child = GetAccessible(aChildNode);
 #ifdef A11Y_LOG
   if (logging::IsEnabled(logging::eTree)) {
-    logging::MsgBegin("TREE", "process content %s",
-                      (aIsInsert ? "insertion" : "removal"));
+    logging::MsgBegin("TREE", "process content removal");
     logging::Node("container", aContainer->GetNode());
     logging::Node("child", aChildNode);
     if (child)
@@ -1704,25 +1749,20 @@ DocAccessible::UpdateTree(Accessible* aContainer, nsIContent* aChildNode,
   }
 #endif
 
+  uint32_t updateFlags = eNoAccessible;
   nsRefPtr<AccReorderEvent> reorderEvent = new AccReorderEvent(aContainer);
   AutoTreeMutation mut(aContainer);
 
   if (child) {
-    updateFlags |= UpdateTreeInternal(child, aIsInsert, reorderEvent);
-  } else {
-    if (aIsInsert) {
-      TreeWalker walker(aContainer, aChildNode, TreeWalker::eWalkCache);
-
-      while ((child = walker.NextChild()))
-        updateFlags |= UpdateTreeInternal(child, aIsInsert, reorderEvent);
+    updateFlags |= UpdateTreeInternal(child, false, reorderEvent);
   } else {
     // aChildNode may not coorespond to a particular accessible, to handle
     // this we go through all the children of aContainer.  Then if a child
     // has aChildNode as an ancestor, or does not have the node for
     // aContainer as an ancestor remove that child of aContainer.  Note that
-      // when we are called aChildNode may already have been removed
-      // from the DOM so we can't expect it to have a parent or what was it's
-      // parent to have it as a child.
+    // when we are called aChildNode may already have been removed from the DOM
+    // so we can't expect it to have a parent or what was it's parent to have
+    // it as a child.
     nsINode* containerNode = aContainer->GetNode();
     for (uint32_t idx = 0; idx < aContainer->ContentChildCount();) {
       Accessible* child = aContainer->ContentChildAt(idx);
@@ -1746,36 +1786,12 @@ DocAccessible::UpdateTree(Accessible* aContainer, nsIContent* aChildNode,
       }
     }
   }
-  }
 
   // Content insertion/removal is not cause of accessible tree change.
   if (updateFlags == eNoAccessible)
     return;
 
-  // Check to see if change occurred inside an alert, and fire an EVENT_ALERT
-  // if it did.
-  if (aIsInsert && !(updateFlags & eAlertAccessible)) {
-    // XXX: tree traversal is perf issue, accessible should know if they are
-    // children of alert accessible to avoid this.
-    Accessible* ancestor = aContainer;
-    while (ancestor) {
-      if (ancestor->ARIARole() == roles::ALERT) {
-        FireDelayedEvent(nsIAccessibleEvent::EVENT_ALERT, ancestor);
-        break;
-      }
-
-      // Don't climb above this document.
-      if (ancestor == this)
-        break;
-
-      ancestor = ancestor->Parent();
-    }
-  }
-
   MaybeNotifyOfValueChange(aContainer);
-
-  // Fire reorder event so the MSAA clients know the children have changed. Also
-  // the event is used internally by MSAA layer.
   FireDelayedEvent(reorderEvent);
 }
 

accessible/generic/DocAccessible.h

@@ -321,7 +321,7 @@ public:
   {
     // Update the whole tree of this document accessible when the container is
     // null (document element is removed).
-    UpdateTree((aContainer ? aContainer : this), aChildNode, false);
+    UpdateTreeOnRemoval((aContainer ? aContainer : this), aChildNode);
   }
   void ContentRemoved(nsIContent* aContainerNode, nsIContent* aChildNode)
   {
@@ -465,13 +465,17 @@ protected:
   void ProcessInvalidationList();
 
   /**
-   * Update the accessible tree for content insertion or removal.
+   * Update the tree on content insertion.
    */
-  void UpdateTree(Accessible* aContainer, nsIContent* aChildNode,
-                  bool aIsInsert);
+  void UpdateTreeOnInsertion(Accessible* aContainer);
 
   /**
-   * Helper for UpdateTree() method. Go down to DOM subtree and updates
+   * Update the accessible tree for content removal.
+   */
+  void UpdateTreeOnRemoval(Accessible* aContainer, nsIContent* aChildNode);
+
+  /**
+   * Helper for UpdateTreeOn methods. Go down to DOM subtree and updates
    * accessible tree. Return one of these flags.
    */
   enum EUpdateTreeFlags {

accessible/html/HTMLSelectAccessible.cpp

@@ -359,6 +359,7 @@ HTMLComboboxAccessible::
   HTMLComboboxAccessible(nsIContent* aContent, DocAccessible* aDoc) :
   AccessibleWrap(aContent, aDoc)
 {
+  mType = eHTMLComboboxType;
   mGenericTypes |= eCombobox;
 }
 

accessible/tests/mochitest/events/test_mutation.html

@@ -336,6 +336,28 @@
       }
     }
 
+    function insertReferredElm(aContainerID)
+    {
+      this.containerNode = getNode(aContainerID);
+
+      this.eventSeq = [
+        new invokerChecker(EVENT_SHOW, function(aNode) { return aNode.lastChild; }, this.containerNode),
+        new invokerChecker(EVENT_SHOW, function(aNode) { return aNode.firstChild; }, this.containerNode),
+        new invokerChecker(EVENT_REORDER, this.containerNode)
+      ];
+
+      this.invoke = function insertReferredElm_invoke()
+      {
+        this.containerNode.innerHTML =
+          "<span id='insertReferredElms_span'></span><input aria-labelledby='insertReferredElms_span'>";
+      }
+
+      this.getID = function insertReferredElm_getID()
+      {
+        return "insert inaccessible element and then insert referring element to make it accessible";
+      }
+    }
+
     /**
      * Target getters.
      */
@@ -343,6 +365,10 @@
     {
       return [aNode.firstChild];
     }
+    function getLastChild(aNode)
+    {
+      return [aNode.lastChild];
+    }
 
     function getNEnsureFirstChild(aNode)
     {
@@ -457,6 +483,7 @@
       gQueue.push(new test2("testContainer", "testContainer2"));
       gQueue.push(new test2("testContainer", "testNestedContainer"));
       gQueue.push(new test3("testContainer"));
+      gQueue.push(new insertReferredElm("testContainer3"));
 
       gQueue.invoke(); // Will call SimpleTest.finish();
     }
@@ -516,5 +543,6 @@
     <div id="testNestedContainer"></div>
   </div>
   <div id="testContainer2"></div>
+  <div id="testContainer3"></div>
 </body>
 </html>

accessible/tests/mochitest/tree/test_select.html

@@ -86,7 +86,7 @@
                     ]
                   },
                 ]
-},
+              },
               {
                 role: ROLE_COMBOBOX_OPTION,
                 children: [

accessible/tests/mochitest/treeupdate/test_optgroup.html

@@ -21,6 +21,7 @@
     {
       this.selectNode = getNode(aID);
       this.select = getAccessible(this.selectNode);
+      this.selectList = this.select.firstChild;
 
       this.invoke = function addOptGroup_invoke()
       {
@@ -39,7 +40,7 @@
       }
 
       this.eventSeq = [
-        new invokerChecker(EVENT_REORDER, this.select)
+        new invokerChecker(EVENT_REORDER, this.selectList)
       ];
 
       this.finalCheck = function addOptGroup_finalCheck()

accessible/tests/mochitest/treeupdate/test_select.html

@@ -21,6 +21,7 @@
     {
       this.selectNode = getNode(aID);
       this.select = getAccessible(this.selectNode);
+      this.selectList = this.select.firstChild;
 
       this.invoke = function addOptions_invoke()
       {
@@ -34,7 +35,7 @@
       }
 
       this.eventSeq = [
-        new invokerChecker(EVENT_REORDER, this.select)
+        new invokerChecker(EVENT_REORDER, this.selectList)
       ];
 
       this.finalCheck = function addOptions_finalCheck()