More role-parity-with-html ARIA roles: * https://w3c.github.io/aria/#code * https://w3c.github.io/aria/#emphasis * https://w3c.github.io/aria/#strong * https://w3c.github.io/aria/#generic
<rdar://problem/56506497>
Created attachment 381604 [details] V1: Subroles for emphasis and strong
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. :)
Created attachment 381613 [details] V2: NO Subroles for emphasis and strong This is an alternative version of the previous patch for discussion and consideration.
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. :)
(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
(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?
Oops just saw the r+. Never mind. See sleep deprivation comment. Thanks very much!!
Comment on attachment 381604 [details] V1: Subroles for emphasis and strong Clearing flags on attachment: 381604 Committed r251469: <https://trac.webkit.org/changeset/251469>
All reviewed patches have been landed. Closing bug.
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.
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.
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.
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
Rolled out in r251515
(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.
(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.
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!
(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; }
See also #253751
See also bug 253751
Tyler, I think you are tracking all this work elsewhere, so should this one be closed?