RESOLVED FIXED 118966
[ATK] Implement allAttributes() for AccessibilityUIElement 
https://bugs.webkit.org/show_bug.cgi?id=118966
Summary [ATK] Implement allAttributes() for AccessibilityUIElement
Mario Sanchez Prada
Reported 2013-07-22 06:21:44 PDT
Implementing this function would help to run properly the following tests, now in TestExpectations: accessibility/table-detection.html accessibility/table-one-cell.html accessibility/table-with-rules.html accessibility/th-as-title-ui.html accessibility/transformed-element.html Additionally, it should help to to provide more accurate (and useful!) results for the following tests: deleting-iframe-destroys-axcache.html image-link-inline-cont.html image-link.html non-data-table-cell-title-ui-element.html table-cell-spans.html table-cells.html
Attachments
Patch proposal (48.24 KB, patch)
2013-07-26 04:03 PDT, Mario Sanchez Prada 		gustavo: review+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Mario Sanchez Prada
Comment 1 2013-07-26 04:03:10 PDT
Created attachment 207521 [details] Patch proposal This patch implements the missing functionality in DRT & WKTR, removes four tests from TestExpectations and updates expectations for 3 more tests. Please review :)
Mario Sanchez Prada
Comment 2 2013-07-26 04:04:06 PDT
Adding to CC people that might be interested.
Mario Sanchez Prada
Comment 3 2013-07-26 05:33:49 PDT
*** Bug 98381 has been marked as a duplicate of this bug. ***
Gustavo Noronha (kov)
Comment 4 2013-07-29 07:23:49 PDT
Comment on attachment 207521 [details] Patch proposal View in context: https://bugs.webkit.org/attachment.cgi?id=207521&action=review > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:76 > + GString* str = g_string_new(0); Might be worth it taking advantage of the change to use StringBuilder and String here =) > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:241 > + if (parentName && g_utf8_strlen(parentName, -1)) { > + attributeLine.set(g_strdup_printf(": %s", parentName)); > + > + } No need for braces, empty line > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:257 > + attributeLine.set(g_strdup_printf("%s\n", title.utf8().data())); How about using String::format("%s\n", title.utf8().data())? Then you can have attributeLine live on the stack and avoid dealing with char pointers. > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:303 > + // We append the ATK specific attributes as a single line at the end. > + GOwnPtr<char> attributeData(getAtkAttributeSetAsString(element->platformUIElement())); > + attributeLine.set(g_strdup_printf("AXPlatformAttributes: %s", attributeData.get())); > + builder.append(attributeLine.get()); Ditto. > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:493 > - String roleString = roleToString(role); > - return JSStringCreateWithUTF8CString(roleString.utf8().data()); > + GOwnPtr<char> roleStringWithPrefix(g_strdup_printf("AXRole: %s", roleToString(role))); > + return JSStringCreateWithUTF8CString(roleStringWithPrefix.get()); Ditto. > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:292 > + attributeLine.set(g_strdup_printf("%s\n", element->role()->string().utf8().data())); > + builder.append(attributeLine.get()); This file also has a number of potential changes from char* to String.
Mario Sanchez Prada
Comment 5 2013-07-29 10:13:25 PDT
Thanks for the review, Gustavo. See some comments below... (In reply to comment #4) > (From update of attachment 207521 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=207521&action=review > > > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:76 > > + GString* str = g_string_new(0); > > Might be worth it taking advantage of the change to use StringBuilder and String here =) Sure. Changed. > > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:241 > > + if (parentName && g_utf8_strlen(parentName, -1)) { > > + attributeLine.set(g_strdup_printf(": %s", parentName)); > > + > > + } > > No need for braces, empty line Ok. > > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:257 > > + attributeLine.set(g_strdup_printf("%s\n", title.utf8().data())); > > How about using String::format("%s\n", title.utf8().data())? Then you can have attributeLine live on the stack and avoid dealing with char pointers. Ok. Changed the patch to do that. > > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:303 > > + // We append the ATK specific attributes as a single line at the end. > > + GOwnPtr<char> attributeData(getAtkAttributeSetAsString(element->platformUIElement())); > > + attributeLine.set(g_strdup_printf("AXPlatformAttributes: %s", attributeData.get())); > > + builder.append(attributeLine.get()); > > Ditto. Done. > > Tools/DumpRenderTree/atk/AccessibilityUIElementAtk.cpp:493 > > - String roleString = roleToString(role); > > - return JSStringCreateWithUTF8CString(roleString.utf8().data()); > > + GOwnPtr<char> roleStringWithPrefix(g_strdup_printf("AXRole: %s", roleToString(role))); > > + return JSStringCreateWithUTF8CString(roleStringWithPrefix.get()); > > Ditto. Done. > > Tools/WebKitTestRunner/InjectedBundle/atk/AccessibilityUIElementAtk.cpp:292 > > + attributeLine.set(g_strdup_printf("%s\n", element->role()->string().utf8().data())); > > + builder.append(attributeLine.get()); > > This file also has a number of potential changes from char* to String. I agree with that, but that's probably something better to tackle in different patches.
Mario Sanchez Prada
Comment 6 2013-07-29 10:15:19 PDT
Committed r153432: <http://trac.webkit.org/changeset/153432>
Mario Sanchez Prada
Comment 7 2013-07-29 10:55:15 PDT
EFL guys might be interested in rebaselining their tests (and maybe unskipping some too), now we are dumping some extra information
Krzysztof Czech
Comment 8 2013-07-30 00:52:36 PDT
> EFL guys might be interested in rebaselining their tests (and maybe unskipping some too), now we are dumping some extra information Thanks Mario for information. I will take a look at it.
Note You need to log in before you can comment on or make changes to this bug.