Bug 236156 - AX: The isolated tree needs to fix-up table cell parent relationships
Summary: AX: The isolated tree needs to fix-up table cell parent relationships
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Tyler Wilcock
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-02-04 13:56 PST by Tyler Wilcock
Modified: 2022-02-07 13:20 PST (History)
10 users (show)

See Also:


Attachments
Patch (9.48 KB, patch)
2022-02-04 14:01 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (15.19 KB, patch)
2022-02-05 08:39 PST, Tyler Wilcock
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (15.19 KB, patch)
2022-02-05 08:45 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff
Patch (16.04 KB, patch)
2022-02-05 22:53 PST, Tyler Wilcock
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tyler Wilcock 2022-02-04 13:56:39 PST
Tables are not very navigable by AX clients in isolated tree mode. This is because our current representation of table elements doesn't perfectly follow the normal parent-child relationship of other elements, i.e. some non-row elements can return cells from children(), but the cells must regard their row as the "real" parent. When creating the isolated tree, we need to work around this.

We've had to work around this in the live tree in AccessibilityObject::insertChild, too:

https://github.com/WebKit/WebKit/blob/main/Source/WebCore/accessibility/AccessibilityObject.cpp#L626#L628
Comment 1 Radar WebKit Bug Importer 2022-02-04 13:56:50 PST
<rdar://problem/88506756>
Comment 2 Tyler Wilcock 2022-02-04 14:01:58 PST
Created attachment 450936 [details]
Patch
Comment 3 chris fleizach 2022-02-04 14:25:21 PST
Comment on attachment 450936 [details]
Patch

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

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.cpp:212
> +

extra newline

> Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:389
> +    AXID computeParentForObject(AXCoreObject&, AXID assumedParentID);

feel like compute might not be a necessary verb here

is 

parentIDForObject() 

sufficient?
Comment 4 Tyler Wilcock 2022-02-04 14:54:01 PST
> > Source/WebCore/accessibility/isolatedtree/AXIsolatedTree.h:389
> > +    AXID computeParentForObject(AXCoreObject&, AXID assumedParentID);
> 
> feel like compute might not be a necessary verb here
> 
> is 
> 
> parentIDForObject() 
> 
> sufficient?
Yeah, that sounds good to me. Will change in the next revision of the patch.
Comment 5 Tyler Wilcock 2022-02-05 08:39:05 PST
Created attachment 450986 [details]
Patch
Comment 6 Tyler Wilcock 2022-02-05 08:45:55 PST
Created attachment 450987 [details]
Patch
Comment 7 Tyler Wilcock 2022-02-05 22:53:35 PST
Created attachment 451025 [details]
Patch
Comment 8 Andres Gonzalez 2022-02-07 06:13:52 PST
Can we fix the table hierarchy in the live tree instead? 

Does this fix some of the existing table tests that are failing in ITM?

Thanks.
Comment 9 Tyler Wilcock 2022-02-07 10:43:39 PST
(In reply to Andres Gonzalez from comment #8)
> Can we fix the table hierarchy in the live tree instead?
The problem is that AccessibilityTableColumn and AccessibilityTableHeaderContainer return children that they shouldn't (e.g table cells). The AccessibilityTableRows also (rightly) return cells as their children.

Unfortunately, it seems that AppKit expects that AccessibilityTableColumn and AccessibilityTableHeaderContainer return these children, so I think this workaround is required.

> Does this fix some of the existing table tests that are failing in ITM?
No, seems like those tests are failing for other reasons, and we don't have a test that exercises search traversal of a table (until this patch).
Comment 10 EWS 2022-02-07 13:20:43 PST
Committed r289238 (246922@main): <https://commits.webkit.org/246922@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 451025 [details].