Bug 157822

Created attachment 279191 [details] Patch

<rdar://problem/26337017>

<rdar://problem/26337023>

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.) :)

<rdar://problem/26337077>

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

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

(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

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

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

(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

(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).

Comment on attachment 279191 [details] Patch Clearing flags on attachment: 279191 Committed r201072: <http://trac.webkit.org/changeset/201072>

All reviewed patches have been landed. Closing bug.