AX: Implement support for new ARIA roles: strong and emphasis

• Component: Accessibility
• Status: REOPENED ---

[Joanmarie Diggs]

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

[Radar WebKit Bug Importer]

<rdar://problem/56506497>

[Joanmarie Diggs]

Created attachment 381604 [details]
V1: Subroles for emphasis and strong

[Joanmarie Diggs]

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]

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]

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]

(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]

(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]

Oops just saw the r+. Never mind. See sleep deprivation comment.

Thanks very much!!

[WebKit Commit Bot]

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]

All reviewed patches have been landed.  Closing bug.

[Jonathan Bedard]

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]

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]

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]

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]

Rolled out in r251515

[Joanmarie Diggs]

(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]

(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]

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]

(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]

See also #253751

[James Craig]

See also bug 253751

[James Craig]

Tyler, I think you are tracking all this work elsewhere, so should this one be closed?

[James Craig]

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]

<rdar://problem/108593598>

[James Craig]

Also affects 3 subtests in:
https://wpt.fyi/results/html-aam/roles.html?label=master&label=experimental&aligned