Skip to content

AX: accessibility/clip-path-bounding-box.html fails in macOS Tahoe because AccessibilityNodeObject and AccessibilityRenderObject don't explicitly enforce a minimum size for controls #47087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Jun 24, 2025
Merged

AX: accessibility/clip-path-bounding-box.html fails in macOS Tahoe because AccessibilityNodeObject and AccessibilityRenderObject don't explicitly enforce a minimum size for controls #47087

merged 1 commit into from
Jun 24, 2025

Conversation

twilco
Copy link
Contributor

@twilco twilco commented Jun 23, 2025 •
edited by webkit-early-warning-system
Loading

f79edb6

AX: accessibility/clip-path-bounding-box.html fails in macOS Tahoe because AccessibilityNodeObject and AccessibilityRenderObject don't explicitly enforce a minimum size for controls
https://bugs.webkit.org/show_bug.cgi?id=294868
rdar://154135798

Reviewed by Joshua Hoffman.

With this commit, we now enforce a minimum size for controls for main-thread accessibility objects (matching the same
change done for AXIsolatedObjects some time ago), ensuring the rects we return are large enough for assistive technologies
to draw a cursor for. This is helpful for low-vision users who rely on magnifying the cursor contents — having no cursor
(as happens for 1x1px elements in ATs like VoiceOver) can be disorienting, especially for interactive elements like controls.

Fixes accessibility/clip-path-bounding-box.html for macOS Tahoe. We used to implicitly get this behavior, but no longer
do after the controls redesign in Tahoe, so this commit makes it explicit to fix the test. This logic, which was in
AXIsolatedObject, is now moved to the WebAccessibilityObjectWrapperMac so both object subclasses benefit from it.

* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::Accessibility::adjustControlRect):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::relativeFrame const):

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

6a37f1b

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 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@twilco twilco self-assigned this Jun 23, 2025
@twilco twilco added the Accessibility For bugs related to accessibility. label Jun 23, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 23, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 3d35aee)

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 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

minorninth
minorninth reviewed Jun 24, 2025
View reviewed changes
minorninth
minorninth approved these changes Jun 24, 2025
View reviewed changes
Copy link
Contributor

@minorninth minorninth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

My only question is whether this is the right layer. By enforcing this for every object in the tree at this level, that might prevent us from querying whether an object has 0 width or height in other heuristics.

Instead, what if we did this at the level of WebAccessibilityObjectWrapperXXX, so only when we're exposing the platform bounds we enforce a minimum size.

@twilco twilco force-pushed the eng/AX-accessibility-clip-path-bounding-box-html-fails-in-macOS-Tahoe-because-AccessibilityNodeObject-and-AccessibilityRenderObject-don-t-explicitly-enforce-a-minimum-size-for-controls branch from 3d35aee to 03cfe20 Compare June 24, 2025 01:48
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on previous version of this PR (hash 03cfe20)

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 ⏳ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@twilco twilco force-pushed the eng/AX-accessibility-clip-path-bounding-box-html-fails-in-macOS-Tahoe-because-AccessibilityNodeObject-and-AccessibilityRenderObject-don-t-explicitly-enforce-a-minimum-size-for-controls branch from 03cfe20 to 6a37f1b Compare June 24, 2025 01:49
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 24, 2025 •
edited
Loading

EWS run on current version of this PR (hash 6a37f1b)

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 ✅ 🛠 playstation
✅ 🛠 tv ✅ 🛠 mac-safer-cpp
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@twilco twilco added the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
@twilco
AX: accessibility/clip-path-bounding-box.html fails in macOS Tahoe be…
f79edb6 
…cause AccessibilityNodeObject and AccessibilityRenderObject don't explicitly enforce a minimum size for controls

https://bugs.webkit.org/show_bug.cgi?id=294868
rdar://154135798

Reviewed by Joshua Hoffman.

With this commit, we now enforce a minimum size for controls for main-thread accessibility objects (matching the same
change done for AXIsolatedObjects some time ago), ensuring the rects we return are large enough for assistive technologies
to draw a cursor for. This is helpful for low-vision users who rely on magnifying the cursor contents — having no cursor
(as happens for 1x1px elements in ATs like VoiceOver) can be disorienting, especially for interactive elements like controls.

Fixes accessibility/clip-path-bounding-box.html for macOS Tahoe. We used to implicitly get this behavior, but no longer
do after the controls redesign in Tahoe, so this commit makes it explicit to fix the test. This logic, which was in
AXIsolatedObject, is now moved to the WebAccessibilityObjectWrapperMac so both object subclasses benefit from it.

* Source/WebCore/accessibility/AXCoreObject.h:
(WebCore::Accessibility::adjustControlRect):
* Source/WebCore/accessibility/isolatedtree/AXIsolatedObject.cpp:
(WebCore::AXIsolatedObject::relativeFrame const):

Canonical link: https://commits.webkit.org/296550@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/AX-accessibility-clip-path-bounding-box-html-fails-in-macOS-Tahoe-because-AccessibilityNodeObject-and-AccessibilityRenderObject-don-t-explicitly-enforce-a-minimum-size-for-controls branch from 6a37f1b to f79edb6 Compare June 24, 2025 04:22
@webkit-commit-queue
Copy link
Collaborator

Committed 296550@main (f79edb6): https://commits.webkit.org/296550@main

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

@webkit-commit-queue webkit-commit-queue merged commit f79edb6 into WebKit:main Jun 24, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jun 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@minorninth minorninth minorninth approved these changes

@hoffmanjoshua hoffmanjoshua hoffmanjoshua approved these changes

Assignees

@twilco twilco

Labels
Accessibility For bugs related to accessibility.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@twilco @webkit-early-warning-system @webkit-commit-queue @minorninth @hoffmanjoshua