Bug 227906 - AX: nested role="presentation" elements break role="tree" behavior
Summary: AX: nested role="presentation" elements break role="tree" behavior
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: Safari 14
Hardware: All macOS 11
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-07-13 08:46 PDT by Sufian Rhazi
Modified: 2023-11-30 16:00 PST (History)
13 users (show)

See Also:


Attachments
Patch (5.96 KB, patch)
2022-12-19 23:49 PST, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (5.96 KB, patch)
2022-12-20 08:48 PST, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (5.95 KB, patch)
2022-12-20 22:23 PST, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (5.86 KB, patch)
2022-12-27 00:09 PST, chris fleizach
no flags Details | Formatted Diff | Diff
Patch (5.29 KB, patch)
2023-01-03 23:58 PST, chris fleizach
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Sufian Rhazi 2021-07-13 08:46:28 PDT
When there are 2 or more nested role="presentation" elements as direct children between role="tree" and its owned role="treeitem" elements, the tree role semantics are lost.

There are several issues in this case:
- The role="tree" element is not read by VoiceOver as if it is a "table" (losing the "Entering table" and "Leaving table" announcements)
- When navigating via VO-left and VO-right, the role="tree" element is not a distinct element; navigation switches between the individual role="treeitem" items. As such, you cannot user VO-shift-down / VO-shift-up to go into/out from the inner tree navigation, and cannot use VO-down / VO-up to navigate the leaves
- The role="treeitem" elements do not retain the treeitem semantics, so the aria-checked="true" or aria-checked="false" states are not announced

A case showing the differences can be found at this codepen URL: https://codepen.io/sufianrhazi/pen/zYwoaNb -- trees with nesting 3 and 2 have navigational differences and do not announce "checked" / "unchecked", the trees with nesting 1 and 0 behave as expected.

Note: this reproduction case is simplified to use only one level of role="treeitem" elements, with no role="group" grouping.

Reproduced on Safari Version 14.1.1 (16611.2.7.1.4) with MacOS 11.4.

In other browsers on MacOS:

Chrome Version 91.0.4472.114 on MacOS 11.4 behaves as expected:
* Retains the tree semantics of the role="tree" element
* The "Entering table" / "Leaving table" announcement is made
* Tree is treated as a single VO-left/VO-right item
* Tree can be entered/exited via VO-shift-down/VO-shift-up
* When entered can be navigated via VO-down/VO-up, and the aria-checked="true"/aria-checked="false" states on role="treeitem" elements are announced and disabled states are announced
* When navigated via up/down arrow keys, the aria-checked states on role="treeitem" elements are announced

Firefox Version 90.0 on MacOS 11.4 behaves mostly as expected, except for aria-checked announcement:
* Retains the tree semantics of the role="tree" element
* The "Entering table" / "Leaving table" announcement is made
* Tree is treated as a single VO-left/VO-right item
* Tree can be entered/exited via VO-shift-down/VO-shift-up
* When entered can be navigated via VO-down/VO-up, and the aria-checked="true"/aria-checked="false" states on role="treeitem" elements are NOT announced but disabled states are announced
* When navigated via up/down arrow keys, the aria-checked states on role="treeitem" elements are NOT announced
Comment 1 Radar WebKit Bug Importer 2021-07-13 08:46:38 PDT
<rdar://problem/80520596>
Comment 2 Sommer Panage 2022-11-15 18:14:21 PST
Confirmed that this still reproduces on 13.1 Beta (22C5044e)
Comment 3 chris fleizach 2022-12-19 23:49:02 PST
Created attachment 464114 [details]
Patch
Comment 4 chris fleizach 2022-12-20 08:48:25 PST
Created attachment 464122 [details]
Patch
Comment 5 Tyler Wilcock 2022-12-20 09:02:10 PST
Comment on attachment 464122 [details]
Patch

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

> LayoutTests/accessibility/mac/nested-tree-presentation-roles.html:40
> +    window.jsTestIsAsync = true;

This test shouldn't need to be async because there is no JS causing dynamic changes. We like to avoid making tests async when possible because doing so unnecessarily can mask issues in ITM.
Comment 6 chris fleizach 2022-12-20 22:23:02 PST
Created attachment 464142 [details]
Patch
Comment 7 Tyler Wilcock 2022-12-21 11:24:36 PST
Comment on attachment 464142 [details]
Patch

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

Sorry for missing these nits on the last round of review.

> COMMIT_MESSAGE:7
> +Trees can have arbitrary nested of presentational roles. Our checks before were too strict.

"Trees can have arbitrary nested of presentational roles."

The grammar is a little off here. Maybe it could read something like: Trees can validly have arbitrary levels of nesting of role="presentational" elements.

> LayoutTests/accessibility/mac/nested-tree-presentation-roles.html:41
> +    var slider = accessibilityController.accessibleElementById("item1");

This variable isn't ever used in the test.

> LayoutTests/accessibility/mac/nested-tree-presentation-roles.html:43
> +    output += expect("accessibilityController.accessibleElementById('item1').boolAttributeValue('AXSelected')", "false");

Can probably remove all newlines between this one and "if (window.accessibilityController) {"
Comment 8 Tyler Wilcock 2022-12-21 11:25:59 PST
> > COMMIT_MESSAGE:7
> > +Trees can have arbitrary nested of presentational roles. Our checks before were too strict.
> 
> "Trees can have arbitrary nested of presentational roles."
> 
> The grammar is a little off here. Maybe it could read something like: Trees
> can validly have arbitrary levels of nesting of role="presentational"
> elements.
Or maybe just "Trees can have arbitrary nesting of presentational roles"
Comment 9 chris fleizach 2022-12-27 00:09:35 PST
Created attachment 464222 [details]
Patch
Comment 10 Andres Gonzalez 2023-01-03 13:59:12 PST
(In reply to chris fleizach from comment #9)
> Created attachment 464222 [details]
> Patch

diff --git a/LayoutTests/accessibility/mac/nested-tree-presentation-roles.html b/LayoutTests/accessibility/mac/nested-tree-presentation-roles.html
new file mode 100644
index 000000000000..6d83d363fa0a
--- /dev/null
+++ b/LayoutTests/accessibility/mac/nested-tree-presentation-roles.html

+    output += expect("accessibilityController.accessibleElementById('item1').boolAttributeValue('AXSelected')", "false");

This attribute can return a false value even if it fails to retrieve the value from the underlying AX object.:

bool AccessibilityUIElement::boolAttributeValue(NSString *attribute) const
{
    BEGIN_AX_OBJC_EXCEPTIONS
    auto value = attributeValue(attribute);
    if ([value isKindOfClass:[NSNumber class]])
        return [value boolValue];
    END_AX_OBJC_EXCEPTIONS

    return false;
}

To make it a stricter test, we can make aria-checked="true" for item1 and check for that instead.

You can get rid of all the class and tabindex attributes that are not used.
Comment 11 chris fleizach 2023-01-03 23:58:18 PST
Created attachment 464322 [details]
Patch
Comment 12 EWS 2023-01-04 09:20:21 PST
Committed 258435@main (3f6b03504654): <https://commits.webkit.org/258435@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 464322 [details].
Comment 13 Sommer Panage 2023-01-04 10:58:10 PST
Many thanks to you Chris, Tyler and Andres for getting this one in! ~S
Comment 14 Todd Kloots 2023-11-30 16:00:45 PST
Verified fixed. Tested using Mac OS 14.1.1 in both Chrome (Version 119.0.6045.199) and Safari (Version 17.1 (19616.2.9.11.7)). Tested using a forked version of Sufian's original example (https://codepen.io/toddlr/pen/gOqBmaq) as it makes it far easier to confirm checked state is reported correctly by VO.