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
<rdar://problem/70787809>
Note that this fix will require a WebKit and a VoiceOver change. The VoiceOver bug will not have visibility on this website
Created attachment 414697 [details] patch
(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?
(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
--- 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?
(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
(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.
When I said clients above I meant platform code as the client of the core classes.
Created attachment 414755 [details] patch
Created attachment 414768 [details] patch
Created attachment 415032 [details] patch
Created attachment 415068 [details] Patch
(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.
Created attachment 415089 [details] patch Thanks Andres -- addressed.
Committed r270333: <https://trac.webkit.org/changeset/270333> All reviewed patches have been landed. Closing bug and clearing flags on attachment 415089 [details].