Bug 157822 – AX: [ATK] Use WebCore Accessibility's AccessibilityText for AtkObject name and description

• Component: Accessibility
• Status: RESOLVED FIXED

[Joanmarie Diggs]

WebKitGtk never switched fully over to use AccessibilityText for AtkObject name and description. As a result, we have code we don't need and which is in some cases incorrect with respect to the HTML AAM and/or AccName spec.

In addition, there are a number of places where we're taking the AXDescription as the AtkObject description. This would seem logical on the surface, but as outlined in the OR of bug 157187, it's wrong.

I'm making this bug a blocker to bug 157187 so we can fix the exposure and update the layout tests in a way that will hopefully be clear (enough) to verify the code change is not introducing any regressions. After that, we can (I will) come up with a proposal with respect to how we do cross-platform text alternative calculation layout tests.

Patch will be forthcoming.

[Joanmarie Diggs]

Created attachment 279191 [details]
Patch

[Radar WebKit Bug Importer]

<rdar://problem/26337017>

[Radar WebKit Bug Importer]

<rdar://problem/26337023>

[Joanmarie Diggs]

Chris: This is mostly ripping out code from my port to defer to WebCore Accessibility -- and adding a bunch of if/else statements to our shared layout tests to ask for AXTitle+AXDescription on my port and AXDescription+AXHelp on yours. Can you live with the latter if I promise to clean it up in bug 157187? (Please.) :)

[Radar WebKit Bug Importer]

<rdar://problem/26337077>

[chris fleizach]

So the idea is that we'll undo all the platform if else in the other patch?

[Joanmarie Diggs]

(In reply to comment #6)
> So the idea is that we'll undo all the platform if else in the other patch?

Not "undo" in the literal sense; but the if/else stuff is ugly. Some possibilities that spring to mind:

1. Adding something(s) to WKTR like: AccessibilityUIElement::textAlternative()

2. Adding something(s) to accessibility-helper.js to get the correct information from each platform.

3. Change ATK's AccessibilityUIElement so that it converts everything to the property names WebCore Accessibility and AXAPI use.

Of the three approaches, I'm least jazzed by the third because when there is not a one-to-one correspondence between our platforms, you wind up adding hacks and workarounds to fake it. In the specific case of text alternatives, I don't think it would be too horrible:

* AccessibilityUIElement::helpText() would return the AtkObject description
* AccessibilityUIElement::description() would return the AtkObject name

But:

* AccessibilityUIElement::title() would return ???

Almost all of the time, the spec's accessible name is exposed via your AXDescription. But occasionally doing so is wrong; AXTitle should be used instead. But on my platform, that wrongness doesn't exist: The spec's accessible name is always exposed via the AtkObject name.

What I like about approaches 1 and/or 2 is that all platforms have text alternatives with associated specifications. You and I both need to expose the spec's accessible name and the spec's accessible description. What we don't have in common is the exact properties used. I think approaches 1 and/or 2 can solve that.

As an example there are the w3c-svg-*-calculation.html tests. But that was done in each test. Moving that functionality to accessibility-helper.js or AccessibilityUIElement::textAlternative() would make it possible to get all the if/else platformName junk out of all the tests.

Thoughts?

[chris fleizach]

(In reply to comment #7)
> (In reply to comment #6)
> > So the idea is that we'll undo all the platform if else in the other patch?
> 
> Not "undo" in the literal sense; but the if/else stuff is ugly. Some
> possibilities that spring to mind:
> 
> 1. Adding something(s) to WKTR like:
> AccessibilityUIElement::textAlternative()
> 
> 2. Adding something(s) to accessibility-helper.js to get the correct
> information from each platform.
> 
> 3. Change ATK's AccessibilityUIElement so that it converts everything to the
> property names WebCore Accessibility and AXAPI use.
> 
> Of the three approaches, I'm least jazzed by the third because when there is
> not a one-to-one correspondence between our platforms, you wind up adding
> hacks and workarounds to fake it. In the specific case of text alternatives,
> I don't think it would be too horrible:
> 
> * AccessibilityUIElement::helpText() would return the AtkObject description
> * AccessibilityUIElement::description() would return the AtkObject name
> 
> But:
> 
> * AccessibilityUIElement::title() would return ???
> 
> Almost all of the time, the spec's accessible name is exposed via your
> AXDescription. But occasionally doing so is wrong; AXTitle should be used
> instead. But on my platform, that wrongness doesn't exist: The spec's
> accessible name is always exposed via the AtkObject name.
> 
> What I like about approaches 1 and/or 2 is that all platforms have text
> alternatives with associated specifications. You and I both need to expose
> the spec's accessible name and the spec's accessible description. What we
> don't have in common is the exact properties used. I think approaches 1
> and/or 2 can solve that.
> 
> As an example there are the w3c-svg-*-calculation.html tests. But that was
> done in each test. Moving that functionality to accessibility-helper.js or
> AccessibilityUIElement::textAlternative() would make it possible to get all
> the if/else platformName junk out of all the tests.
> 
> Thoughts?

Probably having some helper methods in ax-utilities.js would be the best approach to writing cross platform tests

[Joanmarie Diggs]

(In reply to comment #8)
> Probably having some helper methods in ax-utilities.js would be the best
> approach to writing cross platform tests

Ok, I'll give that a shot. Thanks!

Do you want that done as part of the fix for this bug, or done in bug 157187?

[chris fleizach]

Comment on attachment 279191 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=279191&action=review

> LayoutTests/accessibility/alt-tag-on-image-with-nonimage-role.html:29
> +        if (accessibilityController.platformName == "atk") {

We should probably have some helper method that can handle these platform differences seemlessly

like

accessibleNameShouldBe(group)

But we can probably live with this for now

[chris fleizach]

(In reply to comment #9)
> (In reply to comment #8)
> > Probably having some helper methods in ax-utilities.js would be the best
> > approach to writing cross platform tests
> 
> Ok, I'll give that a shot. Thanks!
> 
> Do you want that done as part of the fix for this bug, or done in bug 157187?

Let's do that in another bug so this one gets in

[Joanmarie Diggs]

(In reply to comment #11)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Probably having some helper methods in ax-utilities.js would be the best
> > > approach to writing cross platform tests
> > 
> > Ok, I'll give that a shot. Thanks!
> > 
> > Do you want that done as part of the fix for this bug, or done in bug 157187?
> 
> Let's do that in another bug so this one gets in

I appreciate it. I hadn't realized how many mappings were incorrect. :-/

I'll get started on the clean up tomorrow (assuming you're on the left coast).

[WebKit Commit Bot]

Comment on attachment 279191 [details]
Patch

Clearing flags on attachment: 279191

Committed r201072: <http://trac.webkit.org/changeset/201072>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.