[Webkit-unassigned] [Bug 261680] AX: columnHeaders() and rowHeaders() are performed on the main thread in ITM
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 19 18:51:50 PDT 2023
https://bugs.webkit.org/show_bug.cgi?id=261680
--- Comment #12 from Andres Gonzalez <andresg_22 at apple.com> ---
(In reply to Joshua Hoffman from comment #11)
> Created attachment 467760 [details]
> Patch
diff --git a/Source/WebCore/accessibility/AccessibilityObjectInterface.h b/Source/WebCore/accessibility/AccessibilityObjectInterface.h
index e727148f9bbb..0fde9758e103 100644
--- a/Source/WebCore/accessibility/AccessibilityObjectInterface.h
+++ b/Source/WebCore/accessibility/AccessibilityObjectInterface.h
@@ -892,6 +892,12 @@ public:
// Table cell support.
virtual bool isTableCell() const = 0;
virtual bool isExposedTableCell() const = 0;
+ virtual bool isColumnHeaderCell() const { return false; }
+ virtual bool isRowHeaderCell() const { return false; }
AG: Can we call these just isColumnHeader and isRowHeader?
+ bool isTableCellInSameRowGroup(AXCoreObject*);
+ bool isTableCellInSameColGroup(AXCoreObject*);
AG: and these isInSameRowGroup and isInSameColumnGroup?
+ virtual AXID rowGroupAncestorID() const { return AXID(); }
+ virtual String scope() const { return String(); }
AG: in both of these:
return { };
// Returns the start location and row span of the cell.
virtual std::pair<unsigned, unsigned> rowIndexRange() const = 0;
// Returns the start location and column span of the cell.
@@ -907,6 +913,7 @@ public:
// Table row support.
virtual bool isTableRow() const = 0;
virtual unsigned rowIndex() const = 0;
+ virtual AXCoreObject* headerObject() { return nullptr; }
AG: maybe headerCell() would be more appropriate?
// ARIA tree/grid row support.
virtual bool isARIATreeGridRow() const = 0;
@@ -1762,6 +1769,25 @@ inline unsigned AXCoreObject::tableLevel() const
return level;
}
+inline bool AXCoreObject::isTableCellInSameRowGroup(AXCoreObject* otherTableCell)
+{
+ if (!otherTableCell)
+ return false;
+
+ AXID ancestorID = rowGroupAncestorID();
+ return ancestorID ? ancestorID == otherTableCell->rowGroupAncestorID() : false;
+}
AG: the return line can be written as:
return ancestorID .isValid() && ancestorID == otherTableCell->rowGroupAncestorID();
+
+inline bool AXCoreObject::isTableCellInSameColGroup(AXCoreObject* tableCell)
+{
+ auto columnRange = columnIndexRange();
+ auto otherColumnRange = tableCell->columnIndexRange();
+
+ if (columnRange.first <= (otherColumnRange.first + otherColumnRange.second))
+ return true;
+ return false;
+}
AG: instead of this if statement you can write:
return columnRange.first <= otherColumnRange.first + otherColumnRange.second;
+
inline String AXCoreObject::ariaLandmarkRoleDescription() const
{
switch (roleValue()) {
diff --git a/Source/WebCore/accessibility/AccessibilityTableCell.cpp b/Source/WebCore/accessibility/AccessibilityTableCell.cpp
index 894462dd065c..d0b21617f6e3 100644
--- a/Source/WebCore/accessibility/AccessibilityTableCell.cpp
+++ b/Source/WebCore/accessibility/AccessibilityTableCell.cpp
@@ -226,34 +226,16 @@ bool AccessibilityTableCell::isRowHeaderCell() const
}
return false;
}
-
-bool AccessibilityTableCell::isTableCellInSameRowGroup(AXCoreObject* otherTableCell)
-{
- Node* parentNode = node();
- for ( ; parentNode; parentNode = parentNode->parentNode()) {
- if (parentNode->hasTagName(theadTag) || parentNode->hasTagName(tbodyTag) || parentNode->hasTagName(tfootTag))
- break;
- }
- Node* otherParentNode = otherTableCell->node();
- for ( ; otherParentNode; otherParentNode = otherParentNode->parentNode()) {
- if (otherParentNode->hasTagName(theadTag) || otherParentNode->hasTagName(tbodyTag) || otherParentNode->hasTagName(tfootTag))
- break;
+AXID AccessibilityTableCell::rowGroupAncestorID() const
+{
+ for (auto* parent = parentObject(); parent; parent = parent->parentObject()) {
+ if (parent->hasTagName(theadTag) || parent->hasTagName(tbodyTag) || parent->hasTagName(tfootTag))
+ return parent->objectID();
}
AG: can we write this using findAncestor?
-
- return otherParentNode == parentNode;
+ return AXID();
}
AG: return { };
-bool AccessibilityTableCell::isTableCellInSameColGroup(AXCoreObject* tableCell)
-{
- auto colRange = columnIndexRange();
- auto otherColRange = tableCell->columnIndexRange();
-
- if (colRange.first <= (otherColRange.first + otherColRange.second))
- return true;
- return false;
-}
-
String AccessibilityTableCell::expandedTextValue() const
{
return getAttribute(abbrAttr);
@@ -285,8 +267,7 @@ AXCoreObject::AccessibilityChildrenVector AccessibilityTableCell::columnHeaders(
continue;
ASSERT(is<AccessibilityObject>(tableCell));
- const AtomString& scope = downcast<AccessibilityObject>(tableCell)->getAttribute(scopeAttr);
- if (scope == "colgroup"_s && isTableCellInSameColGroup(tableCell))
+ if (tableCell->scope() == "colgroup"_s && isTableCellInSameColGroup(tableCell))
headers.append(tableCell);
else if (downcast<AccessibilityObject>(tableCell)->isColumnHeaderCell())
headers.append(tableCell);
@@ -309,9 +290,8 @@ AXCoreObject::AccessibilityChildrenVector AccessibilityTableCell::rowHeaders()
auto* tableCell = parent->cellForColumnAndRow(column, rowRange.first);
if (!tableCell || tableCell == this || headers.contains(tableCell))
continue;
-
- const AtomString& scope = downcast<AccessibilityObject>(tableCell)->getAttribute(scopeAttr);
- if (scope == "rowgroup"_s && isTableCellInSameRowGroup(tableCell))
+
+ if (tableCell->scope() == "rowgroup"_s && isTableCellInSameRowGroup(tableCell))
headers.append(tableCell);
else if (downcast<AccessibilityObject>(tableCell)->isRowHeaderCell())
headers.append(tableCell);
diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
index 79a35120c760..602b3426be2d 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp
@@ -209,6 +209,10 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
setProperty(AXPropertyName::RowIndexRange, object.rowIndexRange());
setProperty(AXPropertyName::AXColumnIndex, object.axColumnIndex());
setProperty(AXPropertyName::AXRowIndex, object.axRowIndex());
+ setProperty(AXPropertyName::IsColumnHeaderCell, object.isColumnHeaderCell());
+ setProperty(AXPropertyName::IsRowHeaderCell, object.isRowHeaderCell());
+ setProperty(AXPropertyName::Scope, object.scope());
AG: this is a String right? If so, you need to cache .isolatedcopy().
+ setProperty(AXPropertyName::RowGroupAncestorID, object.rowGroupAncestorID());
}
if (object.isTableColumn()) {
@@ -226,6 +230,9 @@ void AXIsolatedObject::initializeProperties(const Ref<AccessibilityObject>& axOb
setObjectProperty(AXPropertyName::DisclosedByRow, object.disclosedByRow());
}
+ if (object.isARIATreeGridRow() || object.isTableRow())
+ setObjectProperty(AXPropertyName::HeaderObject, object.headerObject());
+
if (object.isTreeItem()) {
setProperty(AXPropertyName::IsTreeItem, true);
setObjectVectorProperty(AXPropertyName::ARIATreeItemContent, object.ariaTreeItemContent());
@@ -940,18 +947,12 @@ T AXIsolatedObject::getOrRetrievePropertyValue(AXPropertyName propertyName)
});
break;
}
- case AXPropertyName::ColumnHeaders:
- value = axIDs(axObject->columnHeaders());
- break;
case AXPropertyName::InnerHTML:
value = axObject->innerHTML().isolatedCopy();
break;
case AXPropertyName::OuterHTML:
value = axObject->outerHTML().isolatedCopy();
break;
- case AXPropertyName::RowHeaders:
- value = axIDs(axObject->rowHeaders());
- break;
default:
break;
}
@@ -1834,7 +1835,40 @@ std::optional<String> AXIsolatedObject::attributeValue(const String& attributeNa
AXCoreObject::AccessibilityChildrenVector AXIsolatedObject::columnHeaders()
{
- return tree()->objectsForIDs(const_cast<AXIsolatedObject*>(this)->getOrRetrievePropertyValue<Vector<AXID>>(AXPropertyName::ColumnHeaders));
+ AccessibilityChildrenVector headers;
+ if (isTable()) {
+ auto columnsCopy = columns();
+ for (const auto& column : columnsCopy) {
+ if (auto* header = column->columnHeader())
+ headers.append(header);
+ }
+ } else if (isExposedTableCell()) {
+ auto* parent = exposedTableAncestor();
+ if (!parent)
+ return { };
+
+ // Choose columnHeaders as the place where the "headers" attribute is reported.
+ headers = relatedObjects(AXRelationType::Headers);
+ // If the headers attribute returned valid values, then do not further search for column headers.
+ if (!headers.isEmpty())
+ return headers;
+
+ auto rowRange = rowIndexRange();
+ auto colRange = columnIndexRange();
+
+ for (unsigned row = 0; row < rowRange.first; row++) {
+ auto* tableCell = parent->cellForColumnAndRow(colRange.first, row);
+ if (!tableCell || tableCell == this || headers.contains(tableCell))
+ continue;
+
+ if (tableCell->scope() == "colgroup"_s && isTableCellInSameColGroup(tableCell))
+ headers.append(tableCell);
+ else if (tableCell->isColumnHeaderCell())
+ headers.append(tableCell);
+ }
+ }
+
+ return headers;
}
String AXIsolatedObject::innerHTML() const
@@ -1849,7 +1883,33 @@ String AXIsolatedObject::outerHTML() const
AXCoreObject::AccessibilityChildrenVector AXIsolatedObject::rowHeaders()
{
- return tree()->objectsForIDs(const_cast<AXIsolatedObject*>(this)->getOrRetrievePropertyValue<Vector<AXID>>(AXPropertyName::RowHeaders));
+ AccessibilityChildrenVector headers;
+ if (isTable()) {
+ auto rowsCopy = rows();
+ for (const auto& row : rowsCopy) {
+ if (auto* header = row->headerObject())
+ headers.append(header);
+ }
+ } else if (isExposedTableCell()) {
+ auto* parent = exposedTableAncestor();
+ if (!parent)
+ return headers;
AG: In the column method you return { } here, which I prefer as well.
+
+ auto rowRange = rowIndexRange();
+ auto colRange = columnIndexRange();
+ for (unsigned column = 0; column < colRange.first; column++) {
+ auto* tableCell = parent->cellForColumnAndRow(column, rowRange.first);
+ if (!tableCell || tableCell == this || headers.contains(tableCell))
+ continue;
+
+ if (tableCell->scope() == "rowgroup"_s && isTableCellInSameRowGroup(tableCell))
+ headers.append(tableCell);
+ else if (tableCell->isRowHeaderCell())
+ headers.append(tableCell);
+ }
+ }
+
+ return headers;
}
#if !PLATFORM(MAC)
diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
index 8d91d1e5f2dc..81d16ec18c67 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
@@ -162,6 +162,10 @@ private:
std::pair<unsigned, unsigned> columnIndexRange() const override { return pairAttributeValue<unsigned>(AXPropertyName::ColumnIndexRange); }
int axColumnIndex() const override { return intAttributeValue(AXPropertyName::AXColumnIndex); }
int axRowIndex() const override { return intAttributeValue(AXPropertyName::AXRowIndex); }
+ bool isColumnHeaderCell() const final { return boolAttributeValue(AXPropertyName::IsColumnHeaderCell); }
+ bool isRowHeaderCell() const final { return boolAttributeValue(AXPropertyName::IsRowHeaderCell); }
+ String scope() const final { return stringAttributeValue(AXPropertyName::Scope); }
+ AXID rowGroupAncestorID() const final { return propertyValue<AXID>(AXPropertyName::RowGroupAncestorID); }
// Table column support.
bool isTableColumn() const override { return boolAttributeValue(AXPropertyName::IsTableColumn); }
@@ -171,6 +175,7 @@ private:
// Table row support.
bool isTableRow() const override { return boolAttributeValue(AXPropertyName::IsTableRow); }
unsigned rowIndex() const override { return unsignedAttributeValue(AXPropertyName::RowIndex); }
+ AXCoreObject* headerObject() final { return objectAttributeValue(AXPropertyName::HeaderObject); };
// ARIA tree/grid row support.
bool isARIATreeGridRow() const override { return boolAttributeValue(AXPropertyName::IsARIATreeGridRow); }
diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
index 166832e864c9..bb59bd515a1e 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp
@@ -531,6 +531,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
propertyMap.set(AXPropertyName::IsChecked, axObject.isChecked());
propertyMap.set(AXPropertyName::ButtonState, axObject.checkboxOrRadioValue());
break;
+ case AXPropertyName::IsColumnHeaderCell:
+ propertyMap.set(AXPropertyName::IsColumnHeaderCell, axObject.isColumnHeaderCell());
+ break;
case AXPropertyName::IsEnabled:
propertyMap.set(AXPropertyName::IsEnabled, axObject.isEnabled());
break;
@@ -543,6 +546,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
case AXPropertyName::IsSelected:
propertyMap.set(AXPropertyName::IsSelected, axObject.isSelected());
break;
+ case AXPropertyName::IsRowHeaderCell:
+ propertyMap.set(AXPropertyName::IsRowHeaderCell, axObject.isRowHeaderCell());
+ break;
case AXPropertyName::MaxValueForRange:
propertyMap.set(AXPropertyName::MaxValueForRange, axObject.maxValueForRange());
break;
@@ -564,6 +570,9 @@ void AXIsolatedTree::updateNodeProperties(AXCoreObject& axObject, const Vector<A
case AXPropertyName::AXRowIndex:
propertyMap.set(AXPropertyName::AXRowIndex, axObject.axRowIndex());
break;
+ case AXPropertyName::Scope:
+ propertyMap.set(AXPropertyName::Scope, axObject.scope());
AG: axObject.scope().isolatedCopy();
+ break;
case AXPropertyName::SetSize:
propertyMap.set(AXPropertyName::SetSize, axObject.setSize());
break;
diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
index f83675983b94..67170b88c7d7 100644
--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
+++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h
@@ -105,6 +105,7 @@ enum class AXPropertyName : uint16_t {
HasPlainText,
HasUnderline,
HeaderContainer,
+ HeaderObject,
HeadingLevel,
HelpText,
HierarchicalLevel,
@@ -119,6 +120,7 @@ enum class AXPropertyName : uint16_t {
IsAttachment,
IsBusy,
IsChecked,
+ IsColumnHeaderCell,
AG: IsColumnHeader
IsControl,
IsEnabled,
IsExpanded,
@@ -148,6 +150,7 @@ enum class AXPropertyName : uint16_t {
IsMultiSelectable,
IsPressed,
IsRequired,
+ IsRowHeaderCell,
AG: IsRowHeader
IsSecureField,
IsSelected,
IsSelectedOptionActive,
@@ -196,10 +199,12 @@ enum class AXPropertyName : uint16_t {
RolePlatformString,
RoleDescription,
Rows,
+ RowGroupAncestorID,
RowHeaders,
RowIndex,
RowIndexRange,
ScreenRelativePosition,
+ Scope,
SelectedChildren,
SessionID,
SetSize,
--
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20230920/91dd0d86/attachment-0001.htm>
More information about the webkit-unassigned
mailing list