Bug 1250506 - check if node is acceptable as a child before creating an accessible for it, r=davidb

accessible/base/TreeWalker.cpp

@@ -107,9 +107,14 @@ TreeWalker::Next(ChildrenIterator* aIter, Accessible** aAccesible,
   *aSkipSubtree = false;
 
   if (childEl) {
-    Accessible* accessible = mFlags & eWalkCache ?
-      mDoc->GetAccessible(childEl) :
-      GetAccService()->GetOrCreateAccessible(childEl, mContext, aSkipSubtree);
+    Accessible* accessible = nullptr;
+    if (mFlags & eWalkCache) {
+      accessible = mDoc->GetAccessible(childEl);
+    }
+    else if (mContext->IsAcceptableChild(childEl)) {
+      accessible = GetAccService()->
+        GetOrCreateAccessible(childEl, mContext, aSkipSubtree);
+    }
 
     // Ignore the accessible and its subtree if it was repositioned by means of
     // aria-owns.

accessible/base/nsAccessibilityService.cpp

@@ -1105,9 +1105,6 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
 
     newAcc = CreateAccessibleByFrameType(frame, content, aContext);
-    if (!aContext->IsAcceptableChild(newAcc))
-      return nullptr;
-
     document->BindToDocument(newAcc, nullptr);
     newAcc->AsTextLeaf()->SetText(text.mString);
     return newAcc;
@@ -1131,9 +1128,6 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
 
     newAcc = new HyperTextAccessibleWrap(content, document);
-    if (!aContext->IsAcceptableChild(newAcc))
-      return nullptr;
-
     document->BindToDocument(newAcc, aria::GetRoleMap(aNode));
     return newAcc;
   }
@@ -1291,10 +1285,9 @@ nsAccessibilityService::GetOrCreateAccessible(nsINode* aNode,
     }
   }
 
-  if (!newAcc || !aContext->IsAcceptableChild(newAcc))
-    return nullptr;
-
-  document->BindToDocument(newAcc, roleMapEntry);
+  if (newAcc) {
+    document->BindToDocument(newAcc, roleMapEntry);
+  }
   return newAcc;
 }
 
@@ -1414,13 +1407,7 @@ nsAccessibilityService::CreateAccessibleByType(nsIContent* aContent,
                                                DocAccessible* aDoc)
 {
   nsAutoString role;
-  for (const nsXBLBinding* binding = aContent->GetXBLBinding(); binding; binding = binding->GetBaseBinding()) {
-    nsIContent* bindingElm = binding->PrototypeBinding()->GetBindingElement();
-    bindingElm->GetAttr(kNameSpaceID_None, nsGkAtoms::role, role);
-    if (!role.IsEmpty())
-      break;
-  }
-
+  nsCoreUtils::XBLBindingRole(aContent, role);
   if (role.IsEmpty() || role.EqualsLiteral("none"))
     return nullptr;
 

accessible/base/nsCoreUtils.cpp

@@ -686,3 +686,15 @@ nsCoreUtils::DispatchAccEvent(RefPtr<nsIAccessibleEvent> event)
 
   obsService->NotifyObservers(event, NS_ACCESSIBLE_EVENT_TOPIC, nullptr);
 }
+
+void
+nsCoreUtils::XBLBindingRole(const nsIContent* aEl, nsAString& aRole)
+{
+  for (const nsXBLBinding* binding = aEl->GetXBLBinding(); binding;
+       binding = binding->GetBaseBinding()) {
+    nsIContent* bindingElm = binding->PrototypeBinding()->GetBindingElement();
+    bindingElm->GetAttr(kNameSpaceID_None, nsGkAtoms::role, aRole);
+    if (!aRole.IsEmpty())
+      break;
+  }
+}

accessible/base/nsCoreUtils.h

@@ -330,6 +330,11 @@ public:
    * Notify accessible event observers of an event.
    */
   static void DispatchAccEvent(RefPtr<nsIAccessibleEvent> aEvent);
+
+  /**
+   * Return a role attribute on XBL bindings of the element.
+   */
+  static void XBLBindingRole(const nsIContent* aEl, nsAString& aRole);
 };
 
 #endif

accessible/generic/Accessible.h

@@ -497,7 +497,8 @@ public:
   /**
    * Return true if the accessible is an acceptable child.
    */
-  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const { return true; }
+  virtual bool IsAcceptableChild(nsIContent* aEl) const
+    { return true; }
 
   /**
    * Returns text of accessible if accessible has text role otherwise empty

accessible/xul/XULColorPickerAccessible.cpp

@@ -130,10 +130,14 @@ XULColorPickerAccessible::AreItemsOperable() const
 }
 
 ////////////////////////////////////////////////////////////////////////////////
-// XULColorPickerAccessible: protected Accessible
+// XULColorPickerAccessible: Accessible
 
 bool
-XULColorPickerAccessible::IsAcceptableChild(Accessible* aPossibleChild) const
+XULColorPickerAccessible::IsAcceptableChild(nsIContent* aEl) const
 {
-  return roles::ALERT == aPossibleChild->Role();
+  nsAutoString role;
+  nsCoreUtils::XBLBindingRole(aEl, role);
+  return role.EqualsLiteral("xul:panel") &&
+    aEl->AttrValueIs(kNameSpaceID_None, nsGkAtoms::noautofocus,
+                     nsGkAtoms::_true, eCaseMatters);
 }

accessible/xul/XULColorPickerAccessible.h

@@ -48,7 +48,7 @@ public:
   virtual bool IsActiveWidget() const override;
   virtual bool AreItemsOperable() const override;
 
-  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const override;
+  virtual bool IsAcceptableChild(nsIContent* aEl) const override;
 };
 
 } // namespace a11y

accessible/xul/XULFormControlAccessible.cpp

@@ -165,7 +165,7 @@ XULButtonAccessible::ContainerWidget() const
 }
 
 bool
-XULButtonAccessible::IsAcceptableChild(Accessible* aPossibleChild) const
+XULButtonAccessible::IsAcceptableChild(nsIContent* aEl) const
 {
   // In general XUL button has not accessible children. Nevertheless menu
   // buttons can have button (@type="menu-button") and popup accessibles
@@ -173,17 +173,21 @@ XULButtonAccessible::IsAcceptableChild(Accessible* aPossibleChild) const
 
   // 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();
+  nsAutoString role;
+  nsCoreUtils::XBLBindingRole(aEl, role);
 
   // Get an accessible for menupopup or panel elements.
-  if (role == roles::MENUPOPUP)
+  if (role.EqualsLiteral("xul:menupopup")) {
     return true;
+  }
 
   // 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()->IsXULElement(nsGkAtoms::dropMarker))
+  if ((!role.EqualsLiteral("xul:button") &&
+       !role.EqualsLiteral("xul:toolbarbutton")) ||
+      aEl->IsXULElement(nsGkAtoms::dropMarker)) {
     return false;
+  }
 
   return mContent->AttrValueIs(kNameSpaceID_None, nsGkAtoms::type,
                                nsGkAtoms::menuButton, eCaseMatters);

accessible/xul/XULFormControlAccessible.h

@@ -50,7 +50,7 @@ public:
   virtual bool AreItemsOperable() const override;
   virtual Accessible* ContainerWidget() const override;
 
-  virtual bool IsAcceptableChild(Accessible* aPossibleChild) const override;
+  virtual bool IsAcceptableChild(nsIContent* aEl) const override;
 
 protected:
   virtual ~XULButtonAccessible();