Bug 218316 – AX: VoiceOver does not announce the aria-checked state for ARIA treeitem

• Component: Accessibility
• Status: RESOLVED FIXED

[Todd Kloots]

Created attachment 412593 [details]
Screen recording using an ARIA tree with the aria-checked state in Safari with VoiceOver

What steps will reproduce the problem?
1. Start VoiceOver
2. Goto https://codepen.io/toddlr/pen/PozQELp
3. Move focus to the ARIA tree instance
4. Use the up/down arrow keys to move focus through the ARIA treeitem instances
5. Notice the aria-checked state is not announced
6. Use the spacebar to toggle the aria-checked state
7. Notice the aria-checked state is not announced when the value changes

What is the expected result?
VoiceOver should announce the aria-checked state


What happens instead of that?
VoiceOver does not announce the aria-checked state

[Radar WebKit Bug Importer]

<rdar://problem/70787809>

[chris fleizach]

Note that this fix will require a WebKit and a VoiceOver change. The VoiceOver bug will not have visibility on this website

[chris fleizach]

Created attachment 414697 [details]
patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #3)
> Created attachment 414697 [details]
> patch

Why a Mac only test since seems you are adding it for iOS too?

[chris fleizach]

(In reply to Andres Gonzalez from comment #4)
> (In reply to chris fleizach from comment #3)
> > Created attachment 414697 [details]
> > patch
> 
> Why a Mac only test since seems you are adding it for iOS too?

Will add one

[Andres Gonzalez]

--- a/Source/WebCore/accessibility/AccessibilityObject.cpp
+++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
@@ -2736,6 +2736,11 @@ bool AccessibilityObject::supportsRowCountChange() const
     }
 }

+bool AccessibilityObject::treeItemSupportsCheckedState() const
+{
+    return roleValue() == AccessibilityRole::TreeItem && hasAttribute(aria_checkedAttr);
+}
+

Shouldn't we add this to the AccessibilityTreeItem::supportsCheckedState instead of adding a new method to the interface?

[chris fleizach]

(In reply to Andres Gonzalez from comment #6)
> --- a/Source/WebCore/accessibility/AccessibilityObject.cpp
> +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
> @@ -2736,6 +2736,11 @@ bool AccessibilityObject::supportsRowCountChange()
> const
>      }
>  }
> 
> +bool AccessibilityObject::treeItemSupportsCheckedState() const
> +{
> +    return roleValue() == AccessibilityRole::TreeItem &&
> hasAttribute(aria_checkedAttr);
> +}
> +
> 
> Shouldn't we add this to the AccessibilityTreeItem::supportsCheckedState
> instead of adding a new method to the interface?

Yes we can do that, I just get nervous about methods only in one class which sometimes causes confusion when we do a bad cast

[Andres Gonzalez]

(In reply to chris fleizach from comment #7)
> (In reply to Andres Gonzalez from comment #6)
> > --- a/Source/WebCore/accessibility/AccessibilityObject.cpp
> > +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp
> > @@ -2736,6 +2736,11 @@ bool AccessibilityObject::supportsRowCountChange()
> > const
> >      }
> >  }
> > 
> > +bool AccessibilityObject::treeItemSupportsCheckedState() const
> > +{
> > +    return roleValue() == AccessibilityRole::TreeItem &&
> > hasAttribute(aria_checkedAttr);
> > +}
> > +
> > 
> > Shouldn't we add this to the AccessibilityTreeItem::supportsCheckedState
> > instead of adding a new method to the interface?
> 
> Yes we can do that, I just get nervous about methods only in one class which
> sometimes causes confusion when we do a bad cast

It would be much cleaner. The need of casting AccessibilityObjects to its subclasses is greatly diminished because AXCoreObjects are not just AccessibilityObjects any more, but also AXIsolatedObjects. So as we move clients to using AXCoreObject, they cannot cast any more, which is a good thing.

[Andres Gonzalez]

When I said clients above I meant platform code as the client of the core classes.

[chris fleizach]

Created attachment 414755 [details]
patch

[chris fleizach]

Created attachment 414768 [details]
patch

[chris fleizach]

Created attachment 415032 [details]
patch

[chris fleizach]

Created attachment 415068 [details]
Patch

[Andres Gonzalez]

(In reply to chris fleizach from comment #13)
> Created attachment 415068 [details]
> Patch

Looks good. Just a couple of minor issues.

--- a/LayoutTests/accessibility/ios-simulator/checked-status-tree-items.html
+++ a/LayoutTests/accessibility/ios-simulator/checked-status-tree-items.html

+        var treeItemNotChecked = accessibilityController.accessibleElementById("treeitem_notchecked");

This is not used and it will be null since there is no element with that id attribute.

--- a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h
+++ a/Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.h

+    bool supportsCheckedState() const override { return boolAttributeValue(AXPropertyName::SupportsCheckedState); };

Extra ; at the end of the line.

[chris fleizach]

Created attachment 415089 [details]
patch

Thanks Andres -- addressed.

[EWS]

Committed r270333: <https://trac.webkit.org/changeset/270333>

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