Bug 252941 – AX: Adding display:block to table rows inside a header makes the entire table inaccessible

• Status: NEW
• Component: Accessibility

[Tyler Wilcock]

Adding display:block to table rows inside a header makes the entire table inaccessible.

[Radar WebKit Bug Importer]

<rdar://problem/105912396>

[Tyler Wilcock]

Created attachment 465165 [details]
Patch

[chris fleizach]

Comment on attachment 465165 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=465165&action=review

> Source/WebCore/accessibility/AccessibilityTableRow.cpp:114
> +    while (isAnonymousTablePart(parent))

Is there a chance patent becomes nil and we loop forever

> Source/WebCore/accessibility/AccessibilityTableRow.cpp:116
> +    return parent;

Do we need to implement this in parent object unignored if we’re already doing it in parent object ?

[Andres Gonzalez]

(In reply to Tyler Wilcock from comment #2)
> Created attachment 465165 [details]
> Patch

"Because no `AXIsolatedObject` was created for these anonymous renderers (as expected), `AXIsolatedTree::nodeForID` failed, breaking the tree hierarchy."

Why it is expected that no isolated object is created for an anonymous renderer. Because it is ignored? If so, that's the actual cause of the problem.

--- a/Source/WebCore/accessibility/AccessibilityTableRow.cpp
+++ b/Source/WebCore/accessibility/AccessibilityTableRow.cpp
@@ -90,7 +90,32 @@ bool AccessibilityTableRow::computeAccessibilityIsIgnored() const

     return false;
 }
-
+
+static bool isAnonymousTablePart(AccessibilityObject* axObject)
+{
+    if (auto* renderer = axObject ? axObject->renderer() : nullptr)
+        return renderer->isAnonymous() && renderer->isTablePart();
+    return false;
+}
+
+AccessibilityObject* AccessibilityTableRow::parentObject() const
+{
+    auto* parent = AccessibilityRenderObject::parentObject();
+    // For some combinations of display values given to <tr> / <tbody>, anonymous table rows / cells
+    // can end up parenting non-anonymous table rows in the render tree. Skip past these.
+    while (isAnonymousTablePart(parent))
+        parent = parent->parentObject();
+    return parent;
+}
+

Can we make isAnonymousTablePart() an AccessibilityObject or AccessibilityRenderObject method instead?

+AccessibilityObject* AccessibilityTableRow::parentObjectUnignored() const
+{
+    auto* parent = AccessibilityRenderObject::parentObjectUnignored();
+    while (isAnonymousTablePart(parent))
+        parent = parent->parentObjectUnignored();
+    return parent;
+}
+
We shouldn't need to override parentObjectUnignored since it is a composite of parentObject and isIgnored.