Bug 157187 - AX: Layout tests related to text alternative computation need to be done differently
Summary: AX: Layout tests related to text alternative computation need to be done diff...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All Linux
: P2 Normal
Assignee: Joanmarie Diggs
URL:
Keywords: InRadar
: 106340 131496 (view as bug list)
Depends on: 157822
Blocks:
  Show dependency treegraph
 
Reported: 2016-04-29 10:21 PDT by Joanmarie Diggs
Modified: 2016-05-20 11:33 PDT (History)
12 users (show)

See Also:


Attachments
Proposal 1, Take 1 (100.15 KB, patch)
2016-05-18 23:57 PDT, Joanmarie Diggs
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews112 for mac-yosemite (1.48 MB, application/zip)
2016-05-19 00:39 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joanmarie Diggs 2016-04-29 10:21:58 PDT
The "Accessible Name and Description: Computation and API Mappings 1.1" spec [1] describes how the "name" and "description" are mapped on the various platforms we support [2]. The simplified version of the most-typical [3] cases are:

       Spec            Platform        Layout Test
       ----         --------------    -------------
AXAPI  name         AXDescription     AXDescription
       description  AXHelp            AXHelp
ATK    name         name              AXTitle
       description  description       AXDescription

If that were not confusing enough, even though ATK has no such property or concept of "help text," WebKitTestRunner exposes descriptions which originate from ATK_RELATION_DESCRIBED_BY though AXHelp as a string (and thus duplicates what is in AXDescription) rather than a list of pointers to elements (which is what the AtkRelation provides).

We have a number of failing tests -- and a number of ATK implementation bugs -- related to this confusion. In the former case, gardeners have filed one bug per failure. We also have at least a few cases where what a cross-platform test should be testing is not actually what it's testing (e.g. testing the spec "name" via "AXDescription").

The purpose of this bug here is to gather all the failures in a single place, fix the alternative text calculation bugs in ATK, and when appropriate tweak the cross-platform tests to be more cross-platform friendly (e.g. just always dump AXTitle, AXDescription, and AXHelp and have platform-specific expectations).

[1] https://w3c.github.io/aria/accname-aam/accname-aam.html
[2] https://w3c.github.io/aria/accname-aam/accname-aam.html#accessible-name-and-description-mapping
[3] In some cases in AXAPI, e.g. certain widgets, spec "name" maps to AXTitle
Comment 1 Radar WebKit Bug Importer 2016-04-29 10:22:44 PDT
<rdar://problem/26006749>
Comment 2 Joanmarie Diggs 2016-04-29 10:24:54 PDT
*** Bug 106340 has been marked as a duplicate of this bug. ***
Comment 3 Michael Catanzaro 2016-05-15 17:08:36 PDT
I see only one failing test marked against this bug: accessibility/gtk/title-and-alt.html. This test was fixed by r200261, a commit clearly unrelated to accessibility code.
Comment 4 Michael Catanzaro 2016-05-15 17:10:13 PDT
Updated expectations in r200935.
Comment 5 Joanmarie Diggs 2016-05-16 05:44:47 PDT
Michael: This bug is not about a specific failure; it's about the need to write the tests in such a way as to not rely upon a specific property (e.g. description) having a specific value.

FWIW, there are at least a few tests which currently pass even though they are a functionally failing because in ATK the property is being exposed on the same property (e.g. description) as the Mac even though description on the Mac is not the same as description in ATK.
Comment 6 Joanmarie Diggs 2016-05-16 06:02:15 PDT
(In reply to comment #3)
> I see only one failing test marked against this bug:
> accessibility/gtk/title-and-alt.html. This test was fixed by r200261, a
> commit clearly unrelated to accessibility code.

BTW: 200261 did not fix it. See https://trac.webkit.org/changeset/200260.

Maybe the "FAIL" expectations weren't the right approach. But the test is still failing and there is a bug which still need to be fixed. But because they are not in the TestExpectations, one now needs to grep for "FAIL" rather than look at TestExpectations.
Comment 7 Joanmarie Diggs 2016-05-17 15:49:57 PDT
*** Bug 131496 has been marked as a duplicate of this bug. ***
Comment 8 Joanmarie Diggs 2016-05-18 23:57:29 PDT
Created attachment 279365 [details]
Proposal 1, Take 1
Comment 9 Build Bot 2016-05-19 00:38:59 PDT
Comment on attachment 279365 [details]
Proposal 1, Take 1

Attachment 279365 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/1346738

New failing tests:
storage/websql/change-version.html
Comment 10 Build Bot 2016-05-19 00:39:04 PDT
Created attachment 279368 [details]
Archive of layout-test-results from ews112 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5
Comment 11 Joanmarie Diggs 2016-05-19 00:54:14 PDT
(In reply to comment #10)
> Created attachment 279368 [details]
> Archive of layout-test-results from ews112 for mac-yosemite
> 
> The attached test failures were seen while running run-webkit-tests on the
> mac-debug-ews.
> Bot: ews112  Port: mac-yosemite  Platform: Mac OS X 10.10.5

I think this is a false positive because the only thing I've touched in my patch is accessibility layout tests, along with adding some utility methods that only the accessibility layout tests call. How those changes could break storage/websql/change-version.html is beyond me.
Comment 12 Joanmarie Diggs 2016-05-19 00:56:13 PDT
Comment on attachment 279365 [details]
Proposal 1, Take 1

Chris, here's my initial proposal. Please let me know what you think. Thanks!
Comment 13 chris fleizach 2016-05-20 09:32:25 PDT
Comment on attachment 279365 [details]
Proposal 1, Take 1

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

> LayoutTests/resources/accessibility-helper.js:36
> +function platformValueForW3CName(accessibilityObject, includeSource=false) {

Looks great.

Why did you go with "W3CName"? Do you think that suffix is necessary, or would platformValueForAccessibleName be valid?

Thanks
Comment 14 Joanmarie Diggs 2016-05-20 10:24:42 PDT
(In reply to comment #13)

> Why did you go with "W3CName"? Do you think that suffix is necessary, or
> would platformValueForAccessibleName be valid?

Brutal explicitness. :) Description is worse, so to use that:

Description could hypothetically be:
1. Some generic term (like properties)
2. The WKTR's AccessibilityUIElement::description()
3. Your platform's AXDescription
4. My platform's AtkObject description
5. The W3C's accessible description

I think it's worth three chars to eliminate the possibility of confusion.
Comment 15 Joanmarie Diggs 2016-05-20 11:10:59 PDT
Comment on attachment 279365 [details]
Proposal 1, Take 1

cq+ing this patch because as previously indicated, I believe the identified failure was/is a false positive. This patch consists entirely of Accessibility layout test changes.
Comment 16 WebKit Commit Bot 2016-05-20 11:33:13 PDT
Comment on attachment 279365 [details]
Proposal 1, Take 1

Clearing flags on attachment: 279365

Committed r201216: <http://trac.webkit.org/changeset/201216>
Comment 17 WebKit Commit Bot 2016-05-20 11:33:18 PDT
All reviewed patches have been landed.  Closing bug.