Bug 25411

Xan: As icky as this one might be, what do you think about giving this one a go next(ish)?

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.

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).

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.

Comment on attachment 41372 [details] proposed patch - take 1 Looking at the history indicates that it should be fine to use set_parent here.

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?

(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.

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.

(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.

Comment on attachment 41372 [details] proposed patch - take 1 Clear the flags.

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?

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. :-)

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.

Comment on attachment 41413 [details] More conservative approach Clearing flags on attachment: 41413 Committed r49885: <http://trac.webkit.org/changeset/49885>

All reviewed patches have been landed. Closing bug.

(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. :-)