Bug 95286 – buildAccessibilityTree() needs to be removed from individual layout tests

• Component: Accessibility
• Status: RESOLVED FIXED

[Joanmarie Diggs]

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?

[chris fleizach]

We could make an accessibility-utlity.js file that we can import in our layout tests that has this method and perhaps others

[Joanmarie Diggs]

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

[Radar WebKit Bug Importer]

<rdar://problem/15710425>

[Mario Sanchez Prada]

Created attachment 234349 [details]
Patch proposal

Let's get rid of this nasty duplication once and for all...

[chris fleizach]

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

[Build Bot]

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

[Build Bot]

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

[Mario Sanchez Prada]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Build Bot]

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

[Mario Sanchez Prada]

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

[Mario Sanchez Prada]

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

[Mario Sanchez Prada]

Committed r170806: <http://trac.webkit.org/changeset/170806>