Bug 261680

Summary: AX: columnHeaders() and rowHeaders() are performed on the main thread in ITM
Product: WebKit Reporter: Joshua Hoffman <jhoffman23>
Component: AccessibilityAssignee: Joshua Hoffman <jhoffman23>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, tyler_w, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch
none
Patch none

Joshua Hoffman
Reported 2023-09-18 09:26:07 PDT
The methods columnHeaders() and rowHeaders() are performed on the main thread in ITM, which can be the source of several issues.
Attachments
Patch (29.70 KB, patch)
2023-09-18 11:01 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (31.86 KB, patch)
2023-09-18 22:36 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (33.25 KB, patch)
2023-09-19 09:09 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (33.65 KB, patch)
2023-09-19 10:33 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (37.45 KB, patch)
2023-09-20 11:28 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (37.65 KB, patch)
2023-09-20 21:17 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (37.80 KB, patch)
2023-09-21 09:25 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Patch (37.80 KB, patch)
2023-09-22 11:31 PDT, Joshua Hoffman 		no flags
DetailsFormatted DiffDiff
Show Obsolete (7) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-09-18 09:26:17 PDT
<rdar://problem/115661951>
Joshua Hoffman
Comment 2 2023-09-18 11:01:03 PDT
Created attachment 467739 [details] Patch
Tyler Wilcock
Comment 3 2023-09-18 17:45:36 PDT
Comment on attachment 467739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467739&action=review > Source/WebCore/accessibility/AccessibilityObject.h:155 > + String scope() const override { return getAttribute(HTMLNames::scopeAttr); } Can this be final instead of override? > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1789 > + if (parent->roleValue() == AccessibilityRole::RowGroup) This is a behavior change relative to what this function used to do (compare thead, tbody, or tfoot ancestors). Maybe we need a new AXID property and virtual function called row-group container which we cache for table-cells only and compare here? > Source/WebCore/accessibility/AccessibilityTableRow.h:44 > + AXCoreObject* headerObject() override; Can this be final? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1849 > + AccessibilityChildrenVector columnsCopy = columns(); Can this be auto? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1875 > + const String& headerScope = tableCell->scope(); > + > + if (headerScope == "colgroup"_s && isTableCellInSameColGroup(tableCell)) Do we need the headerScope temporary, or can we do this: if (tableCell->scope() == "colgroup"_s ...) > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1899 > + AccessibilityChildrenVector rowsCopy = rows(); Can this be auto? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1905 > + AXCoreObject* parent = exposedTableAncestor(); Can this be auto*? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:1916 > + const String& headerScope = scope(); Do we need this headerScope temporary? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:167 > + bool isColumnHeaderCell() const override { return boolAttributeValue(AXPropertyName::IsColumnHeaderCell); } > + bool isRowHeaderCell() const override { return boolAttributeValue(AXPropertyName::IsRowHeaderCell); } > + String scope() const override { return stringAttributeValue(AXPropertyName::Scope); } Can these be final? > Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:177 > + AXCoreObject* headerObject() override { return objectAttributeValue(AXPropertyName::HeaderObject); }; Can this be final? > LayoutTests/accessibility/column-header-scope.html:21 > + <table id="table1"> > + <tr> > + <th scope="col" id="title-cell">Title</th> > + <th>Col 1</th> > + <th>Col 2</th> > + <th>Col 3</th> > + </tr> > + <tr> > + <td>Data abc</td> > + <td>Data def</td> > + <td>Data ghi</td> > + <td>Data jkl</td> > + </tr> > + </table> Seems like a lot of lines in this test have 8 space indents — I think we've been trying to stick with 4. Also, I think we typically don't indent after the <body>. So like this: <body> <table> .... </table> <script> ... </script> > LayoutTests/accessibility/column-header-scope.html:33 > + var thirdRow = "<tr><td id='cell2'>Data abc</td><td>Data def</td><td>Data ghi</td><td>Data jkl</td></tr>"; Is this used? > LayoutTests/accessibility/column-header-scope.html:36 > + testOutput += "The table cell at (0, 1) should have exactly 1 column header, currently it has " + colHeaders1.length + " column header(s).\n"; > + testOutput += "The table cell at (2, 0) should have exactly 0 row headers, currently it has " + rowHeaders1.length + " row header(s).\n"; Format strings might make this a bit nicer. e.g. testOutput += `The table cell at (0, 1) should have exactly 1 column header, currently it has ${colHeaders1.length} column header(s).\n`; Same for other situations through this file. > LayoutTests/accessibility/column-header-scope.html:41 > + Extra newline here I think. > LayoutTests/accessibility/table-headers-changing.html:44 > + var head_row = document.getElementById("row-in-head"); I think our style has been to use for camelCase over snake_case.
Joshua Hoffman
Comment 4 2023-09-18 19:43:50 PDT
Comment on attachment 467739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467739&action=review >> Source/WebCore/accessibility/AccessibilityObjectInterface.h:1789 >> + if (parent->roleValue() == AccessibilityRole::RowGroup) > > This is a behavior change relative to what this function used to do (compare thead, tbody, or tfoot ancestors). Maybe we need a new AXID property and virtual function called row-group container which we cache for table-cells only and compare here? Sounds good, let's do that instead! >> Source/WebCore/accessibility/AccessibilityTableRow.h:44 >> + AXCoreObject* headerObject() override; > > Can this be final? This one can't because it can be overridden by AccessibilityARIAGridRow, but I'll mark that one as final. >> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:167 >> + String scope() const override { return stringAttributeValue(AXPropertyName::Scope); } > > Can these be final? Yep, that was an oversight on my part >> LayoutTests/accessibility/column-header-scope.html:21 >> + </table> > > Seems like a lot of lines in this test have 8 space indents — I think we've been trying to stick with 4. Also, I think we typically don't indent after the <body>. So like this: > > <body> > <table> > .... > </table> > <script> > ... > </script> will update that along with the other style suggestions!
Joshua Hoffman
Comment 5 2023-09-18 19:43:51 PDT
Comment on attachment 467739 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467739&action=review >> Source/WebCore/accessibility/AccessibilityObjectInterface.h:1789 >> + if (parent->roleValue() == AccessibilityRole::RowGroup) > > This is a behavior change relative to what this function used to do (compare thead, tbody, or tfoot ancestors). Maybe we need a new AXID property and virtual function called row-group container which we cache for table-cells only and compare here? Sounds good, let's do that instead! >> Source/WebCore/accessibility/AccessibilityTableRow.h:44 >> + AXCoreObject* headerObject() override; > > Can this be final? This one can't because it can be overridden by AccessibilityARIAGridRow, but I'll mark that one as final. >> Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:167 >> + String scope() const override { return stringAttributeValue(AXPropertyName::Scope); } > > Can these be final? Yep, that was an oversight on my part >> LayoutTests/accessibility/column-header-scope.html:21 >> + </table> > > Seems like a lot of lines in this test have 8 space indents — I think we've been trying to stick with 4. Also, I think we typically don't indent after the <body>. So like this: > > <body> > <table> > .... > </table> > <script> > ... > </script> will update that along with the other style suggestions!
Joshua Hoffman
Comment 6 2023-09-18 22:36:54 PDT
Created attachment 467748 [details] Patch
Joshua Hoffman
Comment 7 2023-09-18 22:40:49 PDT
As a follow up to this patch, it would be good to increase our test coverage of scope=rowgroup. This patch does not modify that calculation and uses the same structure as before for the Isolated Object.
Tyler Wilcock
Comment 8 2023-09-18 22:59:29 PDT
Comment on attachment 467748 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=467748&action=review > Source/WebCore/accessibility/AXObjectCache.cpp:2444 > + postNotification(element, AXObjectCache::AXScopeChanged); Because we are inside AXObjectCache, I don't think we need the AXObjectCache:: prefix here. > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1789 > +inline bool AXCoreObject::isTableCellInSameRowGroup(AXCoreObject* otherTableCell) > +{ > + return rowGroupAncestorID() ? rowGroupAncestorID() == otherTableCell->rowGroupAncestorID() : false; > +} Seems like we unconditionally dereference otherTableCell. I think we either need a null-check or to pass a reference instead of a pointer. Also, since rowGroupAncestorID() does a lot of iteration, let's compute it only once. e.g.: AXID ancestorID = rowGroupAncestorID(); return ancestorID ? ancestorID == otherTableCell->rowGroupAncestorID() : false; > Source/WebCore/accessibility/AccessibilityObjectInterface.h:1798 > + auto colRange = columnIndexRange(); > + auto otherColRange = tableCell->columnIndexRange(); > + > + if (colRange.first <= (otherColRange.first + otherColRange.second)) > + return true; > + return false; I know you didn't originally write this code, but let's bring it up to our style standards by fully spelling out columnRange vs. colRange, same for otherColRange.
Joshua Hoffman
Comment 9 2023-09-19 09:09:18 PDT
Created attachment 467754 [details] Patch
Joshua Hoffman
Comment 10 2023-09-19 09:10:12 PDT
I went ahead and added in a new scope=rowgroup change to the test in this revision to enhance our coverage.
Joshua Hoffman
Comment 11 2023-09-19 10:33:36 PDT
Created attachment 467760 [details] Patch
Andres Gonzalez
Comment 12 2023-09-19 18:51:50 PDT
(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,
Joshua Hoffman
Comment 13 2023-09-20 11:28:14 PDT
(In reply to Andres Gonzalez from comment #12) > (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? Yep, I'll change those. > + bool isTableCellInSameRowGroup(AXCoreObject*); > + bool isTableCellInSameColGroup(AXCoreObject*); > > AG: and these isInSameRowGroup and isInSameColumnGroup? For these two, I think keeping cell in the name makes the most sense, since that is the context in which they should be called (instead of callers trying this on rows themselves). > virtual bool isTableRow() const = 0; > virtual unsigned rowIndex() const = 0; > + virtual AXCoreObject* headerObject() { return nullptr; } > > AG: maybe headerCell() would be more appropriate? Agree this should be renamed. Since the context is rows, I think rowHeaderCell() would be the clearest option.
Joshua Hoffman
Comment 14 2023-09-20 11:28:46 PDT
Created attachment 467791 [details] Patch
Joshua Hoffman
Comment 15 2023-09-20 21:17:07 PDT
Created attachment 467805 [details] Patch
Joshua Hoffman
Comment 16 2023-09-20 21:18:25 PDT
Updated patch to move methods to AXCoreObject.cpp after that file was added in https://bugs.webkit.org/show_bug.cgi?id=261742.
Andres Gonzalez
Comment 17 2023-09-21 07:10:41 PDT
(In reply to Joshua Hoffman from comment #15) > Created attachment 467805 [details] > Patch diff --git a/Source/WebCore/accessibility/AXCoreObject.cpp b/Source/WebCore/accessibility/AXCoreObject.cpp index 1f32cd90b66a..74a9324a9a8a 100644 --- a/Source/WebCore/accessibility/AXCoreObject.cpp +++ b/Source/WebCore/accessibility/AXCoreObject.cpp @@ -276,6 +276,23 @@ unsigned AXCoreObject::tableLevel() const return level; } +bool AXCoreObject::isTableCellInSameRowGroup(AXCoreObject* otherTableCell) +{ + if (!otherTableCell) + return false; + + AXID ancestorID = rowGroupAncestorID(); + return ancestorID.isValid() && ancestorID == otherTableCell->rowGroupAncestorID(); +} + +bool AXCoreObject::isTableCellInSameColGroup(AXCoreObject* tableCell) +{ + auto columnRange = columnIndexRange(); + auto otherColumnRange = tableCell->columnIndexRange(); AG: tableCell can be null. If not, change the param of the method to be const AXCoreObject&. + + return columnRange.first <= otherColumnRange.first + otherColumnRange.second; +} + String AXCoreObject::ariaLandmarkRoleDescription() const { switch (roleValue()) { diff --git a/Source/WebCore/accessibility/AXCoreObject.h b/Source/WebCore/accessibility/AXCoreObject.h index f364792fe5fa..b220306e58c7 100644 --- a/Source/WebCore/accessibility/AXCoreObject.h +++ b/Source/WebCore/accessibility/AXCoreObject.h @@ -897,6 +897,12 @@ public: // Table cell support. virtual bool isTableCell() const = 0; virtual bool isExposedTableCell() const = 0; + virtual bool isColumnHeader() const { return false; } + virtual bool isRowHeader() const { return false; } + bool isTableCellInSameRowGroup(AXCoreObject*); + bool isTableCellInSameColGroup(AXCoreObject*); + virtual AXID rowGroupAncestorID() const { return { }; } + virtual String scope() const { return { }; } AG: cellScope() ? // 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. @@ -912,6 +918,7 @@ public: // Table row support. virtual bool isTableRow() const = 0; virtual unsigned rowIndex() const = 0; + virtual AXCoreObject* rowHeaderCell() { return nullptr; } AG: rowHeader ? Cell is redundant in the name. // ARIA tree/grid row support. virtual bool isARIATreeGridRow() const = 0; diff --git a/Source/WebCore/accessibility/AXLogger.cpp b/Source/WebCore/accessibility/AXLogger.cpp index 92625adbcc4a..f2db3626b7db 100644 --- a/Source/WebCore/accessibility/AXLogger.cpp +++ b/Source/WebCore/accessibility/AXLogger.cpp @@ -640,6 +640,9 @@ TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notific case AXObjectCache::AXNotification::AXRowSpanChanged: stream << "AXRowSpanChanged"; break; + case AXObjectCache::AXNotification::AXScopeChanged: + stream << "AXScopeChanged"; + break; case AXObjectCache::AXNotification::AXSelectedChildrenChanged: stream << "AXSelectedChildrenChanged"; break; diff --git a/Source/WebCore/accessibility/AXObjectCache.cpp b/Source/WebCore/accessibility/AXObjectCache.cpp index bed612dfe2e1..06bfdd7a2243 100644 --- a/Source/WebCore/accessibility/AXObjectCache.cpp +++ b/Source/WebCore/accessibility/AXObjectCache.cpp @@ -2436,7 +2436,8 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName& } else if (attrName == colspanAttr) { postNotification(element, AXColumnSpanChanged); recomputeParentTableProperties(element, TableProperty::CellSlots); - } + } else if (attrName == scopeAttr) + postNotification(element, AXScopeChanged); if (!attrName.localName().string().startsWith("aria-"_s)) return; @@ -4143,6 +4144,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili case AXRowIndexChanged: tree->updateNodeProperty(*notification.first, AXPropertyName::AXRowIndex); break; + case AXScopeChanged: + tree->updateNodeProperties(*notification.first, { AXPropertyName::Scope, AXPropertyName::IsColumnHeader, AXPropertyName::IsRowHeader }); + break; // FIXME: Contrary to the name "AXSelectedCellsChanged", this notification can be posted on a cell // who has changed selected state, not just on table or grid who has changed its selected cells. case AXSelectedCellsChanged: diff --git a/Source/WebCore/accessibility/AXObjectCache.h b/Source/WebCore/accessibility/AXObjectCache.h index ee4c3d9bac6f..30f3f8e25a19 100644 --- a/Source/WebCore/accessibility/AXObjectCache.h +++ b/Source/WebCore/accessibility/AXObjectCache.h @@ -365,6 +365,7 @@ public: AXRoleDescriptionChanged, AXRowIndexChanged, AXRowSpanChanged, + AXScopeChanged, AG: conversely, should this be CellScope? AXSelectedChildrenChanged, AXSelectedCellsChanged, AXSelectedStateChanged, diff --git a/Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp b/Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp index 0b1403411dce..adf0347875e5 100644 --- a/Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp +++ b/Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp @@ -147,7 +147,7 @@ AccessibilityTable* AccessibilityARIAGridRow::parentTable() const })); } -AXCoreObject* AccessibilityARIAGridRow::headerObject() +AXCoreObject* AccessibilityARIAGridRow::rowHeaderCell() { for (const auto& child : children()) { if (child->roleValue() == AccessibilityRole::RowHeader) diff --git a/Source/WebCore/accessibility/AccessibilityARIAGridRow.h b/Source/WebCore/accessibility/AccessibilityARIAGridRow.h index 43d55762b7be..4834eeb625e1 100644 --- a/Source/WebCore/accessibility/AccessibilityARIAGridRow.h +++ b/Source/WebCore/accessibility/AccessibilityARIAGridRow.h @@ -43,7 +43,7 @@ public: AccessibilityChildrenVector disclosedRows() override; AXCoreObject* disclosedByRow() const override; - AXCoreObject* headerObject() override; + AXCoreObject* rowHeaderCell() final; private: explicit AccessibilityARIAGridRow(RenderObject*); diff --git a/Source/WebCore/accessibility/AccessibilityObject.h b/Source/WebCore/accessibility/AccessibilityObject.h index 840982e7d6cc..0ac044c71ad7 100644 --- a/Source/WebCore/accessibility/AccessibilityObject.h +++ b/Source/WebCore/accessibility/AccessibilityObject.h @@ -151,6 +151,7 @@ public: AccessibilityChildrenVector columnHeaders() override { return AccessibilityChildrenVector(); } AccessibilityChildrenVector rowHeaders() override { return AccessibilityChildrenVector(); } AccessibilityChildrenVector visibleRows() override { return AccessibilityChildrenVector(); } + String scope() const final { return getAttribute(HTMLNames::scopeAttr); } AXCoreObject* headerContainer() override { return nullptr; } int axColumnCount() const override { return 0; } int axRowCount() const override { return 0; } @@ -163,8 +164,6 @@ public: std::pair<unsigned, unsigned> rowIndexRange() const override { return { 0, 1 }; } // Returns the start location and column span of the cell. std::pair<unsigned, unsigned> columnIndexRange() const override { return { 0, 1 }; } - virtual bool isColumnHeaderCell() const { return false; } - virtual bool isRowHeaderCell() const { return false; } int axColumnIndex() const override { return -1; } int axRowIndex() const override { return -1; } diff --git a/Source/WebCore/accessibility/AccessibilityTable.cpp b/Source/WebCore/accessibility/AccessibilityTable.cpp index 3087200d4d97..1983aed238e5 100644 --- a/Source/WebCore/accessibility/AccessibilityTable.cpp +++ b/Source/WebCore/accessibility/AccessibilityTable.cpp @@ -741,7 +741,7 @@ AXCoreObject::AccessibilityChildrenVector AccessibilityTable::rowHeaders() // Sometimes m_rows can be reset during the iteration, we cache it here to be safe. AccessibilityChildrenVector rowsCopy = m_rows; for (const auto& row : rowsCopy) { - if (auto* header = downcast<AccessibilityTableRow>(*row).headerObject()) + if (auto* header = downcast<AccessibilityTableRow>(*row).rowHeaderCell()) headers.append(header); } diff --git a/Source/WebCore/accessibility/AccessibilityTableCell.cpp b/Source/WebCore/accessibility/AccessibilityTableCell.cpp index 894462dd065c..ae36ec7afe08 100644 --- a/Source/WebCore/accessibility/AccessibilityTableCell.cpp +++ b/Source/WebCore/accessibility/AccessibilityTableCell.cpp @@ -144,9 +144,9 @@ AccessibilityRole AccessibilityTableCell::determineAccessibilityRole() if (!isExposedTableCell()) return defaultRole; - if (isColumnHeaderCell()) + if (isColumnHeader()) return AccessibilityRole::ColumnHeader; - if (isRowHeaderCell()) + if (isRowHeader()) return AccessibilityRole::RowHeader; return AccessibilityRole::Cell; @@ -173,7 +173,7 @@ bool AccessibilityTableCell::isTableHeaderCell() const return false; } -bool AccessibilityTableCell::isColumnHeaderCell() const +bool AccessibilityTableCell::isColumnHeader() const { const AtomString& scope = getAttribute(scopeAttr); if (scope == "col"_s || scope == "colgroup"_s) @@ -201,7 +201,7 @@ bool AccessibilityTableCell::isColumnHeaderCell() const return false; } -bool AccessibilityTableCell::isRowHeaderCell() const +bool AccessibilityTableCell::isRowHeader() const { const AtomString& scope = getAttribute(scopeAttr); if (scope == "row"_s || scope == "rowgroup"_s) @@ -226,32 +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; - } - - return otherParentNode == parentNode; -} - -bool AccessibilityTableCell::isTableCellInSameColGroup(AXCoreObject* tableCell) +AXID AccessibilityTableCell::rowGroupAncestorID() const { - auto colRange = columnIndexRange(); - auto otherColRange = tableCell->columnIndexRange(); + auto* rowGroup = Accessibility::findAncestor<AccessibilityObject>(*this, false, [] (const auto& ancestor) { + return ancestor.hasTagName(theadTag) || ancestor.hasTagName(tbodyTag) || ancestor.hasTagName(tfootTag); + }); + if (!rowGroup) + return { }; - if (colRange.first <= (otherColRange.first + otherColRange.second)) - return true; - return false; + return rowGroup->objectID(); } String AccessibilityTableCell::expandedTextValue() const @@ -285,10 +269,9 @@ 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()) + else if (downcast<AccessibilityObject>(tableCell)->isColumnHeader()) headers.append(tableCell); } @@ -309,11 +292,10 @@ 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()) + else if (downcast<AccessibilityObject>(tableCell)->isRowHeader()) headers.append(tableCell); } diff --git a/Source/WebCore/accessibility/AccessibilityTableCell.h b/Source/WebCore/accessibility/AccessibilityTableCell.h index 86252ab79b2b..8ea7c934b5a2 100644 --- a/Source/WebCore/accessibility/AccessibilityTableCell.h +++ b/Source/WebCore/accessibility/AccessibilityTableCell.h @@ -44,8 +44,10 @@ public: bool isExposedTableCell() const final; bool isTableHeaderCell() const; - bool isColumnHeaderCell() const override; - bool isRowHeaderCell() const override; + bool isColumnHeader() const override; + bool isRowHeader() const override; + + AXID rowGroupAncestorID() const final; virtual AccessibilityTable* parentTable() const; @@ -93,8 +95,6 @@ private: AccessibilityTableRow* ariaOwnedByParent() const; void ensureIndexesUpToDate() const; - bool isTableCellInSameRowGroup(AXCoreObject*); - bool isTableCellInSameColGroup(AXCoreObject*); }; } // namespace WebCore diff --git a/Source/WebCore/accessibility/AccessibilityTableRow.cpp b/Source/WebCore/accessibility/AccessibilityTableRow.cpp index 3528e38d8a9f..3002700d0511 100644 --- a/Source/WebCore/accessibility/AccessibilityTableRow.cpp +++ b/Source/WebCore/accessibility/AccessibilityTableRow.cpp @@ -116,7 +116,7 @@ AccessibilityTable* AccessibilityTableRow::parentTable() const return nullptr; } -AXCoreObject* AccessibilityTableRow::headerObject() +AXCoreObject* AccessibilityTableRow::rowHeaderCell() { const auto& rowChildren = children(); if (rowChildren.isEmpty()) diff --git a/Source/WebCore/accessibility/AccessibilityTableRow.h b/Source/WebCore/accessibility/AccessibilityTableRow.h index edbdac785d38..ccb1f22c1dd2 100644 --- a/Source/WebCore/accessibility/AccessibilityTableRow.h +++ b/Source/WebCore/accessibility/AccessibilityTableRow.h @@ -41,7 +41,7 @@ public: virtual ~AccessibilityTableRow(); // retrieves the "row" header (a th tag in the rightmost column) - virtual AXCoreObject* headerObject(); + AXCoreObject* rowHeaderCell() override; virtual AccessibilityTable* parentTable() const; void setRowIndex(unsigned rowIndex) { m_rowIndex = rowIndex; } diff --git a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp b/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp index 79a35120c760..ddbdcbb95642 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::IsColumnHeader, object.isColumnHeader()); + setProperty(AXPropertyName::IsRowHeader, object.isRowHeader()); + setProperty(AXPropertyName::Scope, object.scope().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::RowHeaderCell, object.rowHeaderCell()); + 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->isColumnHeader()) + 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->rowHeaderCell()) + headers.append(header); + } + } else if (isExposedTableCell()) { + auto* parent = exposedTableAncestor(); + if (!parent) + return { }; + + 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->isRowHeader()) + 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 b0caa179c3aa..8d189ec4d99a 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 isColumnHeader() const final { return boolAttributeValue(AXPropertyName::IsColumnHeader); } + bool isRowHeader() const final { return boolAttributeValue(AXPropertyName::IsRowHeader); } + 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* rowHeaderCell() final { return objectAttributeValue(AXPropertyName::RowHeaderCell); }; // 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 8490f48585a7..4504615099b9 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::IsColumnHeader: + propertyMap.set(AXPropertyName::IsColumnHeader, axObject.isColumnHeader()); + 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::IsRowHeader: + propertyMap.set(AXPropertyName::IsRowHeader, axObject.isRowHeader()); + 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: 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 7e35a7781b80..2a1b9fa944b3 100644 --- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h +++ b/Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h @@ -119,6 +119,7 @@ enum class AXPropertyName : uint16_t { IsAttachment, IsBusy, IsChecked, + IsColumnHeader, IsControl, IsEnabled, IsExpanded, @@ -148,6 +149,7 @@ enum class AXPropertyName : uint16_t { IsMultiSelectable, IsPressed, IsRequired, + IsRowHeader, IsSecureField, IsSelected, IsSelectedOptionActive, @@ -196,10 +198,13 @@ enum class AXPropertyName : uint16_t { RolePlatformString, RoleDescription, Rows, + RowGroupAncestorID, + RowHeaderCell, AG: RowHeader ? RowHeaders, RowIndex, RowIndexRange, ScreenRelativePosition, + Scope, AG: CellScope ? SelectedChildren, SessionID, SetSize,
Joshua Hoffman
Comment 18 2023-09-21 09:25:33 PDT
Created attachment 467814 [details] Patch
Joshua Hoffman
Comment 19 2023-09-22 11:31:11 PDT
Created attachment 467825 [details] Patch
EWS
Comment 20 2023-09-22 13:36:40 PDT
Committed 268330@main (ad80707652bd): <https://commits.webkit.org/268330@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 467825 [details].
Note You need to log in before you can comment on or make changes to this bug.