Bug 131294 – [AX][GTK] No new lines in some AX tests output

• Status: RESOLVED
• Resolution: FIXED
• Component: Accessibility

[Sergio Villar Senin]

Several tests fail:
accessibility/adjacent-continuations-cause-assertion-failure.html
accessibility/deleting-iframe-destroys-axcache.html
accessibility/div-within-anchors-causes-crash.html
accessibility/lists.html

This is the kind of errors bots are observing:
-AXRole: AXWebArea
- AXRole: AXSection AXValue: y
- AXRole: AXSection AXValue: End of test
+AXRole: AXWebArea AXValue: x<\n>y<\n>z<\n>End of test<\n>Make sure that a debug assert is not triggered when constructing the accessibility tree for this page.<\n><\n>On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".<\n><\n><\n>

[Radar WebKit Bug Importer]

<rdar://problem/16537388>

[Sergio Villar Senin]

Actually the failures is not only about the "</n>" thing, but it seems that some roles for children are not printed.

[Sergio Villar Senin]

Another 3 more tests

platform/gtk/accessibility/entry-and-password.html
platform/gtk/accessibility/media-emits-object-replacement.html
platform/gtk/accessibility/replaced-objects-in-anonymous-blocks.html

[Mario Sanchez Prada]

Curiously enough, I can only reproduce this with WebKit2 and WKTR. With WebKit1 and DRT everything is fine.

I'm taking a look to this right now

[Martin Robinson]

(In reply to comment #4)
> Curiously enough, I can only reproduce this with WebKit2 and WKTR. With WebKit1 and DRT everything is fine.
> 
> I'm taking a look to this right now

Great. And FWIW, WebKit1 is toast soon.

[Mario Sanchez Prada]

(In reply to comment #5)
> (In reply to comment #4)
> > Curiously enough, I can only reproduce this with WebKit2 and WKTR. With WebKit1 and DRT everything is fine.
> > 
> > I'm taking a look to this right now
> 
> Great. And FWIW, WebKit1 is toast soon.

I know. Just mentioned it because I found the difference in behaviour quite interesting, as it could mean the problem is not in WebKit itself, but in the testing tools

[Carlos Alberto Lopez Perez]

Two more tests are also failing:

* accessibility/children-changed-sends-notification.html
* accessibility/notification-listeners.html


The diffs are the following:

--- /stuff/webkit/webkit/layout-test-results/accessibility/children-changed-sends-notification-expected.txt
+++ /stuff/webkit/webkit/layout-test-results/accessibility/children-changed-sends-notification-actual.txt
@@ -6,12 +6,8 @@
 Plain text paragraph 
 
 End of test
-PARAGRAPH notification: AXChildrenRemoved
-GLOBAL notification: AXChildrenRemoved on element with role AXRole: AXParagraph
-PARAGRAPH notification: AXChildrenAdded
-GLOBAL notification: AXChildrenAdded on element with role AXRole: AXParagraph
 PASS paragraphNotificationCount is globalNotificationCount
-PASS globalNotificationCount is 2
+FAIL globalNotificationCount should be 2. Was 0.
 PASS successfullyParsed is true
 
 TEST COMPLETE

--- /stuff/webkit/webkit/layout-test-results/accessibility/notification-listeners-expected.txt
+++ /stuff/webkit/webkit/layout-test-results/accessibility/notification-listeners-actual.txt
@@ -7,11 +7,13 @@
 Slider
 SELECT AXInvalidStatusChanged
 GLOBAL AXInvalidStatusChanged on element with role AXRole: AXComboBox
+SLIDER AXChildrenAdded
+GLOBAL AXChildrenAdded on element with role AXRole: AXSlider
 SLIDER AXValueChanged
 GLOBAL AXValueChanged on element with role AXRole: AXSlider
 PASS selectNotificationCount is 1
-PASS sliderNotificationCount is 1
-PASS globalNotificationCount is 2
+FAIL sliderNotificationCount should be 1. Was 2.
+FAIL globalNotificationCount should be 2. Was 3.
 PASS successfullyParsed is true
 
 TEST COMPLETE

[Sergio Villar Senin]

(In reply to comment #7)
> Two more tests are also failing:
> 
> * accessibility/children-changed-sends-notification.html
> * accessibility/notification-listeners.html

I didn't include them because there seem to be different types of failures. The differences in the output with the expected results are not the same.

[Carlos Alberto Lopez Perez]

(In reply to comment #8)
> (In reply to comment #7)
> > Two more tests are also failing:
> > 
> > * accessibility/children-changed-sends-notification.html
> > * accessibility/notification-listeners.html
> 
> I didn't include them because there seem to be different types of failures. The differences in the output with the expected results are not the same.

Ok. Reported it on bug #131380

[Mario Sanchez Prada]

(In reply to comment #0)
> accessibility/lists.html

Quick thing: this test is not failing due to the same issue, but due to an <hr> being removed in http://trac.webkit.org/changeset/166175, which just needs rebaselining.

I'll land a gardening patch related to that independently

[Mario Sanchez Prada]

(In reply to comment #10)
> (In reply to comment #0)
> > accessibility/lists.html
> 
> Quick thing: this test is not failing due to the same issue, but due to an <hr> being removed in http://trac.webkit.org/changeset/166175, which just needs rebaselining.
> 
> I'll land a gardening patch related to that independently

http://trac.webkit.org/changeset/167009

[Mario Sanchez Prada]

The problem is caused by http://trac.webkit.org/changeset/166194 because, as per my own recommendation, the call to updateRoleAfterChildrenCreation() is happening now in AccessibilityObject::updateBackingStore(), which unveiled a problem in the way we determined which ATK interfaces to implement in the ATK wrapper.

Because of this issue, all these tests were printing a lot of text when called the stringValue() function of AccessibilityUIElement through WKTR, since the ATK object for the WebArea would be recognized as implementing AtkText. Therefore, calling atk_text_get_text() over it would return a textual representation of the whole web page (internally using WebCore's TextIterator), which resulted in all that text being shown in the output.

And of course, the javascript code in buildAccessibilityTree() would not keep digging into the subtree for the WebArea because, due to the WebArea already returning a lot of text through the AtkText interface, and due to that text already including the 'End of test' string (used as condition to stop digging more in the tree).

So, we need to make sure we never, ever, implement AtkText for objects with the WebArea role. Patch coming soon

[Mario Sanchez Prada]

Created attachment 228953 [details]
Patch proposal

See the patch proposal attached. Notice that this patch not only prevents the WebArea object from implementing AtkText but also AtkHypertext, which I believe is correct because that other interface will be implemented anyway by the real containers of the different blocks of content in the web page.

[Mario Sanchez Prada]

Created attachment 228956 [details]
Patch proposal

Forgot the parenthesis in the previous patch. Adding them now to make EFL happy (well, and because I think it's clearer this way too)

[Martin Robinson]

Comment on attachment 228956 [details]
Patch proposal

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

> Source/WebCore/ChangeLog:8
> +        Do not implement Hypetext or AtkText for the WebArea. Those

Small typo: Hypetext -> Hypertext

[Mario Sanchez Prada]

(In reply to comment #15)
> (From update of attachment 228956 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=228956&action=review
> 
> > Source/WebCore/ChangeLog:8
> > +        Do not implement Hypetext or AtkText for the WebArea. Those
> 
> Small typo: Hypetext -> Hypertext

Oops! Thanks

[Mario Sanchez Prada]

Committed r167011: <http://trac.webkit.org/changeset/167011>