Bug 126685 - AX: Merge layout test from Mac and GTK checking accessibility roles
Summary: AX: Merge layout test from Mac and GTK checking accessibility roles
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
: 125585 (view as bug list)
Depends on: 126689
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-09 02:21 PST by Mario Sanchez Prada
Modified: 2014-03-24 03:29 PDT (History)
12 users (show)

See Also:


Attachments
Patch proposal (90.16 KB, patch)
2014-01-09 03:53 PST, Mario Sanchez Prada
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (456.48 KB, application/zip)
2014-01-13 04:05 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion (493.02 KB, application/zip)
2014-01-13 04:25 PST, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (548.09 KB, application/zip)
2014-01-13 04:56 PST, Build Bot
no flags Details
Patch proposal (91.32 KB, patch)
2014-01-13 05:25 PST, Mario Sanchez Prada
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2014-01-09 02:21:31 PST
As it was discussed in bug 125525 (see https://bugs.webkit.org/show_bug.cgi?id=125525#c5), it looks like a good idea to merge platform/mac/accessibility/role-subrole-roledescription.html and platform/gtk/accessibility/roles-exposed.html into a single test that handles the platform-specific differences gracefully, instead of having two -almost- duplicated HTML files for the same purpose.
Comment 1 Radar WebKit Bug Importer 2014-01-09 02:21:43 PST
<rdar://problem/15780752>
Comment 2 Mario Sanchez Prada 2014-01-09 03:53:05 PST
Created attachment 220711 [details]
Patch proposal

This patch merges the two tests in one, following the idea (or my interpretation of it) suggested by Chris in bug 125525 (see https://bugs.webkit.org/show_bug.cgi?id=125525#c4).

Compared to the original test from the Mac, the differences now are:

* A new data attribute is added to specify which platforms should care about a given element, or ignore it ('data-platform'), specifying a comma-separated list of platforms as returned by accessibilityController.platformName ("atk", "mac" or "win". See bug 126689). If the current "accessibility platform" is not listed for an element, it will be ignored and nothing will be checked about it. See some examples:

<a data-platform="atk,mac" data-role="AXLink" data-subrole="" data-roledescription="link" href="#" data-note="[href]" class="ex">X</a>
<input data-platform="mac" type="color" value="X" data-role="AXColorWell" data-subrole="" data-roledescription="color well" class="ex" data-note="[type='color']">


* To handle the case where the expected role would NOT be the same for all the platforms, a new 'data-role-<platform>' attribute can be added to an element. See some examples:

<dl data-platform="atk,mac" data-role-atk="AXDescriptionList" data-role-mac="AXList" data-subrole="AXDescriptionList" data-alternatesubrole="AXDefinitionList" data-roledescription="description list" class="ex">
    <dt data-platform="atk,mac" data-role-atk="AXDescriptionTerm" data-role-mac="AXGroup" data-subrole="AXTerm" data-roledescription="term" class="ex">X</dt>
    <dd data-platform="atk,mac" data-role-atk="AXDescriptionValue" data-role-mac="AXGroup" data-subrole="AXDescription" data-roledescription="description" class="ex">X</dd>
</dl>


* To handle the case where the expected role would be the same for more than one platform, the 'data-role' attribute can still be used (see the first examples above), as the role checking code will fallback to it in case it could not find a 'data-role-<platform>'one:

        expectedRole = "";
        if (el.hasAttribute('data-role-' + currentPlatform)) {
            expectedRole = el.getAttribute('data-role-' + currentPlatform);
        } else if (el.hasAttribute('data-role')) {
            expectedRole = el.getAttribute('data-role');
        }


* This test depends on bug 126689 because it expects accessibilityController.platformName instead of testRunner.platformName, and also because it expects "atk" for both the GTK and EFL ports, so it can compare it against the values included in the new 'data-platform' attribute:

    var currentPlatform = accessibilityController.platformName;
    [...]
    for (var i = 0, c = examples.length; i < c; i++) {
        el = examples[i];

        supportedPlatforms = el.getAttribute('data-platform');
        if (!supportedPlatforms || supportedPlatforms.indexOf(currentPlatform) == -1)
            continue;

        [...]
    }


I have carefully changed the code using the Mac test as a base and only changing bits that should not have altered the logic (and the output) in the mac port, but I can't be 100% sure that I did not break anything there, so I'd rather set the r? flag once bug 126689 has been fixed and the EWS for this test have been run. It "should work fine", though :)

In the meanwhile, any feedback is welcome of course!
Comment 3 chris fleizach 2014-01-09 08:58:30 PST
Comment on attachment 220711 [details]
Patch proposal

i think this plan looks good
Comment 4 Build Bot 2014-01-13 04:05:27 PST
Comment on attachment 220711 [details]
Patch proposal

Attachment 220711 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5125621755674624

New failing tests:
accessibility/roles-exposed.html
Comment 5 Build Bot 2014-01-13 04:05:31 PST
Created attachment 221025 [details]
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-12  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Build Bot 2014-01-13 04:25:22 PST
Comment on attachment 220711 [details]
Patch proposal

Attachment 220711 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5069476198350848

New failing tests:
accessibility/roles-exposed.html
Comment 7 Build Bot 2014-01-13 04:25:25 PST
Created attachment 221027 [details]
Archive of layout-test-results from webkit-ews-05 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-05  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 8 Build Bot 2014-01-13 04:56:17 PST
Comment on attachment 220711 [details]
Patch proposal

Attachment 220711 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6013784887918592

New failing tests:
accessibility/roles-exposed.html
Comment 9 Build Bot 2014-01-13 04:56:20 PST
Created attachment 221029 [details]
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-07  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 10 Mario Sanchez Prada 2014-01-13 05:23:17 PST
So glad to see that the only text diff in the mac seems to be this one:

-This tests that native elements and ARIA overrides result in the expected role, subrole, and role description.
+This tests that native elements and ARIA overrides result in the expected role, subrole and role description.

I'll update the mac specific expectations fixing that typo now and upload a new patch
Comment 11 Mario Sanchez Prada 2014-01-13 05:25:52 PST
Created attachment 221030 [details]
Patch proposal
Comment 12 WebKit Commit Bot 2014-01-15 05:11:39 PST
Comment on attachment 221030 [details]
Patch proposal

Clearing flags on attachment: 221030

Committed r162067: <http://trac.webkit.org/changeset/162067>
Comment 13 WebKit Commit Bot 2014-01-15 05:11:42 PST
All reviewed patches have been landed.  Closing bug.
Comment 14 Alexey Proskuryakov 2014-01-15 10:19:05 PST
This patch broke accessibility/roles-exposed.html on Mac Mavericks:

http://webkit-test-results.appspot.com/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=accessibility%2Froles-exposed.html
Comment 15 Alexey Proskuryakov 2014-01-15 10:56:58 PST
Corrected Mac test results in <http://trac.webkit.org/r162082>.
Comment 16 Mario Sanchez Prada 2014-03-24 03:29:05 PDT
*** Bug 125585 has been marked as a duplicate of this bug. ***