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

Description Joshua Hoffman 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.
Comment 1 Radar WebKit Bug Importer 2023-09-18 09:26:17 PDT 
<rdar://problem/115661951>
Comment 2 Joshua Hoffman 2023-09-18 11:01:03 PDT 
Created attachment 467739 [details]
Patch
Comment 3 Tyler Wilcock 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.
Comment 4 Joshua Hoffman 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!
Comment 5 Joshua Hoffman 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!
Comment 6 Joshua Hoffman 2023-09-18 22:36:54 PDT 
Created attachment 467748 [details]
Patch
Comment 7 Joshua Hoffman 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.
Comment 8 Tyler Wilcock 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.
Comment 9 Joshua Hoffman 2023-09-19 09:09:18 PDT 
Created attachment 467754 [details]
Patch
Comment 10 Joshua Hoffman 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.
Comment 11 Joshua Hoffman 2023-09-19 10:33:36 PDT 
Created attachment 467760 [details]
Patch
Comment 12 Andres Gonzalez 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,
Comment 13 Joshua Hoffman 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.
Comment 14 Joshua Hoffman 2023-09-20 11:28:46 PDT 
Created attachment 467791 [details]
Patch
Comment 15 Joshua Hoffman 2023-09-20 21:17:07 PDT 
Created attachment 467805 [details]
Patch
Comment 16 Joshua Hoffman 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.
Comment 17 Andres Gonzalez 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,
Comment 18 Joshua Hoffman 2023-09-21 09:25:33 PDT 
Created attachment 467814 [details]
Patch
Comment 19 Joshua Hoffman 2023-09-22 11:31:11 PDT 
Created attachment 467825 [details]
Patch
Comment 20 EWS 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].