Bug 25411

Summary: [GTK] ATK accessible ancestry broken
Product: WebKit Reporter: Joanmarie Diggs <jdiggs>
Component: AccessibilityAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: apinheiro, commit-queue, walker.willie, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: Linux   
Bug Depends on:    
Bug Blocks: 25531    
Attachments:
Description Flags
dogtail test script to help illustrate the problem
none
proposed patch - take 1
none
More conservative approach none

Joanmarie Diggs
Reported 2009-04-26 14:21:56 PDT
Steps to reproduce: 1. In WebKit Gtk or Epiphany, view http://google.com 2. In Accerciser's tree of accessibles, locate the app chosen in step 1, and select the scroll pane. 3. In Accerciser's iPython console, type: acc[0].parent == acc 4. In Accerciser's tree of accessibles, locate the accessible of role panel which is the first child of the document frame. 5. Repeat step 3. Expected results: Steps 3 and 5 would cause True to be returned. Actual results: Steps 3 and 5 cause False to be returned. For some reason asking for the parent of the document frame returns None rather than the scroll pane which is its parent. Asking for the parent of the text object named "Web" returns an accessible of role unknown which isn't visible in the tree of accessibles.
Attachments
dogtail test script to help illustrate the problem (1.10 KB, text/plain)
2009-06-17 17:30 PDT, Joanmarie Diggs
no flags
proposed patch - take 1 (3.51 KB, patch)
2009-10-17 23:29 PDT, Joanmarie Diggs
no flags
More conservative approach (3.53 KB, patch)
2009-10-19 05:53 PDT, Joanmarie Diggs
no flags
Joanmarie Diggs
Comment 1 2009-06-10 14:19:34 PDT
Xan: As icky as this one might be, what do you think about giving this one a go next(ish)?
Joanmarie Diggs
Comment 2 2009-06-17 17:30:28 PDT
Created attachment 31476 [details] dogtail test script to help illustrate the problem Xan: I got to this sooner than I thought I would. :-) This simple dogtail script descends the hierarchy from the scroll pane (parent of the document frame), looking at the first child at each level in the hierarchy. It then reverses direction and ascends the ancestry. The end results should be the same; they are not, which is what this bug is about. :-) Examples: ~~~ $ ./bug-25411.py http://google.com ==================== TOP DOWN/HIERARCHY ==================== scroll pane document frame panel text ==================== BOTTOM UP/ANCESTRY ==================== document frame panel panel panel unknown unknown text ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ $ ./bug-25411.py http://yahoo.com ==================== TOP DOWN/HIERARCHY ==================== scroll pane document frame list link text ==================== BOTTOM UP/ANCESTRY ==================== document frame panel panel panel panel panel panel panel list list item link unknown text ~~~ Note that the top-down should match what is seen in Accerciser's tree of accessibles. Dogtail does assume a11y has been enabled. Session-wide I believe. I didn't investigate if that could be avoided. Sorry! Hope this is of some use.
Joanmarie Diggs
Comment 3 2009-10-17 22:50:34 PDT
I just spun off part of this bug as newly-filed bug 30489. This bug will deal with the brokenness *within* web content (i.e. step 5; not step 3 from the opening report).
Joanmarie Diggs
Comment 4 2009-10-17 23:29:09 PDT
Created attachment 41372 [details] proposed patch - take 1 If you set the parent in webkit_accessible_ref_child, the AtkObject will know who its parent is. This seems (to me) to eliminate the need for WebKit's implementation of atk_object_get_parent. More importantly, once webkit_accessible_get_parent is removed, AND the parent has been set, I suddenly start seeing the correct, expected ancestry. So.... Is there any reason WebKit should continue to implement atk_object_get_parent? If there is, it will need smarts to distinguish real parents from fake ones which seems (to me) to be more hassle than it is worth. But if there is, let me know and I will give it a go.
Holger Freyther
Comment 5 2009-10-18 08:49:49 PDT
Comment on attachment 41372 [details] proposed patch - take 1 Looking at the history indicates that it should be fine to use set_parent here.
Xan Lopez
Comment 6 2009-10-18 08:55:51 PDT
Comment on attachment 41372 [details] proposed patch - take 1 So, my only concern is that it seems to me with this patch you basically need to always have called ref_child for get_parent to work. Is this OK?
Joanmarie Diggs
Comment 7 2009-10-18 09:40:43 PDT
(In reply to comment #6) > (From update of attachment 41372 [details]) > So, my only concern is that it seems to me with this patch you basically need > to always have called ref_child for get_parent to work. Is this OK? Heck, I was hoping you knew. ;-) I'm still finding my way around the code, so to be honest, I'm not sure. What happens when a document is loaded in WebKit? If the process is that a document loads and an accessible version of the document is rendered from the top down, I think this would be OK. If instead, nothing gets rendered until the user interacts with it, then, no, it wouldn't be OK.
Joanmarie Diggs
Comment 8 2009-10-18 11:11:30 PDT
I'm working on the label relationship stuff at the moment, but a little voice inside my head is suggesting that it would be safer to leave the WebKit implementation of atk_object_get_parent in place, but give it some smarts.... I'll tackle that next. Sorry for the noise.
Xan Lopez
Comment 9 2009-10-18 11:45:27 PDT
(In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 41372 [details] [details]) > > So, my only concern is that it seems to me with this patch you basically need > > to always have called ref_child for get_parent to work. Is this OK? > > Heck, I was hoping you knew. ;-) I'm still finding my way around the code, so > to be honest, I'm not sure. > > What happens when a document is loaded in WebKit? If the process is that a > document loads and an accessible version of the document is rendered from the > top down, I think this would be OK. If instead, nothing gets rendered until the > user interacts with it, then, no, it wouldn't be OK. So, what I really meant is how applications like Orca use the ATK APIs. If there's any chance than get_parent would be called before ref_child then the patch is wrong, as get_parent would return NULL. AFAIK WebKit constructs the DOM tree when the page is loaded, and the a11y tree is constructed on-demand when the a11y APIs are called. But since you think the patch is wrong anyway we'll clear the queue.
Xan Lopez
Comment 10 2009-10-18 11:45:48 PDT
Comment on attachment 41372 [details] proposed patch - take 1 Clear the flags.
Joanmarie Diggs
Comment 11 2009-10-19 05:53:59 PDT
Created attachment 41413 [details] More conservative approach This is a more conservative version of the previous patch: * Still sets the parent when we ref the child * Still tries to have the Atk child identify its parent BUT * Keeps WebKit's existing implementation of get_parent in place, falling back to it when the child cannot identify its parent Doing so seems to solve the bulk of the issues because more often than not, the user is reading a page and we're descending the accessible hierarchy as a result. The case it doesn't solve (in theory) is this: * Some object the user has not interacted with suddenly starts emitting events on its own, AND * We have a need for the ancestry to perfectly match the hierarchy In which case, having fallen back on the old logic, we are no more broken than we were before. ;-) Rather than try to find such a case in the wild now, I'd rather focus on the immediate (GNOME 2.30/3.0) needs (i.e. all of the other a11y bugs we have open). As soon as we find that special case, we can open a new bug for it. Thoughts?
Joanmarie Diggs
Comment 12 2009-10-20 13:56:20 PDT
Xan: Ping. :-) Since you reviewed the Table Interface patch which I intentionally didn't ask for a review on, I figured you must have missed this one. :-)
Xan Lopez
Comment 13 2009-10-20 14:07:38 PDT
Comment on attachment 41413 [details] More conservative approach This looks like we might be covering some serious issue, but I guess it's OK for now as a workaround.
WebKit Commit Bot
Comment 14 2009-10-20 14:20:57 PDT
Comment on attachment 41413 [details] More conservative approach Clearing flags on attachment: 41413 Committed r49885: <http://trac.webkit.org/changeset/49885>
WebKit Commit Bot
Comment 15 2009-10-20 14:21:01 PDT
All reviewed patches have been landed. Closing bug.
Joanmarie Diggs
Comment 16 2009-10-20 17:21:11 PDT
(In reply to comment #13) > (From update of attachment 41413 [details]) > This looks like we might be covering some serious issue, Yes ... and no. :-) I agree that there is a serious underlying issue which needs to be identified and properly fixed. I also agree that what I've done here is merely a bandaid -- one which gets something that is already in place (WebKit's implementation of atk_object_get_parent) to work as expected from the AT client perspective. That said: * atk_object_get_index_in_parent (not yet implemented) is impacted by this same issue * The accessible hierarchy for tables is completely borked. I was hoping that it would magically unbork itself once we started getting the accessible table interface implemented. No such luck, based on what I'm seeing in my sandbox. In other words, we have at least two opportunities to identify and properly fix the serious underlying hierarchy/ancestry issue(s). I plan to start digging into this underlying issue before too long. And I'll likely be coming to you for assistance/advice once I've dug into it because I suspect it might not be trivial. All a long way of saying that the issue you're referring to is still very much on my radar and will not (arguably cannot) be forgotten. :-)
Note You need to log in before you can comment on or make changes to this bug.