bug 888981 - add Accessible::IsAcceptableChild() r=surkov

this patch does several things
- makes DocAccessible::BindToDocument return void the only case in which
  it would "fail" was when passed null, and we know none of the callers
do that.
- adds Accessible::IsAcceptableChild() which returns true if its ok for
  the possible child to be a child of this accessible.
- replace several over rides of Accessible::CacheChildren() that are
  only to prevent certain types of children to over rides of
Accessible::IsAcceptableChild()

accessible/src/base/nsAccessibilityService.cpp

@@ -886,12 +886,12 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
 
     newAcc = CreateAccessibleByFrameType(frame, content, aContext);
-    if (document->BindToDocument(newAcc, nullptr)) {
-      newAcc->AsTextLeaf()->SetText(text);
-      return newAcc;
-    }
+    if (!aContext->IsAcceptableChild(newAcc))
+      return nullptr;
 
-    return nullptr;
+    document->BindToDocument(newAcc, nullptr);
+    newAcc->AsTextLeaf()->SetText(text);
+    return newAcc;
   }
 
   bool isHTML = content->IsHTML();
@@ -913,9 +913,11 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
 
     newAcc = new HyperTextAccessibleWrap(content, document);
-    if (document->BindToDocument(newAcc, aria::GetRoleMap(aNode)))
-      return newAcc;
-    return nullptr;
+    if (!aContext->IsAcceptableChild(newAcc))
+      return nullptr;
+
+    document->BindToDocument(newAcc, aria::GetRoleMap(aNode));
+    return newAcc;
   }
 
   nsRoleMapEntry* roleMapEntry = aria::GetRoleMap(aNode);
@@ -1044,7 +1046,11 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
   }
 
-  return document->BindToDocument(newAcc, roleMapEntry) ? newAcc : nullptr;
+  if (!newAcc || !aContext->IsAcceptableChild(newAcc))
+    return nullptr;
+
+  document->BindToDocument(newAcc, roleMapEntry);
+  return newAcc;
 }
 
 ////////////////////////////////////////////////////////////////////////////////

accessible/src/generic/Accessible.h

@@ -433,6 +433,11 @@ public:
    */
   virtual bool CanHaveAnonChildren();
 
+  /**
+   * Return true if the accessible is an acceptable child.
+   */
+  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const { return true; }
+
   /**
    * Returns text of accessible if accessible has text role otherwise empty
    * string.

accessible/src/generic/DocAccessible.cpp

@@ -1309,13 +1309,10 @@ DocAccessible::GetAccessibleOrDescendant(nsINode* aNode) const
   return nullptr;
 }
 
-bool
+void
 DocAccessible::BindToDocument(Accessible* aAccessible,
                               nsRoleMapEntry* aRoleMapEntry)
 {
-  if (!aAccessible)
-    return false;
-
   // Put into DOM node cache.
   if (aAccessible->IsNodeMapEntry())
     mNodeToAccessibleMap.Put(aAccessible->GetNode(), aAccessible);
@@ -1328,8 +1325,6 @@ DocAccessible::BindToDocument(Accessible* aAccessible,
   nsIContent* content = aAccessible->GetContent();
   if (content && content->IsElement())
     AddDependentIDsFor(content->AsElement());
-
-  return true;
 }
 
 void

accessible/src/generic/DocAccessible.h

@@ -274,7 +274,7 @@ public:
    * @param  aRoleMapEntry  [in] the role map entry role the ARIA role or nullptr
    *                          if none
    */
-  bool BindToDocument(Accessible* aAccessible, nsRoleMapEntry* aRoleMapEntry);
+  void BindToDocument(Accessible* aAccessible, nsRoleMapEntry* aRoleMapEntry);
 
   /**
    * Remove from document and shutdown the given accessible.

accessible/src/html/HTMLFormControlAccessible.cpp

@@ -12,7 +12,6 @@
 #include "Relation.h"
 #include "Role.h"
 #include "States.h"
-#include "TreeWalker.h"
 
 #include "nsContentList.h"
 #include "nsCxPusher.h"

accessible/src/html/HTMLImageMapAccessible.cpp

@@ -112,8 +112,7 @@ HTMLImageMapAccessible::UpdateChildAreas(bool aDoFireEvents)
     Accessible* area = mChildren.SafeElementAt(idx);
     if (!area || area->GetContent() != areaContent) {
       nsRefPtr<Accessible> area = new HTMLAreaAccessible(areaContent, mDoc);
-      if (!mDoc->BindToDocument(area, aria::GetRoleMap(areaContent)))
-        break;
+      mDoc->BindToDocument(area, aria::GetRoleMap(areaContent));
 
       if (!InsertChildAt(idx, area)) {
         mDoc->UnbindFromDocument(area);

accessible/src/html/HTMLListAccessible.cpp

@@ -51,8 +51,7 @@ HTMLLIAccessible::
   nsBlockFrame* blockFrame = do_QueryFrame(GetFrame());
   if (blockFrame && blockFrame->HasBullet()) {
     mBullet = new HTMLListBulletAccessible(mContent, mDoc);
-    if (!Document()->BindToDocument(mBullet, nullptr))
-      mBullet = nullptr;
+    Document()->BindToDocument(mBullet, nullptr);
   }
 }
 
@@ -112,9 +111,8 @@ HTMLLIAccessible::UpdateBullet(bool aHasBullet)
   DocAccessible* document = Document();
   if (aHasBullet) {
     mBullet = new HTMLListBulletAccessible(mContent, mDoc);
-    if (document->BindToDocument(mBullet, nullptr)) {
-      InsertChildAt(0, mBullet);
-    }
+    document->BindToDocument(mBullet, nullptr);
+    InsertChildAt(0, mBullet);
   } else {
     RemoveChild(mBullet);
     document->UnbindFromDocument(mBullet);

accessible/src/html/HTMLSelectAccessible.cpp

@@ -410,8 +410,7 @@ HTMLComboboxAccessible::CacheChildren()
       new HTMLComboboxListAccessible(mParent, mContent, mDoc);
 
     // Initialize and put into cache.
-    if (!Document()->BindToDocument(mListAccessible, nullptr))
-      return;
+    Document()->BindToDocument(mListAccessible, nullptr);
   }
 
   if (AppendChild(mListAccessible)) {

accessible/src/xul/XULColorPickerAccessible.cpp

@@ -11,7 +11,6 @@
 #include "DocAccessible.h"
 #include "Role.h"
 #include "States.h"
-#include "TreeWalker.h"
 
 #include "nsIDOMElement.h"
 #include "nsMenuPopupFrame.h"
@@ -136,24 +135,8 @@ XULColorPickerAccessible::AreItemsOperable() const
 ////////////////////////////////////////////////////////////////////////////////
 // XULColorPickerAccessible: protected Accessible
 
-void
-XULColorPickerAccessible::CacheChildren()
+bool
+XULColorPickerAccessible::IsAcceptableChild(Accessible* aPossibleChild) const
 {
-  NS_ENSURE_TRUE_VOID(mDoc);
-
-  TreeWalker walker(this, mContent);
-
-  Accessible* child = nullptr;
-  while ((child = walker.NextChild())) {
-    uint32_t role = child->Role();
-
-    // Get an accessible for menupopup or panel elements.
-    if (role == roles::ALERT) {
-      AppendChild(child);
-      return;
-    }
-
-    // Unbind rejected accessibles from the document.
-    Document()->UnbindFromDocument(child);
-  }
+  return roles::ALERT == aPossibleChild->Role();
 }

accessible/src/xul/XULColorPickerAccessible.h

@@ -48,10 +48,7 @@ public:
   virtual bool IsActiveWidget() const;
   virtual bool AreItemsOperable() const;
 
-protected:
-
-  // Accessible
-  virtual void CacheChildren();
+  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const MOZ_OVERRIDE;
 };
 
 } // namespace a11y

accessible/src/xul/XULElementAccessibles.cpp

@@ -46,14 +46,11 @@ XULLabelAccessible::
   nsTextBoxFrame* textBoxFrame = do_QueryFrame(mContent->GetPrimaryFrame());
   if (textBoxFrame) {
     mValueTextLeaf = new XULLabelTextLeafAccessible(mContent, mDoc);
-    if (mDoc->BindToDocument(mValueTextLeaf, nullptr)) {
-      nsAutoString text;
-      textBoxFrame->GetCroppedTitle(text);
-      mValueTextLeaf->SetText(text);
-      return;
-    }
+    mDoc->BindToDocument(mValueTextLeaf, nullptr);
 
-    mValueTextLeaf = nullptr;
+    nsAutoString text;
+    textBoxFrame->GetCroppedTitle(text);
+    mValueTextLeaf->SetText(text);
   }
 }
 

accessible/src/xul/XULFormControlAccessible.cpp

@@ -160,11 +160,8 @@ XULButtonAccessible::ContainerWidget() const
   return nullptr;
 }
 
-////////////////////////////////////////////////////////////////////////////////
-// XULButtonAccessible: Accessible protected
-
-void
-XULButtonAccessible::CacheChildren()
+bool
+XULButtonAccessible::IsAcceptableChild(Accessible* aPossibleChild) const
 {
   // In general XUL button has not accessible children. Nevertheless menu
   // buttons can have button (@type="menu-button") and popup accessibles
@@ -172,41 +169,20 @@ XULButtonAccessible::CacheChildren()
 
   // XXX: no children until the button is menu button. Probably it's not
   // totally correct but in general AT wants to have leaf buttons.
+  roles::Role role = aPossibleChild->Role();
 
-  bool isMenuButton = mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
-                                            nsGkAtoms::menuButton, eCaseMatters);
+  // Get an accessible for menupopup or panel elements.
+  if (role == roles::MENUPOPUP)
+    return true;
 
-  Accessible* menupopup = nullptr;
-  Accessible* button = nullptr;
+  // Button type="menu-button" contains a real button. Get an accessible
+  // for it. Ignore dropmarker button which is placed as a last child.
+  if (role != roles::PUSHBUTTON ||
+      aPossibleChild->GetContent()->Tag() == nsGkAtoms::dropMarker)
+    return false;
 
-  TreeWalker walker(this, mContent);
-
-  Accessible* child = nullptr;
-  while ((child = walker.NextChild())) {
-    roles::Role role = child->Role();
-
-    if (role == roles::MENUPOPUP) {
-      // Get an accessible for menupopup or panel elements.
-      menupopup = child;
-
-    } else if (isMenuButton && role == roles::PUSHBUTTON) {
-      // Button type="menu-button" contains a real button. Get an accessible
-      // for it. Ignore dropmarker button which is placed as a last child.
-      button = child;
-      break;
-
-    } else {
-      // Unbind rejected accessible from document.
-      Document()->UnbindFromDocument(child);
-    }
-  }
-
-  if (!menupopup)
-    return;
-
-  AppendChild(menupopup);
-  if (button)
-    AppendChild(button);
+  return mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
+                               nsGkAtoms::menuButton, eCaseMatters);
 }
 
 ////////////////////////////////////////////////////////////////////////////////

accessible/src/xul/XULFormControlAccessible.h

@@ -52,11 +52,9 @@ public:
   virtual bool AreItemsOperable() const;
   virtual Accessible* ContainerWidget() const;
 
+  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const MOZ_OVERRIDE;
+
 protected:
-
-  // Accessible
-  virtual void CacheChildren();
-
   // XULButtonAccessible
   bool ContainsMenu();
 };

accessible/src/xul/XULTreeAccessible.cpp

@@ -543,10 +543,8 @@ XULTreeAccessible::GetTreeItemAccessible(int32_t aRow)
   nsRefPtr<Accessible> treeItem = CreateTreeItemAccessible(aRow);
   if (treeItem) {
     mAccessibleCache.Put(key, treeItem);
-    if (Document()->BindToDocument(treeItem, nullptr))
-      return treeItem;
-
-    mAccessibleCache.Remove(key);
+    Document()->BindToDocument(treeItem, nullptr);
+    return treeItem;
   }
 
   return nullptr;

accessible/src/xul/XULTreeGridAccessible.cpp

@@ -398,15 +398,9 @@ XULTreeGridRowAccessible::GetCellAccessible(nsITreeColumn* aColumn)
   nsRefPtr<Accessible> cell =
     new XULTreeGridCellAccessibleWrap(mContent, mDoc, this, mTree,
                                       mTreeView, mRow, aColumn);
-  if (cell) {
-    mAccessibleCache.Put(key, cell);
-    if (Document()->BindToDocument(cell, nullptr))
-      return cell;
-
-    mAccessibleCache.Remove(key);
-  }
-
-  return nullptr;
+  mAccessibleCache.Put(key, cell);
+  Document()->BindToDocument(cell, nullptr);
+  return cell;
 }
 
 void

content/base/src/nsGkAtomList.h

@@ -317,6 +317,7 @@ GK_ATOM(drop, "drop")
 GK_ATOM(dropAfter, "dropAfter")
 GK_ATOM(dropBefore, "dropBefore")
 GK_ATOM(dropOn, "dropOn")
+GK_ATOM(dropMarker, "dropmarker")
 GK_ATOM(dt, "dt")
 GK_ATOM(editable, "editable")
 GK_ATOM(editing, "editing")