Skip to content

Commit

Permalink
AX: columnHeaders() and rowHeaders() are performed on the main thread…
Browse files Browse the repository at this point in the history
… in ITM

https://bugs.webkit.org/show_bug.cgi?id=261680
rdar://115661951

Reviewed by Andres Gonzalez.

This patch uses a combination of caching attributes and sharing code to allow the calculation for rowHeaders() and columnHeaders()
to be performed on the secondary AX thread. Previously, we had to jump to the main thread each time these attributes were requested,
which can show up significantly on samples of pages that have very dynamic tables.

Calculating these values is now accomplished through five newly cached attributes in AXIsolatedObjects: Scope, IsColumnHeaderCell,
IsRowHeaderCell, HeaderObject, and RowGroupAncestorID. By caching these attributes, we are able to calculate the row and column headers for a table or
cell within the isolated object. Thus, we are able to remove the row- and column-header properties from
AXIsolatedObject::getOrRetrievePropertyValue.

The methods isTableCellInSameRowGroup and isTableCellInSameColGroup, which were originally part of the AccessibilityTableCell class,
were moved to AXCoreObject so that this code could be shared between the live and isolated objects.

Two new tests have been added to make sure that these cached values work as expected, and to check that we update the scope properly
when it changes.

* LayoutTests/accessibility/column-header-scope-expected.txt: Added.
* LayoutTests/accessibility/column-header-scope.html: Added.
* LayoutTests/accessibility/table-headers-changing-expected.txt: Added.
* LayoutTests/accessibility/table-headers-changing.html: Added.
* Source/WebCore/accessibility/AXCoreObject.cpp:
(WebCore::AXCoreObject::isTableCellInSameRowGroup):
(WebCore::AXCoreObject::isTableCellInSameColGroup):
* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::AXCoreObject::isColumnHeader const):
(WebCore::AXCoreObject::isRowHeader const):
(WebCore::AXCoreObject::rowGroupAncestorID const):
(WebCore::AXCoreObject::cellScope const):
(WebCore::AXCoreObject::rowHeader):
* Source/WebCore/accessibility/AXLogger.cpp:
(WebCore::operator<<):
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::handleAttributeChange):
(WebCore::AXObjectCache::updateIsolatedTree):
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp:
(WebCore::AccessibilityARIAGridRow::rowHeader):
(WebCore::AccessibilityARIAGridRow::headerObject): Deleted.
* Source/WebCore/accessibility/AccessibilityARIAGridRow.h:
* Source/WebCore/accessibility/AccessibilityObject.h:
(WebCore::AccessibilityObject::isColumnHeaderCell const): Deleted.
(WebCore::AccessibilityObject::isRowHeaderCell const): Deleted.
* Source/WebCore/accessibility/AccessibilityTable.cpp:
(WebCore::AccessibilityTable::rowHeaders):
* Source/WebCore/accessibility/AccessibilityTableCell.cpp:
(WebCore::AccessibilityTableCell::determineAccessibilityRole):
(WebCore::AccessibilityTableCell::isColumnHeader const):
(WebCore::AccessibilityTableCell::isRowHeader const):
(WebCore::AccessibilityTableCell::rowGroupAncestorID const):
(WebCore::AccessibilityTableCell::columnHeaders):
(WebCore::AccessibilityTableCell::rowHeaders):
(WebCore::AccessibilityTableCell::isColumnHeaderCell const): Deleted.
(WebCore::AccessibilityTableCell::isRowHeaderCell const): Deleted.
(WebCore::AccessibilityTableCell::isTableCellInSameRowGroup): Deleted.
(WebCore::AccessibilityTableCell::isTableCellInSameColGroup): Deleted.
* Source/WebCore/accessibility/AccessibilityTableCell.h:
* Source/WebCore/accessibility/AccessibilityTableRow.cpp:
(WebCore::AccessibilityTableRow::rowHeader):
(WebCore::AccessibilityTableRow::headerObject): Deleted.
* Source/WebCore/accessibility/AccessibilityTableRow.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::initializeProperties):
(WebCore::AXIsolatedObject::getOrRetrievePropertyValue):
(WebCore::AXIsolatedObject::columnHeaders):
(WebCore::AXIsolatedObject::rowHeaders):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h:
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:
(WebCore::AXIsolatedTree::updateNodeProperties):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:

Canonical link: https://commits.webkit.org/268330@main
  • Loading branch information
hoffmanjoshua authored and AndresGonzalezApple committed Sep 22, 2023
1 parent 0f2b0e3 commit ad80707
Show file tree
Hide file tree
Showing 21 changed files with 324 additions and 53 deletions.
20 changes: 20 additions & 0 deletions LayoutTests/accessibility/column-header-scope-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
This test ensures that the right header is returned when AX clients request the table header.

The table cell at (0, 1) should have exactly 1 column header, currently it has 1 column header(s).
The table cell at (2, 0) should have exactly 0 row headers, currently it has 0 row header(s).
The table cell at (1, 2) should have exactly 0 row headers, currently it has 0 row header(s).

Changing scope of table header at (0, 0) to 'row':
The table cell at (0, 1) should have exactly 0 column headers, currently it has 0 column header(s).
The table cell at (2, 0) should have exactly 1 row header, currently it has 1 row header(s).
The table cell at (1, 2) should have exactly 0 row headers, currently it has 0 row header(s).

Changing scope of cell at (0, 1) to 'rowgroup':
The table cell at (1, 2) should have exactly 1 row header, currently it has 1 row header(s).

PASS successfullyParsed is true

TEST COMPLETE
Title Col 1 Col 2 Col 3
Data abc Data def Data ghi Data jkl
Data abc Data def Data ghi
68 changes: 68 additions & 0 deletions LayoutTests/accessibility/column-header-scope.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
<!DOCTYPE html>
<html>
<head>
<script src="../resources/js-test.js"></script>
<script src="../resources/accessibility-helper.js"></script>
</head>
<body>
<table id="table1">
<tbody>
<tr>
<th scope="col">Title</th>
<th scope="col" id="title-cell">Col 1</th>
<th>Col 2</th>
<th>Col 3</th>
</tr>
<tr>
<th scope="row" id="second-row-cell">Data abc</td>
<td>Data def</td>
<td>Data ghi</td>
<td>Data jkl</td>
</tr>
<tr>
<td>Data abc</td>
<td>Data def</td>
<td>Data ghi</td>
</tr>
</tbody>
</table>
<script>
var testOutput = "This test ensures that the right header is returned when AX clients request the table header.\n\n";

if (window.accessibilityController) {
window.jsTestIsAsync = true;
var table = accessibilityController.accessibleElementById("table1");
var cell1 = table.cellForColumnAndRow(1, 1);
var cell2 = table.cellForColumnAndRow(2, 0);
var cell3 = table.cellForColumnAndRow(1, 2);

testOutput += `The table cell at (0, 1) should have exactly 1 column header, currently it has ${cell1.columnHeaders().length} column header(s).\n`;
testOutput += `The table cell at (2, 0) should have exactly 0 row headers, currently it has ${cell2.rowHeaders().length} row header(s).\n`;
testOutput += `The table cell at (1, 2) should have exactly 0 row headers, currently it has ${cell3.rowHeaders().length} row header(s).\n\n`;

testOutput += "Changing scope of table header at (0, 0) to 'row':\n";
document.getElementById("title-cell").setAttribute("scope", "row");

setTimeout(async function() {
await waitFor(() => cell1.columnHeaders().length == 0);
testOutput += `The table cell at (0, 1) should have exactly 0 column headers, currently it has ${cell1.columnHeaders().length} column header(s).\n`;

await waitFor(() => cell2.rowHeaders().length);
testOutput += `The table cell at (2, 0) should have exactly 1 row header, currently it has ${cell2.rowHeaders().length} row header(s).\n`;
testOutput += `The table cell at (1, 2) should have exactly 0 row headers, currently it has ${cell3.rowHeaders().length} row header(s).\n\n`;

testOutput += "Changing scope of cell at (0, 1) to 'rowgroup':\n";
document.getElementById("second-row-cell").setAttribute("scope", "rowgroup");
document.getElementById("second-row-cell").setAttribute("rowspan", "2");

cell3 = table.cellForColumnAndRow(1, 2);
await waitFor(() => cell3.rowHeaders().length);
testOutput += `The table cell at (1, 2) should have exactly 1 row header, currently it has ${cell3.rowHeaders().length} row header(s).\n`;

debug(testOutput);
finishJSTest();
}, 0);
}
</script>
</body>
</html>
14 changes: 14 additions & 0 deletions LayoutTests/accessibility/table-headers-changing-expected.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
Cell B should have 1 rowHeaders (has 1) and 0 columnHeaders (has 0)
Cell E should have 1 rowHeader (has 1) and 1 columnHeader (has 1)
Move row in <thead> to <tbody>:
Cell B should have 1 rowHeader (has 1) and 0 columnHeaders (has 0)
Cell E should have 1 rowHeader (has 1) and 0 columnHeaders (has 0)
Add header above E:
Cell E should have 1 rowHeaders (has 1) and 1 columnHeader (has 1)

PASS successfullyParsed is true

TEST COMPLETE
A J B C
D E F
G H I
73 changes: 73 additions & 0 deletions LayoutTests/accessibility/table-headers-changing.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<!DOCTYPE html>
<html>
<head>
<script src="../resources/js-test.js"></script>
<script src="../resources/accessibility-helper.js"></script>
</head>
<body>

<table id="table">
<thead id="table-head">
<tr id="row-in-head">
<th id="a" scope="row">A</th>
<td id="b">B</td>
<td>C</td>
</tr>
</thead>
<tbody id="table-body">
<tr id="row-in-body">
<th>D</th>
<td>E</td>
<td>F</td>
</tr>
<tr>
<td id="g">G</td>
<td>H</td>
<td>I</td>
</tr>
</tbody>
</table>

<script>
if (window.accessibilityController) {
window.jsTestIsAsync = true;
var output = "";

var table = accessibilityController.accessibleElementById("table");
var cellB = table.cellForColumnAndRow(1, 0);
var cellE = table.cellForColumnAndRow(1, 1);

output += `Cell B should have 1 rowHeaders (has ${cellB.rowHeaders().length}) and 0 columnHeaders (has ${cellB.columnHeaders().length})\n`;
output += `Cell E should have 1 rowHeader (has ${cellE.rowHeaders().length}) and 1 columnHeader (has ${cellE.columnHeaders().length})\n`;

output += "Move row in <thead> to <tbody>:\n";
var headRow = document.getElementById("row-in-head");
var bodyRow = document.getElementById("row-in-body");
var thead = document.getElementById("table-head");

bodyRow.before(headRow);
thead.remove();

setTimeout(async () => {
await waitFor(() => table.cellForColumnAndRow(1, 0));
cellB = table.cellForColumnAndRow(1, 0);
output += `Cell B should have 1 rowHeader (has ${cellB.rowHeaders().length}) and 0 columnHeaders (has ${cellB.columnHeaders().length})\n`;
output += `Cell E should have 1 rowHeader (has ${cellE.rowHeaders().length}) and 0 columnHeaders (has ${cellE.columnHeaders().length})\n`;

output += "Add header above E:\n";

let th = document.createElement("th");
th.textContent = "J";
document.getElementById("a").after(th);

await waitFor(() => table.cellForColumnAndRow(1, 1).columnHeaders().length);
cellE = table.cellForColumnAndRow(1, 1);
output += `Cell E should have 1 rowHeaders (has ${cellE.rowHeaders().length}) and 1 columnHeader (has ${cellE.columnHeaders().length})\n`;

axDebug(output);
finishJSTest();
}, 0);
}
</script>
</body>
</html>
20 changes: 20 additions & 0 deletions Source/WebCore/accessibility/AXCoreObject.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,26 @@ 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)
{
if (!tableCell)
return false;

auto columnRange = columnIndexRange();
auto otherColumnRange = tableCell->columnIndexRange();

return columnRange.first <= otherColumnRange.first + otherColumnRange.second;
}

String AXCoreObject::ariaLandmarkRoleDescription() const
{
switch (roleValue()) {
Expand Down
7 changes: 7 additions & 0 deletions Source/WebCore/accessibility/AXCoreObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -893,6 +893,12 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
// 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 cellScope() const { 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.
Expand All @@ -908,6 +914,7 @@ class AXCoreObject : public ThreadSafeRefCountedAndCanMakeThreadSafeWeakPtr<AXCo
// Table row support.
virtual bool isTableRow() const = 0;
virtual unsigned rowIndex() const = 0;
virtual AXCoreObject* rowHeader() { return nullptr; }

// ARIA tree/grid row support.
virtual bool isARIATreeGridRow() const = 0;
Expand Down
3 changes: 3 additions & 0 deletions Source/WebCore/accessibility/AXLogger.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,9 @@ TextStream& operator<<(TextStream& stream, AXObjectCache::AXNotification notific
case AXObjectCache::AXNotification::AXRowSpanChanged:
stream << "AXRowSpanChanged";
break;
case AXObjectCache::AXNotification::AXCellScopeChanged:
stream << "AXCellScopeChanged";
break;
case AXObjectCache::AXNotification::AXSelectedChildrenChanged:
stream << "AXSelectedChildrenChanged";
break;
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/accessibility/AXObjectCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2446,6 +2446,8 @@ void AXObjectCache::handleAttributeChange(Element* element, const QualifiedName&
recomputeParentTableProperties(element, TableProperty::CellSlots);
} else if (attrName == popovertargetAttr)
postNotification(element, AXPopoverTargetChanged);
else if (attrName == scopeAttr)
postNotification(element, AXCellScopeChanged);

if (!attrName.localName().string().startsWith("aria-"_s))
return;
Expand Down Expand Up @@ -4155,6 +4157,9 @@ void AXObjectCache::updateIsolatedTree(const Vector<std::pair<RefPtr<Accessibili
case AXRowIndexChanged:
tree->updateNodeProperty(*notification.first, AXPropertyName::AXRowIndex);
break;
case AXCellScopeChanged:
tree->updateNodeProperties(*notification.first, { AXPropertyName::CellScope, 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:
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/accessibility/AXObjectCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ class AXObjectCache : public CanMakeWeakPtr<AXObjectCache>, public CanMakeChecke
AXRoleDescriptionChanged,
AXRowIndexChanged,
AXRowSpanChanged,
AXCellScopeChanged,
AXSelectedChildrenChanged,
AXSelectedCellsChanged,
AXSelectedStateChanged,
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityARIAGridRow.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ AccessibilityTable* AccessibilityARIAGridRow::parentTable() const
}));
}

AXCoreObject* AccessibilityARIAGridRow::headerObject()
AXCoreObject* AccessibilityARIAGridRow::rowHeader()
{
for (const auto& child : children()) {
if (child->roleValue() == AccessibilityRole::RowHeader)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityARIAGridRow.h
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class AccessibilityARIAGridRow final : public AccessibilityTableRow {
AccessibilityChildrenVector disclosedRows() override;
AXCoreObject* disclosedByRow() const override;

AXCoreObject* headerObject() override;
AXCoreObject* rowHeader() final;

private:
explicit AccessibilityARIAGridRow(RenderObject*);
Expand Down
3 changes: 1 addition & 2 deletions Source/WebCore/accessibility/AccessibilityObject.h
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
AccessibilityChildrenVector columnHeaders() override { return AccessibilityChildrenVector(); }
AccessibilityChildrenVector rowHeaders() override { return AccessibilityChildrenVector(); }
AccessibilityChildrenVector visibleRows() override { return AccessibilityChildrenVector(); }
String cellScope() const final { return getAttribute(HTMLNames::scopeAttr); }
AXCoreObject* headerContainer() override { return nullptr; }
int axColumnCount() const override { return 0; }
int axRowCount() const override { return 0; }
Expand All @@ -163,8 +164,6 @@ class AccessibilityObject : public AXCoreObject, public CanMakeWeakPtr<Accessibi
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; }

Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/accessibility/AccessibilityTable.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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).rowHeader())
headers.append(header);
}

Expand Down
Loading

0 comments on commit ad80707

Please sign in to comment.