GNOME Bugzilla – Bug 427722
Web page separators cause Orca to get stuck with go{Next,Previous}Line
Last modified: 2007-04-20 19:09:39 UTC
Mike pointed out a page via email where we were getting stuck: http://ubuntuforums.org/showthread.php?t=75978&page=2 This page has a number of issues we should probably be handling, but stickage being the worst.... Steps to reproduce: 1. Navigate to the line that says: "Re: HOWTO: Using Bluetooth to acces Mobile Phone and use BT-Headset to use Skype" 2. Be sure that the caret is at the beginning of that line and that Orca is controlling the caret, 3. Press Down Arrow. Actual results: You will be stuck, and continue to be stuck. Expected results: You would move to the next line. Seems that the separator is the culprit. Methinks that separators should be added to list of suspect roles in isUselessObject().
Created attachment 86038 [details] [review] add ROLE_SEPARATOR to list of roles to examine in isUselessObject() This removes the stickage for me. Mike? As I mentioned in the original bug, there are other things going on on this page which I will continue to look at. In the interest of sanity and closure I am preferring OBPI (one bug per issue). :-) :-)
This does seem to be a nice improvement and I can't think of any reason we care about separators. I see what you mean about other issues but I can at least navigate and read this page now.
Created attachment 86040 [details] [review] alternative which checks for things that should have text but don't In doing some more looking at Mike's problematic sample page, there is another issue that -- if taken in the extreme -- could seem like getting stuck. Namely, there are some areas where go{Next,Previous}Line seems to move you to Never Never Land: Speech says nothing, braille displays an empty line. Visually the cursor doesn't move when this is taking place and we're not landing on a new, blank line.... It seems like find{Next,Previous}CaretInOrder is finding a paragraph -- and on occasion a section -- with valid extents, but which lacks text (as in obj.text == None). <sigh> So.... What about this: We do a check towards the end of go{Next,Previous}Line. If find{Next,Previous}CaretInOrder returns an object whose role is one of the NEWLINE_ROLES (and thus something that should have some text in it), and that object has no text whatsoever, call find{Next,Previous}CaretInOrder with includeNonText set to False? Seems to work. I also tried it on our wiki and that seems to continue to work. :-) But given the hour I have not done thorough testing. Because such a change might have unanticipated (by me) side effects, I'm not obsoleting my previous patch, but merely tossing this one out as an alternative for feedback. (Mike it still treats separators as potentially "useless")
(In reply to comment #2) > This does seem to be a nice improvement and I can't think of any reason we care > about separators. I see what you mean about other issues but I can at least > navigate and read this page now. Separators are the lazy man's way of grouping information and delineating chunks of data from one another. I actually find them useful for that purpose alone. Are we sure we want to prevent screen reader users from obtaining this kind of information?
I think the good of this patch outweighs the bad. I've tried this on several sites and have not found it to skip anything I care about. Can we try it and if we find out it causes problems for users in the future perhaps take a different approach?
> I think the good of this patch outweighs the bad. I've tried this on several > sites and have not found it to skip anything I care about. Can we try it and > if we find out it causes problems for users in the future perhaps take a > different approach? Mike - you're the UI guy, so I'll defer to you on this. My only protest is that we're now hiding visual information from the user. In my days of getting going with a11y in the early 90's, I had my hands and head slapped and my ears also rang from the loudness of closed-fisted pounding on the table whenever I considered such a thing. ;-)
I don't care much either way, so long as we're not getting stuck. That said... Playing devil's advocate: What is the difference between an unlabeled image (that is not a link) and a separator? Both our visual information without associated textual content.
both "are"....
> Playing devil's advocate: What is the difference between an unlabeled image > (that is not a link) and a separator? Both our visual information without > associated textual content. Agreed that skipping unlabeled images is also skipping things that might cause the closed fisted table banger to bang. :-) All I'm saying is that I have scars from even merely mentioning such heresy as a possibility in the past. We need to defer to our user experience lead for his call on this.
If it weren't causing us to get stuck I wouldn't advocate for hiding it at all. Myself I don't like having information hidden but I like getting stuck when navigating pages even less.
> If it weren't causing us to get stuck I wouldn't advocate for hiding it at all. > Myself I don't like having information hidden but I like getting stuck when > navigating pages even less. Getting rid of separators then seems like putting on bandaide on the problem without fixing the underlying problem. Joanie - do you know why separators are causing the stickage?
> Joanie - do you know why separators are causing the stickage? Nope. I'll see what I can figure out.
Created attachment 86167 [details] [review] new tactic: size DOES matter :-) The problem with separators is that they are very, very, very height-challenged. :-) This patch makes the stickage go away without denigrating separators with respect to their usefulness. It does not address the issue of being able to arrow to things that should have text but don't. (Different issue) Will, on the latter front, is it worth doing a check in go{Next,Previous}Line to see if an obj that should have text doesn't is returned by find{Next,Previous}CaretInOrder() and, if so, do another call to it with includeNonText = False? (i.e. similar to what I did in the "alternative" version, but specifying the roles rather than using NEWLINE_ROLES)
> Will, on the latter front, is it worth doing a check in go{Next,Previous}Line > to see if an obj that should have text doesn't is returned by > find{Next,Previous}CaretInOrder() and, if so, do another call to it with > includeNonText = False? (i.e. similar to what I did in the "alternative" > version, but specifying the roles rather than using NEWLINE_ROLES) What? :-)
Created attachment 86275 [details] [review] alternative which checks for things that should have text but don't > What? :-) Can't you read my mind? <melodramatic sigh> ;-) As I mentioned in comment #3, sometimes you press Up/Down Arrow on the test case page and Orca says nothing and the braille display goes blank. Try arrowing down from the line that says "General Help:" Orca will say nothing after: * All your general support questions for Ubuntu, Kubuntu, Edubuntu and Xubuntu. * Unanswered Posts * #11 * Tags: acces, bluetooth, btheadset, howto, mobile, phone, skype, use, using * Beans 15 find{Next,Previous}CaretInOrder() is finding paragraphs and sections with valid extents, but which lack text (as in obj.text == None). This block of code seems to be responsible: elif includeNonText and (startOffset < 0) \ and (not self.isLayoutOnly(obj)): extents = obj.extents if (extents.width != 0) and (extents.height != 0): return [obj, 0] I assume that the above code is in there for a very good reason (though I wouldn't mind an explanation as to what case it handles, should you have time). :-) However, arrowing by line to nothing is not ideal. So.... I was thinking that we could do a check at the end of the while loop in go{Next, Previous}Line() after we do the call to find{Next,Previous}CaretInOrder(). If we get an object, but that object doesn't have text, and that obj is of ROLE_PARAGRAPH or ROLE_SECTION (i.e. that object is of a role that should have text), then we do another call to find{Next, Previous}CaretInOrder() with IncludeNonText = False.
> Can't you read my mind? <melodramatic sigh> ;-) No...I'm just a knucklehead. Forgive me if the following seems dumb. Not sleeping well these past couple nights (wife and kid are sick, and the coughing is keeping me awake). > * All your general support questions for Ubuntu, Kubuntu, Edubuntu and Xubuntu. > * Unanswered Posts > * #11 > * Tags: acces, bluetooth, btheadset, howto, mobile, phone, skype, use, using > * Beans 15 Interesting. Some of these seem to be related to the button-ish looking things, where the content is going to great lengths to generate rounded corners for the buttons. It seems to have an HTML line for each little tangent of the rounded corner. Here's an example of what I see to draw the top: <b class="ubuntu_unapbutton"> <b class="ubuntu_unapbutton1"><b></b></b> <b class="ubuntu_unapbutton2"><b></b></b> <b class="ubuntu_unapbutton3"></b> <b class="ubuntu_unapbutton4"></b> <b class="ubuntu_unapbutton5"></b></b> (You'll see the opposite ordering of the above to draw the bottom). Interestingly, if you let Firefox control the caret, it happily stops as each one -- you end up seeing a single pixel blink. On these, it appears as though Orca is getting stuck, but it really isn't: you need to just down arrow through them. Firefox and at-poke don't want to cooperate with me right now, though, so I cannot see how these things are exposed via the AT-SPI. I do see stickage when trying to down arrow past "Re: HOWTO: Using Bluetooth to acces Mobile Phone and use BT-Headset to use Skype". If I right arrow, it definitely seems to be a separator causing the problem. Stupid separator. Is there something about that one that we can detect? 0 height? Foreground color of object same as background color of page? Something? Maybe this kind of check (once it's determined) could be added to isLayoutOnly? I'm also still not groking why that separator is causing the problem. Does your "size matters" patch help fix it (I'm afraid to try it right now - I've got enough time invested in writing this reply that I don't want *&*&%%! Firefox to crash on me before I submit this reply)? > extents, but which lack text (as in obj.text == None). This block of code > seems to be responsible: > > elif includeNonText and (startOffset < 0) \ > and (not self.isLayoutOnly(obj)): > extents = obj.extents > if (extents.width != 0) and (extents.height != 0): > return [obj, 0] This is saying: "hey, if you want me to include non-textual objects (such as a checkbox), I will do so as long as they aren't for layout purposes and they actually take up real estate". > :-) However, arrowing by line to nothing is not ideal. Agreed, but if there is a blank, we should say "blank" (assuming the settings say the user wants to hear "blank"). But, if there's a stupid blank, we should skip it. :-) Maybe we need a "skip blank lines" option for Orca? > So.... I was thinking > that we could do a check at the end of the while loop in go{Next, > Previous}Line() after we do the call to find{Next,Previous}CaretInOrder(). If > we get an object, but that object doesn't have text, and that obj is of > ROLE_PARAGRAPH or ROLE_SECTION (i.e. that object is of a role that should have > text), then we do another call to find{Next, Previous}CaretInOrder() with > IncludeNonText = False. What do these ROLE_PARAGRAPH or ROLE_SECTION objects with no text end up doing? Can we also do a check for them in isLayoutOnly?
>> Can't you read my mind? <melodramatic sigh> ;-) > >No...I'm just a knucklehead. I was just joking about having made a cryptic comment that assumed you could read my mind.... Sorry! > I'm also still not groking why that separator is causing > the problem. Does your "size matters" patch help fix it It totally fixes it! :-) Here's what turned out to be the case: In goNext{Previous}Line, we hit this section of code: if validExtents and extents != (0, 0, 0, 0): if not self.onSameLine(extents, lineExtents): if not crossedLineBoundary: We have validExtents and extents != (0, 0, 0, 0) so it's off to onSameLine(). In the case of separators, extents == lineExtents so you'd think that onSameLine would return True. BUT: separators are often height-challenged. The offending separator had a height of 1. So you're moving along in onSameLine() and get to this block of code: # If there's an overlap of 1 pixel or less, they are on different # lines. Keep in mind "lowest" and "highest" mean visually on the # screen, but that the value is the y coordinate. # highestBottom = min(a[1] + a[3], b[1] + b[3]) lowestTop = max(a[1], b[1]) if lowestTop >= highestBottom - 1: return False Let's say that extents and lineExtents are [1342, 788, 1541, 1] (which is what they were during my testing). highestBottom = min(788+1, 788+1) = 789 lowestTop = min(788, 788) = 788 lowestTop == highestBottom -1 therefore, return False (i.e. the space occupied by a and the space occupied by b are not on the same line) If a == b, by definition, the space occupied by a and b are on the same line because it's the same blessed space. :-) THAT'S what my patch checks for, and that check solves the issue of our getting stuck on the separator. > I've got enough time invested in writing this reply that I don't > want *&*&%%! Firefox to crash on me before I submit this reply)? That's why I write my replies in a separate editor and paste them in to Firefox. :-) >> :-) However, arrowing by line to nothing is not ideal. > Agreed, but if there is a blank, we should say "blank" (assuming the settings > say the user wants to hear "blank"). But, if there's a stupid blank, we should > skip it. :-) Exactly. And there are some true blank lines on that page, and we do land on them and Orca does say "blank." That's not the case in the examples I mentioned in my previous comment. > What do these ROLE_PARAGRAPH or ROLE_SECTION objects with no text end up doing? I'm not sure I follow you. These are the creatures that we are able to arrow by line to, but they have no text, and they are not blank lines, so Orca says/displays nothing. > Can we also do a check for them in isLayoutOnly? Dunno. I'll look into that though. Thanks!
> > What do these ROLE_PARAGRAPH or ROLE_SECTION objects with no text end up doing? > > I'm not sure I follow you. These are the creatures that we are able to arrow > by line to, but they have no text, and they are not blank lines, so Orca > says/displays nothing. Are these those silly rounded corner things we're running into? That is, is FF turning these things into the weirdo "I am a text object, but I'm not a text object" paragraph and sections? <b class="ubuntu_unapbutton1"><b></b></b>
I *believe* so.
I did some more at-poking around and compared paragraphs with no text that we're not stopping on with the stuff in the vicinity of the silly rounded corner things. In both cases obj.text == None. Normal paragraphs where obj.text == none have the following states: * Enabled * Horizontal * Sensitive The offending paragraphs also have: * opaque * showing * visible I assume *that* has something to do with why we're landing on them? Regardless, what's the best way to proceed on this? Move this check: if obj and not obj.text and \ obj.role in [rolenames.ROLE_PARAGRAPH, rolenames.ROLE_SECTION]: to the top of isLayoutOnly? Leave them alone because you can, as you pointed out, arrow to them if Orca isn't running? Do something else?
> I assume *that* has something to do with why we're landing on them? Probably. :-( > Regardless, what's the best way to proceed on this? Move this check: > > if obj and not obj.text and \ > obj.role in [rolenames.ROLE_PARAGRAPH, rolenames.ROLE_SECTION]: > > to the top of isLayoutOnly? Leave them alone because you can, as you pointed > out, arrow to them if Orca isn't running? Do something else? Putting the check in isLayoutOnly seems like the thing to do. What happens when you try this?
Created attachment 86317 [details] [review] moved the check > Putting the check in isLayoutOnly seems like the thing to do. What > happens when you try this? It solves the problem. That said, looking at isLayoutOnly(), it says: """Returns True if the given object is a table and is for layout purposes only.""" And all the other checks for the usefulness of an object are taking place in isUselessObject() which is the first thing that isLayoutOnly() calls. So it seemed somewhat more consistent to put the check there instead. What do you think? On a related note, I could not just add paragraph to the list of roles to check because of this block of code from default.getDisplayedText(): elif unicodeText: # [[[TODO: HACK - Welll.....we'll just plain ignore any # text with EMBEDDED_OBJECT_CHARACTERs here. We still need a # general case to handle this stuff and expand objects # with EMBEDDED_OBJECT_CHARACTERs.]]] # for i in range(0, len(unicodeText)): if unicodeText[i] == self.EMBEDDED_OBJECT_CHARACTER: displayedText = None break It was causing things like the "HOT HOT HOT: Notes on access to Firefox 3.0" line on our wiki to get skipped over when navigating by large object. :-(
Will, is this okay to check in?
(In reply to comment #23) > Will, is this okay to check in? > Nag nag nag. :-) It's on my list of bugs to evaluate today. The only comment I have to make on it is in regards to: > It solves the problem. That said, looking at isLayoutOnly(), it says: > > """Returns True if the given object is a table and is for layout > purposes only.""" > > And all the other checks for the usefulness of an object are taking place in > isUselessObject() which is the first thing that isLayoutOnly() calls. So it > seemed somewhat more consistent to put the check there instead. What do you > think? One thing is that the comment for isLayoutOnly needs to be changed since it ultimately deals with more than just tables now. The other thing is the isLayoutOnly versus isUselessObject relationship, which is getting kind of odd now. But, I guess that's covered in Bug 412661. So...go ahead, commit and update the comment for isLayoutOnly if you have the energy. :-)
> So...go ahead, commit and update the comment for > isLayoutOnly if you have the energy. :-) That's why I wash my Ritalin down with espresso shots. :-) Done and done. Thanks!
This seems to be fixed with Joanie's recent work. Thanks Joanie
Okey doke. Closing as FIXED. Thanks!