Summary: | [AX][GTK] No new lines in some AX tests output | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sergio Villar Senin <svillar> | ||||||
Component: | Accessibility | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, cfleizach, clopez, commit-queue, dmazzoni, jcraig, jdiggs, mario, mrobinson, samuel_white, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Linux | ||||||||
Attachments: |
|
Description
Sergio Villar Senin
2014-04-07 00:49:05 PDT
Actually the failures is not only about the "</n>" thing, but it seems that some roles for children are not printed. 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 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 (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. (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 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 (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. (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 (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 (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 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 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.
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)
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 (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 Committed r167011: <http://trac.webkit.org/changeset/167011> |