Bug 236156

Summary: AX: The isolated tree needs to fix-up table cell parent relationships
Product: WebKit Reporter: Tyler Wilcock <tyler_w>
Component: AccessibilityAssignee: Tyler Wilcock <tyler_w>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jcraig, jdiggs, samuel_white, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
ews-feeder: commit-queue-
Patch
none
Patch none

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].