GNOME Bugzilla – Bug 407948
{Shift+}o and {Shift+}h do not work reliably in Firefox
Last modified: 2007-03-27 18:01:49 UTC
The Orca specification for Firefox states that pressing o/Shift+o in HTML content should move the caret/focus to the next/previous logical object. Pressing h/Shift+h should move the caret/focus to the next/previous header. This behavior is currently unreliable. We're working on test cases to demonstrate the problems. One example where o/Shift+o fails is on http://bugzilla.gnome.org/buglist.cgi?query=product%3Aorca.
Both o/Shift+o and h/Shift+h seem to get stuck in http://slashdot.org/ as well.
Created attachment 83132 [details] test case - explained in the comment Hey Will. I just tried out your latest version of Gecko.py. Much less stickage. Yea!!! In the ChangeLog you mentioned there were still problems with the URLs in this bug. I took a look at Slashdot and think I've found the source of the problem -- at least the problem *there*: Slashdot has at least one form on it that contains a fieldset whose contents are not properly formatted as paragraphs. A few things seem to result. Rather than deal with that content, please take a look at the attached and: 1. Try to use H to move from the top to the heading beyond the form. I cannot do it. 2. Press Down Arrow from the top. If you Down Arrow past the legend ("Answer Me This Then") with orca.Gecko.controlCaretNavigation = True, the caret will move to the top of the page automatically. 3. Give the page a poke. The form contents aren't visible (though the heading and paragraph beyond it are). 4. Wrap the form contents (not counting the legend) in <p></p> and try 1-3 again. All the problems go away.
w.r.t #1: According to at-poke, the panel (which seems to be defined by/as the fieldset) has one child: a text object representing the legend. According to obj.childCount when obj is the panel, there are eight children. Because the panel claims to have eight children, when obj is the text object representing the legend, the following line in findNextObject() is True: elif (obj.index + 1) < obj.parent.childCount: However, this: nextObj = obj.parent.child(obj.index + 1) Gives us this error: Child at index 1 is not an Accessible Perhaps we can do some defensive handling? Even if we cannot get *into* the form under these conditions, perhaps we can steer around it....
Created attachment 83142 [details] [review] Patch with some defensive code -- not quite right though :-( Good news 1: The attached patch seems to solve the stickage problem in slashdot and on my sample document. Good news 2: The attached patch seems fine when moving forward by H/O (fine = doesn't skip over anything) Bad news 1: The attached patch *sometimes* skips over an item when moving backward by Shift+H/Shift+O. Example: When using Shift+O in my sample document, you reliably skip over the legend. When using Shift+H on Slashdot, you reliably skip over the Slashdot Poll heading. I'm sure why this is occurring is really quite obvious, but my brain has just decided to call it quits. :-/
p.s. that patch does not solve the problem of not being able to down arrow past the legend when Orca is controlling caret navigation.
Will: w.r.t. Orca getting stuck on our bugzilla page.... It's not. :-) Take rolenames.ROLE_IMAGE out of OBJECT_ROLES and give O a go. There's a bit of a pause as it works its way down to the Long Format button, but O works. What seems to be going on is that we're landing on those "x" graphics. Those "x" graphics lack alternative text: <img src="images/emblems/P.png" width="16" height="16" alt=""> If you leave OBJECT_ROLES alone, ignore the silence, and keep pressing O a bunch of times -- 28 I think -- you'll eventually get to the Long Format button. So the bug in this instance is really that an unlabeled graphic does not a chunk make.
This is a cool finding. As we don't speek/braille images that arn't clickable or labeled these should probably be skipped by the o and shift+o anyway. From the user perspective I don't think I've ever cared about an image that was not clickable and had a meaningless name.
Here's a modified patch that attempts to do two things: 1) Harden the defenses against the broken hierarchy we get from Firefox/Gecko. Boy, does that ever muck with any cleanliness that might have been there before. 2) Add self.isUselessObject checks. These will skip things like objects that are there for layout purposes only and useless images with no name or information associated with them. In between phone calls, urgent e-mails, and IRC/AIM pounces, I've only been able to test this a little bit today.
Created attachment 83181 [details] [review] Patch to try to work around useless objects and broken Gecko hiearchies Here's a modified patch that attempts to do two things: 1) Harden the defenses against the broken hierarchy we get from Firefox/Gecko. Boy, does that ever muck with any cleanliness that might have been there before. 2) Add self.isUselessObject checks. These will skip things like objects that are there for layout purposes only and useless images with no name or information associated with them. In between phone calls, urgent e-mails, and IRC/AIM pounces, I've only been able to test this a little bit today.
This patch seems to work overall. But Shift H gets tripped up by the slashdot poll. On Slashdot, use H to move among headings until you get to the sidebar on the right. Keep H'ing through those headings. You'll pass over the poll, and book reviews. Having passed those, trying using Shift H to backtrack. You'll get stuck at Book Reviews. The previous item is the offending form with the parent who lies about the number of children it has.
I'm clearly not going to get anything done today on the RealJob front. :-) So.... In your patched findPreviousObject: while previousObj.childCount: index = previousObj.childCount - 1 child = None <-------- add me -------------- while index >= 0: child = previousObj.child(index) if child: previousObj = child break else: index -= 1 if child or (index < 0): <-------- add me -------------- break making the indicated changes seem to solve the problem of getting stuck as I described in the previous comment.
With or without the above change, it seems that we're sometimes moving forward to headings but not speaking them. The middle column of slashdot seems to be a good example of where this can occur. Investigating further....
> I'm clearly not going to get anything done today on the RealJob front. :-) :-) > In your patched findPreviousObject: > > while previousObj.childCount: > index = previousObj.childCount - 1 > child = None <-------- add me -------------- > while index >= 0: > child = previousObj.child(index) > if child: > previousObj = child > break > else: > index -= 1 > if child or (index < 0): <-------- add me -------------- > break > > making the indicated changes seem to solve the problem of getting stuck as I > described in the previous comment. I'll need to dig into this some. I think the 'add me' lines are breaking the algorithm of finding the previous object. What it's trying to do is dive down the right hand side of the tree. By breaking on the existence of 'child' (which is also what previousObj is at this point), there's the risk of not really diving down all the way (i.e., child might have children and we want dig down inside the last one). I think the thing to do, which I haven't had time to do, is to just take a snapshot of the HTML causing us pain and just progressively eliminate the surrounding HTML until we understand the construct that's the root of the problem.
> I think the thing to do, which I haven't had time to do, is to just take a > snapshot of the HTML causing us pain and just progressively eliminate the > surrounding HTML until we understand the construct that's the root of the > problem. That's how I came up with my "poll" sample. But something else is going on that I must have eliminated. I'll see if I can come up with a new sample that illustrates the problem.... I'm still trying to work out what's happening going forward. It may be a couple of things because sometimes a heading is not spoken but we don't get any errors and navigation seems to have worked; other times we get the "Child at index x is not an Accessible" error and corresponding traceback. I *think* somewhere we need to be doing some checking to see if the object is an instance of atspi.Accessible, but each time I think I know where to do that check, I'm wrong. :-/ A snapshot/sample would probably help there as well, wouldn't it? :-)
> index x is not an Accessible" error and corresponding traceback. I *think* > somewhere we need to be doing some checking to see if the object is an instance > of atspi.Accessible, but each time I think I know where to do that check, I'm > wrong. :-/ A snapshot/sample would probably help there as well, wouldn't it? From Gecko's implementation I've typically only seen child be an Accessible (what it should be) or None (when it's broken). Have you seen it be other things?
Created attachment 83216 [details] another test case: the slashdot problem(s) isolated I took the entire Slashdot home page and recreated it locally. Then I weeded things out until I had the problems. Turns out that CSS is to blame.... Try the attached using the latest patch. You should find that moving forward with H the headings that are links get focus but are not spoken. You should also find that having moved past the form, you cannot use Shift H to move up from Book Reviews: You get stuck. To remedy the first problem, remove this line from the attached: a:focus { overflow:hidden } To remedy the second problem, remove this line from the attached: fieldset legend { display: none; } Each problem is independent of the other.
Created attachment 83218 [details] full debug.out with added check to be sure child is an Accessible Will, making the change you suggested via IM, namely to change instances of if child: in your patch to isinstance(child, atspi.Accessible): solved some of the problems on slashdot, but doesn't seem to be getting all of them. Please see the tracebacks in the attached. I'll keep looking, but if you beat me to it.... :-)
In this section of findPreviousObject: while not previousObj and index >= 0: previousObj = obj.parent.child(index) index -= 1 After the assignment previousObj = obj.parent.child(index) previousObj is not an Accessible when you're using Shift H in the attached to move from the Book Reviews heading up.
Funny what you notice after taking a break.... Will, any chance that in this section of your patch (from findPreviousObject): if not previousObj: if previousObj.role != rolenames.ROLE_DOCUMENT_FRAME: previousObj = previousObj.parent You mean: if not previousObj: if obj.role != rolenames.ROLE_DOCUMENT_FRAME: previousObj = obj.parent ? That solves the problem of getting stuck on the Book Reviews link too.
> if not previousObj: > if previousObj.role != rolenames.ROLE_DOCUMENT_FRAME: > previousObj = previousObj.parent This is definitely broken code. It's saying, "OK, you have a None. Now, go ahead and get some attributes from it." D'Oh. Sorry. > You mean: > > if not previousObj: > if obj.role != rolenames.ROLE_DOCUMENT_FRAME: > previousObj = obj.parent Yeah, that looks much better. The idea with this part is that we've tried looking at our previous siblings, but the Gecko bugs have pretty much told that the parent is lying -- we don't have any previous siblings at all. Stupid {Gecko,parent}. So, we pop up and look at the previous sibling of our parent. > That solves the problem of getting stuck on the Book Reviews link too. Well, cool! I'll work up a new patch. This one will include some WARNING debug statements and stack traces to remind us Gecko has broken hierarchies so we won't forget to log a bug with them.
Created attachment 83271 [details] [review] One more try at the patch Here's the patch with the isaccessible and obj vs. previousObj changes. In a private copy, I had added some code in there to print a stack when we got a non-None child that wasn't an Accessible, but it never seemed to get called. All I saw in the debug output were the WARNING's we get from atspi when a parent results None when we ask it for a child. So, I ripped that cod out. I'm seeing some oddities where we hear just "Link" *sometimes* when going across the headers that are links. This is odd. I debugged that for a bit, and I think it is a separate problem (maybe Orca, maybe Gecko) where we: 1) seem to get a different Accessible object for the link each time we go to it, and 2) the obj.text field of the object sometimes has text and sometimes doesn't. Yech. But, it is a separate problem, and we can take a look at it as part of 411621. Joanie, if you get a chance, can you take a look at this patch? It would be great to get it in for tomorrow's release.
> private copy, I had added some code in there to print a stack when we got a > non-None child that wasn't an Accessible, but it never seemed to get called. > All I saw in the debug output were the WARNING's we get from atspi when a > parent results None when we ask it for a child. So, I ripped that cod out. I had done the same thing since my most recent comment on the matter. And I found what you found. As for why it *seemed* to make a difference, I now think that was pure coincidence. While trying to get to the bottom of some of these issues, I'm finding that sometimes things happen and sometimes they don't. In retrospect, I lucked out with a "sometimes it works" immediately after making the change to check if child was an Accessible. *sigh* Sorry for the red herring. > Joanie, if you get a chance, can you take a look at this patch? It would be > great to get it in for tomorrow's release. It works very nicely on slashdot and yahoo news. I'll keep testing of course, but I say include it in the release. It's definitely a significant improvement in this issue.
Patch committed for Orca v2.17.92.
Per Will: The {overflow: hidden} issue is being tracked by bug #412677. Since the other issue has been fixed, I'm closing this out as FIXED.