Bug 72525 - [GTK] Consider parent AtkObject in webkit_accessible_get_parent(), if already set
Summary: [GTK] Consider parent AtkObject in webkit_accessible_get_parent(), if already...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks: 72588
  Show dependency treegraph
 
Reported: 2011-11-16 11:00 PST by Mario Sanchez Prada
Modified: 2011-11-17 03:50 PST (History)
2 users (show)

See Also:


Attachments
Patch proposal + unit test (5.96 KB, patch)
2011-11-16 11:07 PST, Mario Sanchez Prada
xan.lopez: review-
Details | Formatted Diff | Diff
Patch proposal + unit test (6.36 KB, patch)
2011-11-17 02:30 PST, Mario Sanchez Prada
xan.lopez: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mario Sanchez Prada 2011-11-16 11:00:26 PST
In AccessibilityObjectWrapperATK.cpp:webkit_accessible_get_parent(), it would be good if we would check for an already set parent AtkObject before looking for it through the a11y hierarchy, for the two following reasons:

R1. Efficiency: 
-----------------
Whenever an a11y object is recognized as the child of another one, a call to atk_object_set_parent(child, parent) is performed (see webkit_accessible_ref_child()), which makes possible to retrieve the parent in the future by calling AtkObject class's implementation of atk_object_get_parent(). Also, if at some point the hierarchy changes, future calls to atk_object_ref_accessible_child() will cause atk_object_set_parent() to be called again, updating the pointers as needed. Thus, it looks like it makes sense to return that parent object when it's already set instead of looking for it throughout the hierarchy each time atk_object_get_parent() is invoked.


R2. WebKit2:
----------------
So far, in WK1, it was fine to always look for the parent AtkObject throughout the hierarchy because whenever the root object was reached, some special checks and tricks would be done to find the AtkObject associated to the WebView's parent (a GtkWidget, so the associated AtkObject would come from GAIL) -see atkParentOfRootObject() in the ATK wrapper-, hence creating the illusion that the AtkParent of the root node in the WebKit world was that other one from the GTK world. However, with WebKit2 this is no longer possible, since the root a11y object will belong to the Web Process, while the GTK world will belong to the UI Process, so atkParentOfRootObject() will always return NULL.

However, if we make webkit_accessible_get_parent() first check if a parent has been already set through atk_object_set_parent(), we could make it so the AtkPlug object that we have in the Web Process (which is, together with the AtkSocket in the UI Process, what allows to communicate the two processes from an a11y POV) could call to atk_object_set_parent (axWkRootObject, atkPlug), that way forcing the root object from WebKit to easily recognize it as its parent and therefore returning it when something called to atk_object_get_parent(axWkRootObject), therefore fixing the bottom-up navigation throughout the whole a11y tree in WebKit2 (the magic to go up from the Web to the UI process would be done by the AtkSocket/AtkPlug combo).


I've tried some experiments so far and seem to work pretty well. Will be uploading a patch soon.
Comment 1 Mario Sanchez Prada 2011-11-16 11:07:54 PST
Created attachment 115411 [details]
Patch proposal + unit test

Patch implementing the idea explained before, plus an unit test to prove that it actually makes a difference :-)
Comment 2 Mario Sanchez Prada 2011-11-17 01:47:20 PST
This blocks a11y support in WebKit2GTK+
Comment 3 Xan Lopez 2011-11-17 02:06:04 PST
Comment on attachment 115411 [details]
Patch proposal + unit test

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

> Source/WebKit/gtk/tests/testatk.c:1669
> +    g_assert(atk_object_get_parent(axRoot) == axButton);

I think it would be useful to expand the test a bit:

- Add the view to an offscreen window. Get the parent. Should be the window.
- Now set_parent to the button, like you do now. Now get_parent should be the button, not the window.

Hope that makes sense, that way we check the base case works and that the transition happens correctly.

Looks good to me otherwise!
Comment 4 Mario Sanchez Prada 2011-11-17 02:30:23 PST
Created attachment 115545 [details]
Patch proposal + unit test

New patch addressing the issues commented by Xan
Comment 5 Xan Lopez 2011-11-17 02:46:34 PST
Comment on attachment 115545 [details]
Patch proposal + unit test

r=me
Comment 6 Mario Sanchez Prada 2011-11-17 03:50:20 PST
Committed r100597: <http://trac.webkit.org/changeset/100597>