REOPENED 203257
AX: Implement support for new ARIA roles: strong and emphasis
https://bugs.webkit.org/show_bug.cgi?id=203257
Summary AX: Implement support for new ARIA roles: strong and emphasis
Attachments
V1: Subroles for emphasis and strong (31.68 KB, patch)
2019-10-22 14:26 PDT, Joanmarie Diggs
no flags
V2: NO Subroles for emphasis and strong (31.52 KB, patch)
2019-10-22 14:54 PDT, Joanmarie Diggs
no flags
Radar WebKit Bug Importer
Comment 1 2019-10-22 10:56:58 PDT
Joanmarie Diggs
Comment 2 2019-10-22 14:26:06 PDT
Created attachment 381604 [details] V1: Subroles for emphasis and strong
Joanmarie Diggs
Comment 3 2019-10-22 14:28:15 PDT
Comment on attachment 381604 [details] V1: Subroles for emphasis and strong Clearing review flag. I just saw James Craig's response to proper exposure for strong and emphasis, and it's not the guess I made in this patch. New patch coming up. :)
Joanmarie Diggs
Comment 4 2019-10-22 14:54:18 PDT
Created attachment 381613 [details] V2: NO Subroles for emphasis and strong This is an alternative version of the previous patch for discussion and consideration.
Joanmarie Diggs
Comment 5 2019-10-22 14:56:12 PDT
Hey James and Chris. I'm not 100% sure I understand James' feedback (on a Core-AAM issue). So I have provided two versions of the same patch. The difference is that in V1, I created new subroles for your platform; in V2 I didn't. Please tell me which is the right version. :)
chris fleizach
Comment 6 2019-10-22 15:47:32 PDT
(In reply to Joanmarie Diggs (irc: joanie) from comment #5) > Hey James and Chris. > > I'm not 100% sure I understand James' feedback (on a Core-AAM issue). So I > have provided two versions of the same patch. The difference is that in V1, > I created new subroles for your platform; in V2 I didn't. > > Please tell me which is the right version. :) Thanks! we'll go with the role version
Joanmarie Diggs
Comment 7 2019-10-22 15:53:21 PDT
(In reply to chris fleizach from comment #6) > (In reply to Joanmarie Diggs (irc: joanie) from comment #5) > > Hey James and Chris. > > > > I'm not 100% sure I understand James' feedback (on a Core-AAM issue). So I > > have provided two versions of the same patch. The difference is that in V1, > > I created new subroles for your platform; in V2 I didn't. > > > > Please tell me which is the right version. :) > > Thanks! we'll go with the role version Forgive my sleep deprivation. V1 or V2?
Joanmarie Diggs
Comment 8 2019-10-22 15:53:54 PDT
Oops just saw the r+. Never mind. See sleep deprivation comment. Thanks very much!!
WebKit Commit Bot
Comment 9 2019-10-22 19:34:08 PDT
Comment on attachment 381604 [details] V1: Subroles for emphasis and strong Clearing flags on attachment: 381604 Committed r251469: <https://trac.webkit.org/changeset/251469>
WebKit Commit Bot
Comment 10 2019-10-22 19:34:10 PDT
All reviewed patches have been landed. Closing bug.
Jonathan Bedard
Comment 11 2019-10-23 08:32:08 PDT
Pretty sure this caused our Windows queues to start crashing: <https://results.webkit.org/suites?platform=win> Could be <https://trac.webkit.org/changeset/251471>, but since we have a bunch of failing accessibility tests, I'm more inclined to blame this change.
Joanmarie Diggs
Comment 12 2019-10-23 08:40:07 PDT
I'm afraid I only have mac and linux systems. So would it be possible for you to determine for certain if this change is the real source of failure? The reason I ask is that the nature of this change is quite similar to all the other new-ARIA-role changes I've made. And none of those have crashed Windows if I'm not mistaken.
Joanmarie Diggs
Comment 13 2019-10-23 09:31:18 PDT
BTW, looking at https://results.webkit.org/suites?platform=win, at the present time it doesn't seem like the Debug bot is doing it's thing. (Last result right now is 251463.) I assume if that bot resumes running, I can look at a stacktrace for the new crashes.
Russell Epstein
Comment 14 2019-10-23 17:07:29 PDT
r251469 appears to have broken the following test (rebaseline?): accessibility/ios-simulator/link-with-images-text.html Test History: https://results.webkit.org/?suite=layout-tests&test=accessibility%2Fios-simulator%2Flink-with-images-text.html Diff: --- /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/accessibility/ios-simulator/link-with-images-text-expected.txt +++ /Volumes/Data/slave/ios-simulator-13-release-tests-wk2/build/layout-test-results/accessibility/ios-simulator/link-with-images-text-actual.txt @@ -4,7 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE". -PASS obj.isIgnored is false +FAIL obj.isIgnored should be false. Was true. PASS obj.description is 'AXLabel: iPhone From $99' PASS successfullyParsed is true
Russell Epstein
Comment 15 2019-10-23 17:10:27 PDT
Rolled out in r251515
Joanmarie Diggs
Comment 16 2019-10-23 18:52:52 PDT
(In reply to Russell Epstein from comment #14) > r251469 appears to have broken the following test (rebaseline?): > > accessibility/ios-simulator/link-with-images-text.html Nah. Looks like a genuine regression. Sorry about that! Will take a look tomorrow.
Jonathan Bedard
Comment 17 2019-10-24 07:39:20 PDT
(In reply to Jonathan Bedard from comment #11) > Pretty sure this caused our Windows queues to start crashing: > <https://results.webkit.org/suites?platform=win> > > Could be <https://trac.webkit.org/changeset/251471>, but since we have a > bunch of failing accessibility tests, I'm more inclined to blame this change. Actually, looks like a JSC change was responsible for this breakage, https://trac.webkit.org/changeset/251468/webkit broke it, then https://trac.webkit.org/changeset/251534/webkit fixed it.
Joanmarie Diggs
Comment 18 2019-10-24 15:50:24 PDT
Chris: I could use your input on how to fix this for iOS. The test has a link with a child 'strong' element. My change causes the 'strong' element to be an accessible object. As a result the link now has an accessibilityElementCount. determineIsAccessibilityElement returns true for most interactive elements, but not for links. It has this conditional logic: // Links can sometimes be elements (when they only contain static text or don't contain anything). // They should not be elements when containing text and other types. case AccessibilityRole::WebCoreLink: case AccessibilityRole::Link: if ([self containsUnnaturallySegmentedChildren] || ![self accessibilityElementCount]) return true; return false; There's nothing else in the test which would cause the link to be included in the accessibility tree, so it gets ignored as a result of my change. Do the accessible/unignored descendants of links in iOS inherit the link nature of the parent? If so, then maybe the test just needs rebasing (and a new test written with, say, the 'b' element). That way we'll have both variants with test coverage. Please let me know what you think. Thanks!
chris fleizach
Comment 19 2019-10-24 16:33:35 PDT
(In reply to Joanmarie Diggs (irc: joanie) from comment #18) > Chris: I could use your input on how to fix this for iOS. > > The test has a link with a child 'strong' element. My change causes the > 'strong' element to be an accessible object. As a result the link now has an > accessibilityElementCount. > > determineIsAccessibilityElement returns true for most interactive elements, > but not for links. It has this conditional logic: > > // Links can sometimes be elements (when they only contain static text > or don't contain anything). > // They should not be elements when containing text and other types. > case AccessibilityRole::WebCoreLink: > case AccessibilityRole::Link: > if ([self containsUnnaturallySegmentedChildren] || ![self > accessibilityElementCount]) > return true; > return false; > > There's nothing else in the test which would cause the link to be included > in the accessibility tree, so it gets ignored as a result of my change. > > Do the accessible/unignored descendants of links in iOS inherit the link > nature of the parent? If so, then maybe the test just needs rebasing (and a > new test written with, say, the 'b' element). That way we'll have both > variants with test coverage. > > Please let me know what you think. Thanks! Hmm, given what you've added I think we need to know be smarter than just ![self accessibilityElementCount] I think we need to do if ([self containsUnnaturallySegmentedChildren] || ![self _accessibilityHasMoreThanOneChild]) return true; - (BOOL)_accessibilityHasMoreThanOneChild { int childrenCount = 0; NSMutableArray *elementStack = [[NSMutableArray alloc] init]; [elementStack addObject:self]; while ([elementStack count]) { id obj = [elementStack removeFirstObject]; if (obj != self) { if ([obj isAccessibilityElement]) childrenCount++; } if (childrenCount > 1) breaks; for (NSInteger k = 0; k < [obj accessibilityElementCount]; k++) { [elementStack addObjet:[obj accessibilityElementAtIndex:k]; } } return childrenCount > 1; }
James Craig
Comment 20 2023-03-20 15:56:42 PDT
See also #253751
James Craig
Comment 21 2023-03-20 15:57:01 PDT
See also bug 253751
James Craig
Comment 22 2023-03-21 00:30:36 PDT
Tyler, I think you are tracking all this work elsewhere, so should this one be closed?
James Craig
Comment 23 2023-04-26 23:49:49 PDT
Looks like code and generic were resolved with bug 253751, but not strong and emphasis. https://wpt.fyi/results/wai-aria/role/roles.html?label=master&label=experimental&aligned Leaving this one open.
James Craig
Comment 24 2023-04-26 23:55:45 PDT
James Craig
Comment 25 2023-04-26 23:58:17 PDT
Note You need to log in before you can comment on or make changes to this bug.