Bug 89223

Summary: [WK2][GTK] Implement AccessibilityUIElement in WKTR for GTK
Product: WebKit Reporter: Mario Sanchez Prada <mario>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: cfleizach, cgarcia
Priority: P2 Keywords: Gtk
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 89226    
Bug Blocks: 72588, 89224    
Attachments:
Description Flags
Patch proposal
none
Patch proposal
none
Patch proposal cfleizach: review+

Description Mario Sanchez Prada 2012-06-15 09:25:52 PDT
At the moment, WebKitTestRunner's implementation of AccessibilityUIElement is only available for the mac.

Thus, we need to provide an implementation for GTK if we want to be able to run the layout tests in WebKit2GTK+ (which we want). The idea would be to implement at least the same features we have in DRT's implementation of AccessibilityUIElement, so we can pass the same tests we already pass for WK1.
Comment 1 Mario Sanchez Prada 2012-06-15 09:59:57 PDT
Created attachment 147842 [details]
Patch proposal

The idea with this patch is to provide basic support for accessibility in GTK to WKTR, and it certainly does that, since almost 100 a11y tests are now passing...

However, some others are not passing yet and some are even crashing. Those will be addressed later in separate bugs.
Comment 2 Mario Sanchez Prada 2012-06-15 10:29:16 PDT
Created attachment 147853 [details]
Patch proposal

Ooops! Sorry, this is the right one (some tests to be skipped missing in the previous patch)
Comment 3 Mario Sanchez Prada 2012-06-18 04:49:46 PDT
Created attachment 148083 [details]
Patch proposal

New patch. Implementing AccessibilityUIElements using AtkValue interface in a better way (see Martin's comments for bug 89226) and adding two more tests that are randomly crashing to the Skipped file.
Comment 4 chris fleizach 2012-07-06 09:48:45 PDT
Comment on attachment 148083 [details]
Patch proposal

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

looks good. a few minor nits

> Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:181
> +{

seems like you could probably implement this method pretty easily

> Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:256
> +    AtkObject* parent =  atk_object_get_parent(ATK_OBJECT(m_element));

extra white space after = sign

> Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:863
> +    // FIXME: implement

two FIXME's here

> Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:869
> +    // FIXME: implement

two FIXMEs here
Comment 5 Mario Sanchez Prada 2012-07-18 03:13:05 PDT
Thanks Chris for reviewing this, just got back from holidays today, so please understand my late reply.

(In reply to comment #4)
> (From update of attachment 148083 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=148083&action=review
> 
> looks good. a few minor nits
> 
> > Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:181
> > +{
> 
> seems like you could probably implement this method pretty easily

Indeed. I just haven't done it so since I was more worried at this point about porting DRT stuff to WKTR rather than adding new stuff.

Still, I agree implementation for this one is very simple, so I just added it.

> > Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:256
> > +    AtkObject* parent =  atk_object_get_parent(ATK_OBJECT(m_element));
> 
> extra white space after = sign

For some reason check-webkit-style didn't get this one. Thanks for pointing it out.

> > Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:863
> > +    // FIXME: implement
> 
> two FIXME's here
> 
> > Tools/WebKitTestRunner/InjectedBundle/gtk/AccessibilityUIElementGtk.cpp:869
> > +    // FIXME: implement
> 
> two FIXMEs here

Both fixed
Comment 6 Mario Sanchez Prada 2012-07-18 03:20:35 PDT
Committed r122940: <http://trac.webkit.org/changeset/122940>