Bug 95286 - buildAccessibilityTree() needs to be removed from individual layout tests
Summary: buildAccessibilityTree() needs to be removed from individual layout tests
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joanmarie Diggs (irc: joanie)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2012-08-28 21:56 PDT by Joanmarie Diggs (irc: joanie)
Modified: 2014-07-04 08:32 PDT (History)
19 users (show)

See Also:


Attachments
Patch proposal (24.00 KB, patch)
2014-07-03 09:39 PDT, Mario Sanchez Prada
cfleizach: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-12 for mac-mountainlion-wk2 (533.19 KB, application/zip)
2014-07-03 12:45 PDT, Build Bot
no flags Details
Patch proposal (26.68 KB, patch)
2014-07-04 04:24 PDT, Mario Sanchez Prada
buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from webkit-ews-07 for mac-mountainlion (519.61 KB, application/zip)
2014-07-04 06:56 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2 (546.96 KB, application/zip)
2014-07-04 07:35 PDT, Build Bot
no flags Details
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion (551.64 KB, application/zip)
2014-07-04 07:47 PDT, Build Bot
no flags Details
Patch proposal (31.22 KB, patch)
2014-07-04 07:54 PDT, 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 Joanmarie Diggs (irc: joanie) 2012-08-28 21:56:29 PDT
I stumbled across a nifty function that seems like it might be an excellent way to have more WebCore-wide a11y Layout tests and fewer mysterious failures and ignores. It's the buildAccessibilityTree() function found here:

  http://trac.webkit.org/browser/trunk/LayoutTests/accessibility/adjacent-continuations-cause-assertion-failure.html

and here:

  http://trac.webkit.org/browser/trunk/LayoutTests/accessibility/div-within-anchors-causes-crash.html

and here:

  http://trac.webkit.org/browser/trunk/LayoutTests/accessibility/deleting-iframe-destroys-axcache.html

It would also solve the problem found here:

  http://trac.webkit.org/browser/trunk/LayoutTests/accessibility/legend.html

Because at least two platforms (Mac and Gtk) have very different a11y hierarchies (not just roles, but objects included and numbers of children), the expected results will often be quite different. But the fundamental things being tested often are not.

So.... Why is this method not <somewhere else> where we can stop duplicating it and start using for all our tests? Assuming the answer is "We just haven't gotten to it yet", where should it go?
Comment 1 chris fleizach 2012-08-28 22:27:32 PDT
We could make an accessibility-utlity.js file that we can import in our layout tests that has this method and perhaps others
Comment 2 Joanmarie Diggs (irc: joanie) 2012-08-29 00:14:43 PDT
Thanks!! I'll give that a try soon.

(In the meantime, I am hoping you won't object to my copying that method into legend.html. You'll see a patch after I run the proposed change on my Mac Mini.)
Comment 3 Radar WebKit Bug Importer 2013-12-20 12:04:47 PST
<rdar://problem/15710425>
Comment 4 Mario Sanchez Prada 2014-07-03 09:39:21 PDT
Created attachment 234349 [details]
Patch proposal

Let's get rid of this nasty duplication once and for all...
Comment 5 chris fleizach 2014-07-03 09:51:07 PDT
Comment on attachment 234349 [details]
Patch proposal

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

> LayoutTests/resources/accessibility-helper.js:14
> +    if (accessibilityObject.stringValue.indexOf('End of test') >= 0)

maybe this should be a "stopElement" that you pass into the method. this way is a bit opaque as to when it would end

> LayoutTests/resources/accessibility-helper.js:27
> +    if (accessibilityObject.stringValue.indexOf('End of test') >= 0)

ditto
Comment 6 Build Bot 2014-07-03 12:45:36 PDT
Comment on attachment 234349 [details]
Patch proposal

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

New failing tests:
media/W3C/video/networkState/networkState_during_loadstart.html
Comment 7 Build Bot 2014-07-03 12:45:41 PDT
Created attachment 234365 [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 8 Mario Sanchez Prada 2014-07-04 04:24:49 PDT
Created attachment 234400 [details]
Patch proposal

(In reply to comment #5)
> (From update of attachment 234349 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234349&action=review
> 
> > LayoutTests/resources/accessibility-helper.js:14
> > +    if (accessibilityObject.stringValue.indexOf('End of test') >= 0)
> 
> maybe this should be a "stopElement" that you pass into the method. this way is a bit opaque as to when it would end
> 
> > LayoutTests/resources/accessibility-helper.js:27
> > +    if (accessibilityObject.stringValue.indexOf('End of test') >= 0)
> 
> ditto

Thanks for the suggestions, I've incorporated it in a new patch that I'm attaching here before landing, just to check whether it's fine for the EWS
Comment 9 Build Bot 2014-07-04 06:56:37 PDT
Comment on attachment 234400 [details]
Patch proposal

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

New failing tests:
accessibility/div-within-anchors-causes-crash.html
accessibility/deleting-iframe-destroys-axcache.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 10 Build Bot 2014-07-04 06:56:42 PDT
Created attachment 234409 [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 11 Build Bot 2014-07-04 07:35:29 PDT
Comment on attachment 234400 [details]
Patch proposal

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

New failing tests:
accessibility/div-within-anchors-causes-crash.html
media/W3C/video/src/src_reflects_attribute_not_source_elements.html
accessibility/deleting-iframe-destroys-axcache.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 12 Build Bot 2014-07-04 07:35:35 PDT
Created attachment 234411 [details]
Archive of layout-test-results from webkit-ews-10 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: webkit-ews-10  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 13 Build Bot 2014-07-04 07:47:21 PDT
Comment on attachment 234400 [details]
Patch proposal

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

New failing tests:
accessibility/div-within-anchors-causes-crash.html
accessibility/deleting-iframe-destroys-axcache.html
accessibility/adjacent-continuations-cause-assertion-failure.html
Comment 14 Build Bot 2014-07-04 07:47:26 PDT
Created attachment 234412 [details]
Archive of layout-test-results from webkit-ews-06 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: webkit-ews-06  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 15 Mario Sanchez Prada 2014-07-04 07:54:26 PDT
Created attachment 234413 [details]
Patch proposal

Judging for the output in the Mac bots: 

--- /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/accessibility/adjacent-continuations-cause-assertion-failure-expected.txt
+++ /Volumes/Data/EWS/WebKit/WebKitBuild/Release/layout-test-results/accessibility/adjacent-continuations-cause-assertion-failure-actual.txt
@@ -13,7 +13,6 @@
         AXRole: AXStaticText AXValue: y
     AXRole: AXStaticText AXValue: z
     AXRole: AXGroup AXValue: 
-        AXRole: AXStaticText AXValue: End of test
 PASS successfullyParsed is true


It looks to me that just removing that line from the expectations is fine, as we are now bailing out from dumpAccessibilityTree() one step earlier (as we pass the "stop element" as a parameter), meaning that such a line won't ever be printed.

The newly attached patch is to (try to) fix that. Let's see what the EWS think about it...
Comment 16 Mario Sanchez Prada 2014-07-04 08:28:58 PDT
(In reply to comment #15)
> [...]
> The newly attached patch is to (try to) fix that. Let's see what the EWS
> think about it...

The new patch seems to be fine in the Mac. I'll be landing it soon...
Comment 17 Mario Sanchez Prada 2014-07-04 08:32:28 PDT
Committed r170806: <http://trac.webkit.org/changeset/170806>