Bug 254317

Summary: AX: article speaks "landmark" and shows up in landmarks rotor on iOS-only
Product: WebKit Reporter: James Craig <jcraig>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, andresg_22, apinheiro, cfleizach, dmazzoni, ews-watchlist, jdiggs, samuel_white, tyler_w, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch none

James Craig
Reported 2023-03-22 23:02:13 PDT
There were some comments on a public accessibility slack that WebKit/VoiceOver vends landmarks inconsistently between the Mac and iOS systems. In particular, role="article" is not in the landmarks list for the ARIA spec, but iOS treats it similar to one. The role description is spoken as "article, landmark" and articles appear in the landmark rotor. Neither of these is true on Mac. Of note, the Mac platform roles for landmarks all start with AXLandmark*: AXLandmarkMain, AXLandmarkNavigation, etc... but article is exposed as "AXDocumentArticle" so the bug is probably either downstream in the iOS mapping or outside of WebKit. The commenter's motivation is not clear to me the yet. I'm unsure if there is real user workflow hindered by the verbosity or rotor, or if the report is purely academic. In any case, I agree that the behavior between platforms should be consistent, unless the VO users on the QA team requested this difference for some reason.
Attachments
Patch (3.55 KB, patch)
2023-03-31 15:08 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
Patch (3.83 KB, patch)
2023-03-31 15:13 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
Patch (4.27 KB, patch)
2023-03-31 23:34 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
Patch (5.21 KB, patch)
2023-04-07 15:20 PDT, chris fleizach 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Radar WebKit Bug Importer
Comment 1 2023-03-22 23:02:34 PDT
<rdar://problem/107118745>
James Craig
Comment 2 2023-03-23 14:54:45 PDT
Commenter has clarified it's the academic reason. He considers this a bug because articles are in the landmark rotor even though they aren't technically landmarks. Tracing it back, this decision was made on purpose (around 10 years ago and perhaps with the input of VO users on the Accessibility Design and Quality team) to reduce the proliferation of additional rotors, but we're now revisiting that topic. Outcome indeterminate.
chris fleizach
Comment 3 2023-03-31 15:08:13 PDT
Created attachment 465715 [details] Patch
chris fleizach
Comment 4 2023-03-31 15:13:15 PDT
Created attachment 465716 [details] Patch
James Craig
Comment 5 2023-03-31 15:20:07 PDT
Comment on attachment 465716 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=465716&action=review > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:-670 > - AccessibilityRole::LandmarkDocRegion, What's the ARIA computedrole for LandmarkDocRegion? "region"? If so, that's included with the landmarks list https://w3c.github.io/aria/#landmark_roles
chris fleizach
Comment 6 2023-03-31 15:24:42 PDT
(In reply to James Craig from comment #5) > Comment on attachment 465716 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=465716&action=review > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:-670 > > - AccessibilityRole::LandmarkDocRegion, > > What's the ARIA computedrole for LandmarkDocRegion? "region"? If so, that's > included with the landmarks list https://w3c.github.io/aria/#landmark_roles Group role { AccessibilityRole::LandmarkDocRegion, NSAccessibilityGroupRole },
Tyler Wilcock
Comment 7 2023-03-31 16:00:53 PDT
(In reply to chris fleizach from comment #6) > (In reply to James Craig from comment #5) > > Comment on attachment 465716 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=465716&action=review > > > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:-670 > > > - AccessibilityRole::LandmarkDocRegion, > > > > What's the ARIA computedrole for LandmarkDocRegion? "region"? If so, that's > > included with the landmarks list https://w3c.github.io/aria/#landmark_roles > > Group role > > { AccessibilityRole::LandmarkDocRegion, NSAccessibilityGroupRole }, The ARIA computed role for LandmarkDocRegion is "region" based on this sequence: AccessibilityObject::computedRoleString() special-cases accessibilityrole::LandmarkDocRegion to defer to AccessibilityRole::LandmarkRegion: https://github.com/WebKit/WebKit/blob/656c0aeae78bfa54346b7a5f5ed224574dd0b78d/Source/WebCore/accessibility/AccessibilityObject.cpp#L2573#L2574 Then we look up AccessibilityRole::LandmarkRegion in the reverse role map: https://github.com/WebKit/WebKit/blob/656c0aeae78bfa54346b7a5f5ed224574dd0b78d/Source/WebCore/accessibility/AccessibilityObject.cpp#L2475 So to James' point, it seems like we shouldn't remove AccessibilityRole::LandmarkDocRegion from `_accessibilityLandmarkAncestor`.
chris fleizach
Comment 8 2023-03-31 17:11:41 PDT
(In reply to Tyler Wilcock from comment #7) > (In reply to chris fleizach from comment #6) > > (In reply to James Craig from comment #5) > > > Comment on attachment 465716 [details] > > > Patch > > > > > > View in context: > > > https://bugs.webkit.org/attachment.cgi?id=465716&action=review > > > > > > > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:-670 > > > > - AccessibilityRole::LandmarkDocRegion, > > > > > > What's the ARIA computedrole for LandmarkDocRegion? "region"? If so, that's > > > included with the landmarks list https://w3c.github.io/aria/#landmark_roles > > > > Group role > > > > { AccessibilityRole::LandmarkDocRegion, NSAccessibilityGroupRole }, > The ARIA computed role for LandmarkDocRegion is "region" based on this > sequence: > > AccessibilityObject::computedRoleString() special-cases > accessibilityrole::LandmarkDocRegion to defer to > AccessibilityRole::LandmarkRegion: > > https://github.com/WebKit/WebKit/blob/ > 656c0aeae78bfa54346b7a5f5ed224574dd0b78d/Source/WebCore/accessibility/ > AccessibilityObject.cpp#L2573#L2574 > > Then we look up AccessibilityRole::LandmarkRegion in the reverse role map: > > https://github.com/WebKit/WebKit/blob/ > 656c0aeae78bfa54346b7a5f5ed224574dd0b78d/Source/WebCore/accessibility/ > AccessibilityObject.cpp#L2475 > > So to James' point, it seems like we shouldn't remove > AccessibilityRole::LandmarkDocRegion from `_accessibilityLandmarkAncestor`. Ok sounds good
chris fleizach
Comment 9 2023-03-31 23:34:26 PDT
Created attachment 465721 [details] Patch
chris fleizach
Comment 10 2023-04-07 15:20:42 PDT
Created attachment 465814 [details] Patch
Tyler Wilcock
Comment 11 2023-04-07 15:29:04 PDT
Comment on attachment 465814 [details] Patch r+ after iOS-simulator tests pass
EWS
Comment 12 2023-04-09 23:58:22 PDT
Committed 262763@main (ff11799319a9): <https://commits.webkit.org/262763@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465814 [details].
Note You need to log in before you can comment on or make changes to this bug.