The accessibility/aria-checkbox-text.html test is failing for the GTK port, both because it has no *-expected file and because the required implementation to get the title from WebKitGtk through the DRT is buggy. We'd need to fix those issues to remove it from the Skipped file then.
Created attachment 57544 [details] Proposed patch Here's a patch that would solve this issue. I've tested everything with and this patch and the only difference is that one more test gets fixed, so I'd say it's right :-) By the way, perhaps we should think about being more coherent and, from now on, to always use this kind of "AXWhatever :" prefixes in the GTK's DRT, in the same way that mac port does it. But that would be something for another bug,,,
Comment on attachment 57544 [details] Proposed patch > - return JSStringCreateWithUTF8CString(name); > + gchar* title = g_strdup_printf("AXTitle: %s", name); > + JSStringRef jsTitle = JSStringCreateWithUTF8CString(title); > + > + g_free(title); > + return jsTitle; You can use GOwnptr here for title.
Created attachment 57570 [details] Proposed patch (In reply to comment #3) > (From update of attachment 57544 [details]) > > > - return JSStringCreateWithUTF8CString(name); > > + gchar* title = g_strdup_printf("AXTitle: %s", name); > > + JSStringRef jsTitle = JSStringCreateWithUTF8CString(title); > > + > > + g_free(title); > > + return jsTitle; > > You can use GOwnptr here for title. Thanks Martin for your comments, which have been addressed in the patch proposed for bug 40009 as at the end I thought it would be better to follow a different approach with this problem and split it in two parts: 1. Make GTK's DRT more coherent with other ports (bug 40009) 2. Actually make sure the AXTitle for controls is properly calculated and returned, to unskip the proper test So here we are the patch for (2), patch for (1) already attached along with bug 40009
Comment on attachment 57570 [details] Proposed patch Looks good. It would make sense to try to just have one expected file now that both ports have identical results...
Comment on attachment 57570 [details] Proposed patch Mario thinks this won't apply anymore to trunk, so removing cq+
(In reply to comment #6) > (From update of attachment 57570 [details]) > Mario thinks this won't apply anymore to trunk, so removing cq+ Yeah, I need to recheck this, and maybe come with an up-to-date patch since it's possible the modifications in AccessibilityobjectWrapperAtk.cpp are no longer needed after fixing bug 36128. PS: Shouldn't you remove the review+`flag as well, btw?
Created attachment 60026 [details] Proposed patch (In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 57570 [details] [details]) > > Mario thinks this won't apply anymore to trunk, so removing cq+ > > Yeah, I need to recheck this, and maybe come with an up-to-date patch since > it's possible the modifications in AccessibilityobjectWrapperAtk.cpp are no > longer needed after fixing bug 36128. Indeed, the previous patch wouldn't apply so this new one makes a lot of sense :-). In the other hand, I was wrong in my guessing and no code can be removed just because of bug 36128, so the new patch is basically the same one than before, but properly applying against latest trunk.
Comment on attachment 57570 [details] Proposed patch Cleared Xan Lopez's review+ from obsolete attachment 57570 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 60026 [details] Proposed patch OK.
Comment on attachment 60026 [details] Proposed patch The review queue has been stuck on this patch for some time. I suspect it just needs to be landed by hand, though I'm not sure why the queue keeps spinning rather than explicitly rejecting it.
Sorry, meant to say commit queue.
(In reply to comment #11) > (From update of attachment 60026 [details]) > The commit queue has been stuck on this patch for some time. I suspect it just > needs to be landed by hand, though I'm not sure why the queue keeps spinning > rather than explicitly rejecting it. Perhaps it's a matter of the patch being too old and not applying correctly? If so, I can provide a new patch rebased against master
It's worth a shot. Normally the queue outright rejects it in such a case, but I suspect this was hitting on some bug in the queue.
Created attachment 63317 [details] Proposed patch (newer version) (In reply to comment #14) > It's worth a shot. Normally the queue outright rejects it in such a case, but I suspect this was hitting on some bug in the queue. Attached
Comment on attachment 63317 [details] Proposed patch (newer version) Rubber stamping per earlier r+
No, the CQ is stuck because the leopard build is broken. :( We need to update the "unable to build and test" message to be more specific.
Comment on attachment 60026 [details] Proposed patch Cleared Xan Lopez's review+ from obsolete attachment 60026 [details] so that this bug does not appear in http://webkit.org/pending-commit.
Comment on attachment 63317 [details] Proposed patch (newer version) Clearing flags on attachment: 63317 Committed r64659: <http://trac.webkit.org/changeset/64659>
All reviewed patches have been landed. Closing bug.
http://trac.webkit.org/changeset/64659 might have broken GTK Linux 64-bit Debug