Title: [284606] trunk/Source/WebCore
Revision
284606
Author
commit-qu...@webkit.org
Date
2021-10-21 09:27:29 -0700 (Thu, 21 Oct 2021)

Log Message

AX: Any addition of children should funnel through AccessibilityObject::addChild
https://bugs.webkit.org/show_bug.cgi?id=231914

Patch by Tyler Wilcock <tyle...@apple.com> on 2021-10-21
Reviewed by Chris Fleizach.

All addition of children now goes through
AccessibilityObject::addChild. This is good for two reasons:

1. It ensures we aren't inserting ignored elements into the tree.
`insertChild` (downstream of `addChild`) checks this. Prior to this
patch, there were cases where we could insert ignored children into the
tree because no check was made.

2. We can reliably set state on the child based on the state of the
parent at insertion time. For example, children can set a flag if
any of their ancestors have an application or document role, which can
be useful for some AX clients.

* accessibility/AccessibilityARIAGrid.cpp:
(WebCore::AccessibilityARIAGrid::addTableCellChild):
(WebCore::AccessibilityARIAGrid::addChildren):
* accessibility/AccessibilityListBox.cpp:
(WebCore::AccessibilityListBox::addChildren):
* accessibility/AccessibilityMenuList.cpp:
(WebCore::AccessibilityMenuList::addChildren):
* accessibility/AccessibilityMenuListPopup.cpp:
(WebCore::AccessibilityMenuListPopup::addChildren):
* accessibility/AccessibilityObject.cpp:
(WebCore::isAutofillButton):
(WebCore::isTableComponent):
(WebCore::AccessibilityObject::insertChild):
* accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::addImageMapChildren):
(WebCore::AccessibilityRenderObject::addTextFieldChildren):
(WebCore::AccessibilityRenderObject::addRemoteSVGChildren):
* accessibility/AccessibilityScrollView.cpp:
(WebCore::AccessibilityScrollView::addChildScrollbar):
* accessibility/AccessibilitySlider.cpp:
(WebCore::AccessibilitySlider::addChildren):
* accessibility/AccessibilitySpinButton.cpp:
(WebCore::AccessibilitySpinButton::addChildren):
* accessibility/AccessibilityTable.cpp:
(WebCore::AccessibilityTable::addChildren):
(WebCore::AccessibilityTable::addTableCellChild):
* accessibility/AccessibilityTableColumn.cpp:
(WebCore::AccessibilityTableColumn::addChildren):
* accessibility/AccessibilityTableHeaderContainer.cpp:
(WebCore::AccessibilityTableHeaderContainer::addChildren):
* accessibility/AccessibilityTableRow.cpp:
(WebCore::AccessibilityTableRow::addChildren):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (284605 => 284606)


--- trunk/Source/WebCore/ChangeLog	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/ChangeLog	2021-10-21 16:27:29 UTC (rev 284606)
@@ -1,3 +1,56 @@
+2021-10-21  Tyler Wilcock  <tyle...@apple.com>
+
+        AX: Any addition of children should funnel through AccessibilityObject::addChild
+        https://bugs.webkit.org/show_bug.cgi?id=231914
+
+        Reviewed by Chris Fleizach.
+
+        All addition of children now goes through
+        AccessibilityObject::addChild. This is good for two reasons:
+
+        1. It ensures we aren't inserting ignored elements into the tree.
+        `insertChild` (downstream of `addChild`) checks this. Prior to this
+        patch, there were cases where we could insert ignored children into the
+        tree because no check was made.
+
+        2. We can reliably set state on the child based on the state of the
+        parent at insertion time. For example, children can set a flag if
+        any of their ancestors have an application or document role, which can
+        be useful for some AX clients.
+
+        * accessibility/AccessibilityARIAGrid.cpp:
+        (WebCore::AccessibilityARIAGrid::addTableCellChild):
+        (WebCore::AccessibilityARIAGrid::addChildren):
+        * accessibility/AccessibilityListBox.cpp:
+        (WebCore::AccessibilityListBox::addChildren):
+        * accessibility/AccessibilityMenuList.cpp:
+        (WebCore::AccessibilityMenuList::addChildren):
+        * accessibility/AccessibilityMenuListPopup.cpp:
+        (WebCore::AccessibilityMenuListPopup::addChildren):
+        * accessibility/AccessibilityObject.cpp:
+        (WebCore::isAutofillButton):
+        (WebCore::isTableComponent):
+        (WebCore::AccessibilityObject::insertChild):
+        * accessibility/AccessibilityRenderObject.cpp:
+        (WebCore::AccessibilityRenderObject::addImageMapChildren):
+        (WebCore::AccessibilityRenderObject::addTextFieldChildren):
+        (WebCore::AccessibilityRenderObject::addRemoteSVGChildren):
+        * accessibility/AccessibilityScrollView.cpp:
+        (WebCore::AccessibilityScrollView::addChildScrollbar):
+        * accessibility/AccessibilitySlider.cpp:
+        (WebCore::AccessibilitySlider::addChildren):
+        * accessibility/AccessibilitySpinButton.cpp:
+        (WebCore::AccessibilitySpinButton::addChildren):
+        * accessibility/AccessibilityTable.cpp:
+        (WebCore::AccessibilityTable::addChildren):
+        (WebCore::AccessibilityTable::addTableCellChild):
+        * accessibility/AccessibilityTableColumn.cpp:
+        (WebCore::AccessibilityTableColumn::addChildren):
+        * accessibility/AccessibilityTableHeaderContainer.cpp:
+        (WebCore::AccessibilityTableHeaderContainer::addChildren):
+        * accessibility/AccessibilityTableRow.cpp:
+        (WebCore::AccessibilityTableRow::addChildren):
+
 2021-10-21  Antti Koivisto  <an...@apple.com>
 
         Move Style::Resolver::State out of header

Modified: trunk/Source/WebCore/accessibility/AccessibilityARIAGrid.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityARIAGrid.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityARIAGrid.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -67,14 +67,7 @@
     
     row.setRowIndex((int)m_rows.size());
     m_rows.append(&row);
-
-    // Try adding the row if it's not ignoring accessibility,
-    // otherwise add its children (the cells) as the grid's children.
-    if (!row.accessibilityIsIgnored())
-        m_children.append(&row);
-    else
-        m_children.appendVector(row.children());
-
+    addChild(&row);
     appendedRows.add(&row);
     return true;
 }
@@ -142,13 +135,10 @@
         column.setColumnIndex(i);
         column.setParent(this);
         m_columns.append(&column);
-        if (!column.accessibilityIsIgnored())
-            m_children.append(&column);
+        addChild(&column);
     }
 
-    auto* headerContainerObject = headerContainer();
-    if (headerContainerObject && !headerContainerObject->accessibilityIsIgnored())
-        m_children.append(headerContainerObject);
+    addChild(headerContainer());
 }
     
 } // namespace WebCore

Modified: trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityListBox.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -73,11 +73,8 @@
 
     m_haveChildren = true;
 
-    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems()) {
-        AccessibilityObject* listOption = listBoxOptionAccessibilityObject(listItem);
-        if (listOption && !listOption->accessibilityIsIgnored())
-            m_children.append(listOption);
-    }
+    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems())
+        addChild(listBoxOptionAccessibilityObject(listItem));
 }
 
 void AccessibilityListBox::setSelectedChildren(const AccessibilityChildrenVector& children)

Modified: trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityMenuList.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -82,8 +82,7 @@
     }
 
     m_haveChildren = true;
-    m_children.append(list);
-
+    addChild(list);
     list->addChildren();
 }
 

Modified: trunk/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityMenuListPopup.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -96,11 +96,8 @@
 
     m_haveChildren = true;
 
-    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems()) {
-        // FIXME: Why does AccessibilityListBox::addChildren check accessibilityIsIgnored but this does not?
-        if (auto option = menuListOptionAccessibilityObject(listItem))
-            m_children.append(option);
-    }
+    for (const auto& listItem : downcast<HTMLSelectElement>(*selectNode).listItems())
+        addChild(menuListOptionAccessibilityObject(listItem));
 }
 
 void AccessibilityMenuListPopup::childrenChanged()

Modified: trunk/Source/WebCore/accessibility/AccessibilityObject.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityObject.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -483,7 +483,14 @@
     if (object)
         results.append(object);
 }
-    
+
+#ifndef NDEBUG
+static bool isTableComponent(AXCoreObject& axObject)
+{
+    return axObject.isTable() || axObject.isTableColumn() || axObject.isTableRow() || axObject.isTableCell();
+}
+#endif
+
 void AccessibilityObject::insertChild(AXCoreObject* child, unsigned index)
 {
     if (!child)
@@ -516,7 +523,9 @@
         for (size_t i = 0; i < length; ++i)
             m_children.insert(index + i, children[i]);
     } else {
-        ASSERT(child->parentObject() == this);
+        // Table component child-parent relationships often don't line up properly, hence the need for methods
+        // like parentTable() and parentRow(). Exclude them from this ASSERT.
+        ASSERT((!isTableComponent(*child) && !isTableComponent(*this)) ? child->parentObject() == this : true);
         m_children.insert(index, child);
     }
     

Modified: trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityRenderObject.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -3283,7 +3283,7 @@
         areaObject.setHTMLMapElement(map);
         areaObject.setParent(this);
         if (!areaObject.accessibilityIsIgnored())
-            m_children.append(&areaObject);
+            addChild(&areaObject);
         else
             axObjectCache()->remove(areaObject.objectID());
     }
@@ -3316,7 +3316,7 @@
     auto& axSpinButton = downcast<AccessibilitySpinButton>(*axObjectCache()->create(AccessibilityRole::SpinButton));
     axSpinButton.setSpinButtonElement(downcast<SpinButtonElement>(spinButtonElement));
     axSpinButton.setParent(this);
-    m_children.append(&axSpinButton);
+    addChild(&axSpinButton);
 }
     
 bool AccessibilityRenderObject::isSVGImage() const
@@ -3380,12 +3380,7 @@
     // In order to connect the AX hierarchy from the SVG root element from the loaded resource
     // the parent must be set, because there's no other way to get back to who created the image.
     root->setParent(this);
-    
-    if (root->accessibilityIsIgnored()) {
-        for (const auto& child : root->children())
-            m_children.append(child);
-    } else
-        m_children.append(root);
+    addChild(root);
 }
 
 void AccessibilityRenderObject::addCanvasChildren()

Modified: trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityScrollView.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -163,7 +163,7 @@
 
     auto& scrollBarObject = downcast<AccessibilityScrollbar>(*cache->getOrCreate(scrollbar));
     scrollBarObject.setParent(this);
-    m_children.append(&scrollBarObject);
+    addChild(&scrollBarObject);
     return &scrollBarObject;
 }
         

Modified: trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilitySlider.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -100,7 +100,7 @@
     if (thumb.accessibilityIsIgnored())
         cache->remove(thumb.objectID());
     else
-        m_children.append(&thumb);
+        addChild(&thumb);
 }
 
 const AtomString& AccessibilitySlider::getAttribute(const QualifiedName& attribute) const

Modified: trunk/Source/WebCore/accessibility/AccessibilitySpinButton.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilitySpinButton.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilitySpinButton.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -91,12 +91,12 @@
     auto& incrementor = downcast<AccessibilitySpinButtonPart>(*cache->create(AccessibilityRole::SpinButtonPart));
     incrementor.setIsIncrementor(true);
     incrementor.setParent(this);
-    m_children.append(&incrementor);
+    addChild(&incrementor);
 
     auto& decrementor = downcast<AccessibilitySpinButtonPart>(*cache->create(AccessibilityRole::SpinButtonPart));
     decrementor.setIsIncrementor(false);
     decrementor.setParent(this);
-    m_children.append(&decrementor);
+    addChild(&decrementor);
 }
     
 void AccessibilitySpinButton::step(int amount)

Modified: trunk/Source/WebCore/accessibility/AccessibilityTable.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityTable.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityTable.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -394,8 +394,11 @@
     if (HTMLTableElement* tableElement = this->tableElement()) {
         if (auto caption = tableElement->caption()) {
             AccessibilityObject* axCaption = axObjectCache()->getOrCreate(caption.get());
+            // While `addChild` won't insert ignored children, we still need this accessibilityIsIgnored
+            // check so that `addChild` doesn't try to add the caption's children in its stead. Basically,
+            // explicitly checking accessibilityIsIgnored() ignores the caption and any of its children.
             if (axCaption && !axCaption->accessibilityIsIgnored())
-                m_children.append(axCaption);
+                addChild(axCaption);
         }
     }
 
@@ -420,14 +423,10 @@
         column.setColumnIndex(i);
         column.setParent(this);
         m_columns.append(&column);
-        if (!column.accessibilityIsIgnored())
-            m_children.append(&column);
+        addChild(&column);
     }
+    addChild(headerContainer());
 
-    auto* headerContainerObject = headerContainer();
-    if (headerContainerObject && !headerContainerObject->accessibilityIsIgnored())
-        m_children.append(headerContainerObject);
-
     // Sometimes the cell gets the wrong role initially because it is created before the parent
     // determines whether it is an accessibility table. Iterate all the cells and allow them to
     // update their roles now that the table knows its status.
@@ -451,8 +450,7 @@
     
     row.setRowIndex(static_cast<int>(m_rows.size()));
     m_rows.append(&row);
-    if (!row.accessibilityIsIgnored())
-        m_children.append(&row);
+    addChild(&row);
     appendedRows.add(&row);
         
     // store the maximum number of columns

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableColumn.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -202,7 +202,7 @@
         if (m_children.size() > 0 && m_children.last() == cell)
             continue;
             
-        m_children.append(cell);
+        addChild(cell);
     }
 }
     

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableHeaderContainer.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityTableHeaderContainer.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableHeaderContainer.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -73,7 +73,8 @@
     if (!parentTable.isExposable())
         return;
 
-    m_children = parentTable.columnHeaders();
+    for (auto& columnHeader : parentTable.columnHeaders())
+        addChild(columnHeader.get());
 
     for (const auto& child : m_children)
         m_headerRect.unite(child->elementRect());

Modified: trunk/Source/WebCore/accessibility/AccessibilityTableRow.cpp (284605 => 284606)


--- trunk/Source/WebCore/accessibility/AccessibilityTableRow.cpp	2021-10-21 16:17:10 UTC (rev 284605)
+++ trunk/Source/WebCore/accessibility/AccessibilityTableRow.cpp	2021-10-21 16:27:29 UTC (rev 284606)
@@ -109,7 +109,7 @@
     
     return nullptr;
 }
-    
+
 AXCoreObject* AccessibilityTableRow::headerObject()
 {
     if (!m_renderer || !m_renderer->isTableRow())
@@ -150,10 +150,12 @@
 void AccessibilityTableRow::addChildren()
 {
     // If the element specifies its cells through aria-owns, return that first.
-    AccessibilityChildrenVector ariaOwns;
-    ariaOwnsElements(ariaOwns);
-    if (ariaOwns.size())
-        m_children = WTFMove(ariaOwns);
+    AccessibilityChildrenVector ariaOwnedElements;
+    ariaOwnsElements(ariaOwnedElements);
+    if (ariaOwnedElements.size()) {
+        for (auto& ariaOwnedElement : ariaOwnedElements)
+            addChild(ariaOwnedElement.get());
+    }
     else
         AccessibilityRenderObject::addChildren();
     
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to