AX: aria-hidden=false should be a synonym of undefined by hoffmanjoshua · Pull Request #32044 · WebKit/WebKit

hoffmanjoshua · 2024-08-12T17:26:16Z

`b365574`

AX: aria-hidden=false should be a synonym of undefined
https://bugs.webkit.org/show_bug.cgi?id=267150
rdar://120557669

Reviewed by Tyler Wilcock.

Per spec changes (see w3c/aria#2090, we should treat `aria-hidden=false` as undefined.

This patch removes support for aria-hidden=false, updating places where we relied on the behaviors of isNodeARIAVisible
with the new behavior. For example, we need to check if a child node is focused before deciding whether to skip it if it is
aria-hidden (tested by accessibility/datetime/input-date-field-labels-and-value-changes.html).

Tests that explicitly validate aria-hidden false were removed, and a new test to check that we are ignoring this property
has been added.

* LayoutTests/accessibility/aria-hidden-false-ignored-expected.txt: Added.
* LayoutTests/accessibility/aria-hidden-false-ignored.html: Added.
* LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html: Removed.
* LayoutTests/accessibility/aria-hidden-negates-no-visibility.html: Removed.
* LayoutTests/accessibility/aria-hidden-subtree-expected.txt: Added.
* LayoutTests/accessibility/aria-hidden-subtree.html: Added.
* LayoutTests/accessibility/aria-modal-expected.txt:
* LayoutTests/accessibility/aria-modal.html:
* LayoutTests/accessibility/aria-visible-element-roles.html: Removed.
* LayoutTests/accessibility/datetime/input-date-field-labels-and-value-changes.html:
* Source/WebCore/accessibility/AXObjectCache.cpp:
(WebCore::AXObjectCache::modalElementHasAccessibleContent):
(WebCore::AXObjectCache::isNodeVisible const):
(WebCore::AXObjectCache::getOrCreate):
(WebCore::isNodeFocused):
(WebCore::isNodeAriaVisible): Deleted.
* Source/WebCore/accessibility/AXObjectCache.h:
* Source/WebCore/accessibility/AccessibilityNodeObject.cpp:
(WebCore::AccessibilityNodeObject::textUnderElement const):
* Source/WebCore/accessibility/AccessibilityObject.cpp:
(WebCore::AccessibilityObject::defaultObjectInclusion const):
* Source/WebCore/accessibility/AccessibilityRenderObject.cpp:
(WebCore::AccessibilityRenderObject::addNodeOnlyChildren):

Canonical link: https://commits.webkit.org/284905@main

53b1903

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-08-12T17:26:23Z

EWS run on previous version of this PR (hash a6312ba)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	❌ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

twilco · 2024-08-12T18:32:35Z

AccessibilityRenderObject::addNodeOnlyChildren() has a comment that probably needs updating:

// Some elements don't have an associated render object, meaning they won't be picked up by a walk of the render tree.
// For example, nodes that are `aria-hidden="false"` and `hidden`, or elements with `display: contents`.
// This function will find and add these elements to the AX tree.
void AccessibilityRenderObject::addNodeOnlyChildren()

webkit-early-warning-system · 2024-08-12T19:28:29Z

EWS run on previous version of this PR (hash 0ca0121)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 wincairo
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 wincairo-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2
	✅ 🛠 tv
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-08-15T17:53:29Z

EWS run on previous version of this PR (hash b3432d6)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	~~🛠 wincairo~~
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 wincairo-tests~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 wpe-cairo~~
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	✅ 🛠 gtk
	✅ 🛠 vision	~~🧪 mac-AS-debug-wk2~~	⏳ 🧪 gtk-wk2
	✅ 🛠 vision-sim	~~🧪 mac-wk2-stress~~	⏳ 🧪 api-gtk
	~~🧪 vision-wk2~~
	~~🛠 tv~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-08-15T18:00:37Z

EWS run on previous version of this PR (hash 60d612e)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	⏳ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	⏳ 🧪 wpe-wk2	⏳ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	❌ 🧪 api-gtk
	✅ 🧪 vision-wk2	⏳ 🧪 mac-intel-wk2
	✅ 🛠 tv	⏳ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

twilco · 2024-09-29T17:20:25Z

The reason aria-modal-in-aria-hidden.html fails on glib platforms only is due to this snippet in AXObjectCache::modalElementHasAccessibleContent:

#if USE(ATSPI)
                // When using ATSPI, an accessibility object with 'StaticText' role is ignored.
                // Its content is exposed by its parent.
                // Treat such elements as having accessible content.
                if (axObject->roleValue() == AccessibilityRole::StaticText)
                    return true;
#endif

This unconditionally returns true for any static text object, even if it is aria-hidden. One possible way to fix this would be to change the if-statement to this:

if (axObject->roleValue() == AccessibilityRole::StaticText && !axObject->isAXHidden())
    return true;

And update the comment to mention that despite wanting to include static text on ATSPI, we still don't want aria-hidden text.

Ideally we could use axObject->defaultObjectInclusion() != AccessibilityObjectInclusion::IgnoreObject instead of isAXHidden(), as defaultObjectInclusion() also accounts for inert and visibility:hidden. However, at the end of defaultObjectInclusion(), accessibilityPlatformIncludesObject() is called, and for ATSPI that returns IgnoreObject for static text, breaking the intention of this if-statement.

So I think we should also add another line to this comment with a FIXME that this might not properly respect inert or visibility, file a follow-up bug stating this issue, and link to that bug in the FIXME comment.

webkit-early-warning-system · 2024-10-04T23:32:57Z

EWS run on previous version of this PR (hash 510e937)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	✅ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	✅ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-10-08T23:42:44Z

EWS run on previous version of this PR (hash 812643b)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	~~🛠 ios~~	~~🛠 mac~~	~~🛠 wpe~~	~~🛠 win~~
✅ 🧪 bindings	~~🛠 ios-sim~~	❌ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	~~🧪 win-tests~~
✅ 🧪 webkitperl	~~🧪 ios-wk2~~	~~🧪 api-mac~~	~~🧪 api-wpe~~
	~~🧪 ios-wk2-wpt~~	~~🧪 mac-wk1~~	~~🛠 wpe-cairo~~
	~~🧪 api-ios~~	~~🧪 mac-wk2~~	~~🛠 gtk~~
	~~🛠 vision~~	❌ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	~~🛠 vision-sim~~	~~🧪 mac-wk2-stress~~	~~🧪 api-gtk~~
	⏳ 🧪 vision-wk2	~~🧪 mac-intel-wk2~~
	❌ 🛠 tv	~~🛠 mac-safer-cpp~~
	~~🛠 tv-sim~~
	~~🛠 watch~~
	~~🛠 watch-sim~~

webkit-early-warning-system · 2024-10-08T23:54:52Z

EWS run on previous version of this PR (hash 1a99342)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	❌ 🧪 wpe-wk2	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	❌ 🧪 gtk-wk2
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

webkit-early-warning-system · 2024-10-09T14:45:20Z

EWS run on current version of this PR (hash 53b1903)

Details

Misc	iOS, visionOS, tvOS & watchOS	macOS	Linux	Windows
✅ 🧪 style	✅ 🛠 ios	✅ 🛠 mac	✅ 🛠 wpe	✅ 🛠 win
✅ 🧪 bindings	✅ 🛠 ios-sim	✅ 🛠 mac-AS-debug	~~🧪 wpe-wk2~~	✅ 🧪 win-tests
✅ 🧪 webkitperl	✅ 🧪 ios-wk2	✅ 🧪 api-mac	✅ 🧪 api-wpe
	✅ 🧪 ios-wk2-wpt	✅ 🧪 mac-wk1	✅ 🛠 wpe-cairo
	✅ 🧪 api-ios	✅ 🧪 mac-wk2	✅ 🛠 gtk
	✅ 🛠 vision	✅ 🧪 mac-AS-debug-wk2	~~🧪 gtk-wk2~~
	✅ 🛠 vision-sim	✅ 🧪 mac-wk2-stress	✅ 🧪 api-gtk
✅ 🛠 🧪 merge	✅ 🧪 vision-wk2	✅ 🧪 mac-intel-wk2
	✅ 🛠 tv	✅ 🛠 mac-safer-cpp
	✅ 🛠 tv-sim
	✅ 🛠 watch
	✅ 🛠 watch-sim

https://bugs.webkit.org/show_bug.cgi?id=267150 rdar://120557669 Reviewed by Tyler Wilcock. Per spec changes (see w3c/aria#2090), we should treat `aria-hidden=false` as undefined. This patch removes support for aria-hidden=false, updating places where we relied on the behaviors of isNodeARIAVisible with the new behavior. For example, we need to check if a child node is focused before deciding whether to skip it if it is aria-hidden (tested by accessibility/datetime/input-date-field-labels-and-value-changes.html). Tests that explicitly validate aria-hidden false were removed, and a new test to check that we are ignoring this property has been added. * LayoutTests/accessibility/aria-hidden-false-ignored-expected.txt: Added. * LayoutTests/accessibility/aria-hidden-false-ignored.html: Added. * LayoutTests/accessibility/aria-hidden-false-works-in-subtrees.html: Removed. * LayoutTests/accessibility/aria-hidden-negates-no-visibility.html: Removed. * LayoutTests/accessibility/aria-hidden-subtree-expected.txt: Added. * LayoutTests/accessibility/aria-hidden-subtree.html: Added. * LayoutTests/accessibility/aria-modal-expected.txt: * LayoutTests/accessibility/aria-modal.html: * LayoutTests/accessibility/aria-visible-element-roles.html: Removed. * LayoutTests/accessibility/datetime/input-date-field-labels-and-value-changes.html: * Source/WebCore/accessibility/AXObjectCache.cpp: (WebCore::AXObjectCache::modalElementHasAccessibleContent): (WebCore::AXObjectCache::isNodeVisible const): (WebCore::AXObjectCache::getOrCreate): (WebCore::isNodeFocused): (WebCore::isNodeAriaVisible): Deleted. * Source/WebCore/accessibility/AXObjectCache.h: * Source/WebCore/accessibility/AccessibilityNodeObject.cpp: (WebCore::AccessibilityNodeObject::textUnderElement const): * Source/WebCore/accessibility/AccessibilityObject.cpp: (WebCore::AccessibilityObject::defaultObjectInclusion const): * Source/WebCore/accessibility/AccessibilityRenderObject.cpp: (WebCore::AccessibilityRenderObject::addNodeOnlyChildren): Canonical link: https://commits.webkit.org/284905@main

webkit-commit-queue · 2024-10-09T17:56:50Z

Committed 284905@main (b365574): https://commits.webkit.org/284905@main

Reviewed commits have been landed. Closing PR #32044 and removing active labels.

hoffmanjoshua self-assigned this Aug 12, 2024

hoffmanjoshua added the Accessibility For bugs related to accessibility. label Aug 12, 2024

hoffmanjoshua requested review from AndresGonzalezApple and twilco August 12, 2024 17:27

twilco reviewed Aug 12, 2024

View reviewed changes

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 12, 2024

hoffmanjoshua removed the merging-blocked Applied to prevent a change from being merged label Aug 12, 2024

hoffmanjoshua force-pushed the eng/AX-aria-hiddenfalse-should-be-a-synonym-of-undefined branch from a6312ba to 0ca0121 Compare August 12, 2024 19:28

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 12, 2024

AndresGonzalezApple reviewed Aug 13, 2024

View reviewed changes

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated

hoffmanjoshua removed the merging-blocked Applied to prevent a change from being merged label Aug 15, 2024

hoffmanjoshua force-pushed the eng/AX-aria-hiddenfalse-should-be-a-synonym-of-undefined branch from 0ca0121 to b3432d6 Compare August 15, 2024 17:53

hoffmanjoshua force-pushed the eng/AX-aria-hiddenfalse-should-be-a-synonym-of-undefined branch from b3432d6 to 60d612e Compare August 15, 2024 17:59

webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Aug 15, 2024

hoffmanjoshua removed the merging-blocked Applied to prevent a change from being merged label Oct 4, 2024

hoffmanjoshua force-pushed the eng/AX-aria-hiddenfalse-should-be-a-synonym-of-undefined branch from 60d612e to 510e937 Compare October 4, 2024 23:32

hoffmanjoshua commented Oct 4, 2024

View reviewed changes

Comment thread Source/WebCore/accessibility/AXObjectCache.cpp Outdated