GNOME Bugzilla – Bug 354463
[requirement] "Find" command
Last modified: 2008-07-22 19:24:15 UTC
Orca should support a "find" command to find an object in a window or on the screen by name. For more information see: http://cvs.gnome.org/viewcvs/*checkout*/orca/docs/doc-set/orca.html#URFIND
Tracking.
Add accessibility keyword. Apologies for spam.
Created attachment 77311 [details] [review] Patch that *mostly* works. I took a stab at this under Rich's patient tutelage. (Thanks Rich!!!) The good news is that Find, Find Next, and Find Previous seem to work for the most part. :-) There are a couple of problems I could use some help with, however. I'll add those as separate comments.
Created attachment 77312 [details] Sample text file I've used to test find and refer to below. A focus event seems to cause Find to always start from the *begining* of the visible text in OOo Writer rather than from the present location. To see this behavior in comparison with how it should work. 1. Open testDoc (attached) in Gedit 2. Move several lines down into the document 3. Search for "golf" Results: Orca will move to the next instance of "golf" based on your present location (as expected) 4. Perform the above test in OOo Writer. Results: Orca always starts from the top regardless of your position. Plus it then reads the line that contains the caret. :-( Notes: This is only true when using the Orca Find dialog. If you restore focus tracking mode and then use Ins KP_Del for find next, Orca finds the next instance of "golf" *based on your current location* (as expected). The culprit seems to be this event: vvvvv PROCESS OBJECT EVENT focus: vvvvv OBJECT EVENT: focus: detail=(0,0) ^^^^^ PROCESS OBJECT EVENT focus: ^^^^^ With other apps (thus far), I can reliably count on object:state-changed:active as my trigger for the search. I do get such an event with Writer: vvvvv PROCESS OBJECT EVENT object:state-changed:active vvvvv OBJECT EVENT: object:state-changed:active detail=(1,0) ^^^^^ PROCESS OBJECT EVENT object:state-changed:active ^^^^^ But it's immediately followed by the aforementioned "focus:" event. Ideas??
Problem #2 (using testDoc attached above) 1. Launch gnome-terminal 2. less /path/to/testDoc (be sure top of terminal window contains top of the document) 3. KP_Del for Orca Find dialog 4. type "golf" and press Enter Results: Orca wraps to the top (as focus is at the bottom) and finds "golf" at the top. So far so good. 5. Ins KP_Del for find next 6. Rinse and repeat several times Results: Orca continues to find subsequent instances of "golf" (as expected) 7. Ins Shift KP_Del for find previous 8. Rinse and repeat Results: Orca will find previous instances as expected until you try to move to the instance on the first line. At that point, you get this: Traceback (most recent call last):
+ Trace 90115
consumed = self._function(script, inputEvent)
self.find(lastQuery)
location.wordIndex, location.charIndex)
self.targetCharInfo = self.getCurrent(Context.CHAR)
char = chars[self.charIndex]
I see a similar problem when using Find Previous to locate icons on the Desktop.
Thanks Joanie. I'll start to have a look at these problems.
Created attachment 77389 [details] Simpler test file. Using this small test file, I've replicated the second "index out of range" problem as follows. * Create a gnome-terminal window, 80chars wide x 10 lines tall. * Turn of scrollbars in gnome-terminal (Edit->Current Profile, Scrolling tab, Scrollbar is: disabled). * Start Orca with the patch applied. * In the small gnome-terminal window, run: % less smallfile * Hit the Del key on the numeric keypad. * Enter "hit" as the string to find. Hit Return. * Searching wraps around, and find the "hit" on the first line in the gnome-terminal window. * Enter Insert-Del and it find the next "hit" on the third line in the terminal window. * Enter Shift-Insert-Del and instead of again finding the "hit" on the first line in the gnome-terminal window, it gets an "index out of range" exception. The problem is the context is being setup incorrectly. I added the following routine to find.py that gets called near the beginning of the findQuery() method: def dumpContext(self, context): print "DUMP" for i in range(0, len(context.lines)): print " Line %d" % i for j in range(0, len(context.lines[i].zones)): print " Zone: %d" % j for k in range(0, len(context.lines[i].zones[j].words)): print " Word %d = `%s` len(word): %d" % \ (k, context.lines[i].zones[j].words[k].string, \ len(context.lines[i].zones[j].words[k].string)) When it dumps the context for that small gnome-terminal window is gives: DUMP Line 0 Zone: 0 Word 0 = `File` len(word): 4 Zone: 1 Word 0 = `Edit` len(word): 4 Zone: 2 Word 0 = `View` len(word): 4 Zone: 3 Word 0 = `Terminal` len(word): 8 Zone: 4 Word 0 = `Tabs` len(word): 4 Zone: 5 Word 0 = `Help` len(word): 4 Line 1 Zone: 0 Word 0 = `one` len(word): 3 Word 1 = ` ` len(word): 1 Word 2 = `hit` len(word): 3 Word 3 = ` ` len(word): 1 Word 4 = `two` len(word): 3 Word 5 = ` ` len(word): 70 Line 2 Zone: 0 Word 0 = `three` len(word): 5 Word 1 = ` ` len(word): 1 Word 2 = `four` len(word): 4 Word 3 = ` ` len(word): 2 Line 3 Zone: 0 Word 0 = `five` len(word): 4 Word 1 = ` ` len(word): 1 Word 2 = `six` len(word): 3 Word 3 = ` ` len(word): 1 Word 4 = `hit` len(word): 3 Word 5 = ` ` len(word): 2 Line 4 Zone: 0 Word 0 = `seven` len(word): 5 Word 1 = ` ` len(word): 2 Line 5 Zone: 0 Word 0 = `eight` len(word): 5 Word 1 = ` ` len(word): 2 Line 6 Zone: 0 Word 0 = `hit` len(word): 3 Word 1 = ` ` len(word): 1 Word 2 = `nine` len(word): 4 Word 3 = ` ` len(word): 1 Word 4 = `ten` len(word): 3 Word 5 = ` ` len(word): 2 Line 7 Zone: 0 Word 0 = `elevel` len(word): 6 Word 1 = ` ` len(word): 2 Line 8 Zone: 0 Word 0 = `twelve` len(word): 6 Word 1 = ` ` len(word): 2 Line 9 Zone: 0 Word 0 = `thirteen` len(word): 8 Word 1 = ` ` len(word): 2 Line 10 Zone: 0 Word 0 = `smallfile` len(word): 9 Word 1 = ` ` len(word): 72 Notice that there are a couple of bogus length for the newlines at the end of lines 1 and 10. I not need to determine why this is happening, but was guess is that we are getting bogus information back from gnome-terminal.
s/I not need to determine/I now need to determine/
Created attachment 77392 [details] [review] New revision of the find patch. Mainly adding in some debug clauses to make it easier to see what's going on.
Hi Rich. I *think* I may have it. 1. gnome-terminal seems to be padding those lines with spaces. You can actually flat review character by character through them. 2. I seem to have overlooked something which became apparent because of the above behavior: When searching backwards, you want to start at the end of the line/zone. goEnd is, of course, setting context.charIndex in the process. In the case of the padded line, charIndex might be something like 45. Then along comes the following (find.py line 189): if self.searchBackwards and \ (offsetDiff < 0) or \ not self.searchBackwards and \ (offsetDiff > 0): context.wordIndex = nextInstance.index found = True If you are search backwards and happen to find a match, wordIndex gets set appropriately but charIndex does not. Thus you might try to move to word 4, char 45 (which doesn't exist, hence the traceback). We aren't seeing this problem when searching forward because charIndex is 0. The find routine reviews the current word in which the string was found, even if it is a partial match with the string occuring in the middle of the word. Therefore, I think the solution to this problem is to set the charIndex to 0: if self.searchBackwards and \ (offsetDiff < 0) or \ not self.searchBackwards and \ (offsetDiff > 0): context.wordIndex = nextInstance.index context.charIndex = 0 found = True This seems to solve the problem in gnome-terminal as well as the problem I was occasionally seeing when searching desktop icons. What do you think?
I just noticed something else that should be changed -- not related to the above, however. In default.py, onStateChanged: if event.type == "object:state-changed:active": try: if self.findCommandRun: self.findCommandRun = False self.find() return except: debug.println(debug.LEVEL_FINEST, "onStateChanged: searchQuery not initialized") The try/except is left-over from the way I was initially checking to see if a find needed to be performed. When I switched to making it a part of script.py as you had suggested, setting it to False in __init__, I neglected to remove the try/except -- which should no longer be necessary. Thus: if event.type == "object:state-changed:active": if self.findCommandRun: self.findCommandRun = False self.find() return
Created attachment 77428 [details] [review] Patch to fix the second problem (gnome-terminal). Your fix looks great. Thanks! I've also removed the try/except clause as you suggest. I've checked this patch into CVS HEAD to make it easier to work with (I had to resolve conflicts in default.py in order to generate it because of other work that's going on in the same area). Leaving the bug open so that we can adjust the other remaining known problem.
'Fraid I found another unique case that doesn't work as expected. Documenting it now; will try to fix it later today/tonight. Problem: Given a flat review context in which there is 1. Only one instance of the string to be found 2. That instance is the last string in the context Searching backwards will not cause that item to be located. To reproduce: 1. Launch Gedit 2. In the resulting blank document, type "This is a test." 3. Get into the Orca Find dialog, type "test", check the Search backwards checkbox, and then press the Find button
I've just tried to reproduce the problem in comment #4. On my Ubuntu Edgy system, with all latest updates plus Orca from CVS HEAD, I can reliably get OOo (v2.0.4) to go into Error Recovery mode when I hit Return in the "Search for:" text field in the Orca Find dialog. (I'll file another OOo bug on that now). I've no idea why this works for you :-( The fix I was going to try (and perhaps, you can investigate in this area Joanie, as you seem to have a working OOo ...) was to modify two methods in the StarOffice.py script: In onStateChanged(), I'd try adding something like the following near the top: if event.type == "object:state-changed:active": if self.findCommandRun: return In locusOfFocusChanged(), I'd now add something like the following near the top (for now -- if this works, we can setup a new numbered clause in that routine): if self.findCommandRun: self.findCommandRun = False self.find() return
I've just filed issue #72155 http://www.openoffice.org/issues/show_bug.cgi?id=72155 against OOo oowriter for their problem.
W.r.t. the problem in comment #13. It looks like it's a problem in the regular expression parsing. If I do the following: 1. Launch Gedit 2. In the resulting blank document, type "This is a test " 3. Get into the Orca Find dialog, type "test", check the Search backwards checkbox, and then press the Find button it happily find the "test" string. My knowledge of regexp strings is minimal so I'm leaving that to after lunch.
Created attachment 77451 [details] [review] Fixup diffs to find.py - self.debugLevel should default to debug.debugLevel - Only print out final location variable, if not None. - Removed unwanted commented out section. Patch checked into CVS HEAD. These changes don't fix the problem in comment #13, but they do prevent the Traceback: AttributeError: 'NoneType' object has no attribute 'lineIndex'
w.r.t. comment #14: Oh so very close! If I take the code you suggest and add a check in locusOfFocusChanged() to be sure the event type is "object:state-changed:focused", it works. (Without that check, the window:activate event seemed to be triggering the find.) In onStateChanged(), added the following near the top: if event.type == "object:state-changed:active": if self.findCommandRun: return In locusOfFocusChanged(), added the following near the top: if self.findCommandRun and \ event.type == "object:state-changed:focused": self.findCommandRun = False self.find() return Thanks so much!!! As for OOo going into Error Recovery mode, I couldn't tell you. On all three of my linux boxes, I have Ubuntu and OOo 2.0.4 (as installed with Ubuntu; NOT the version from openoffice.org). Two of my systems are running Edgy (fully updated). The other is an Edgy-Feisty hybrid (i.e. I'm using Feisty repositories now).
Created attachment 77458 [details] [review] Patch to fix the oowriter find problem. Thanks Joanie. I'm flying a little blind here. I've checked the attached patch in. Please let me know if it *DOESN'T* fix the problem, per your remarks in comment #18. It should do.
Created attachment 77497 [details] [review] Patch from Joanie to fix the problem described in comment #13 Thanks Joanie. I've checked your patch into CVS HEAD. I'm also including your other comments here that you sent to me in email, so they are not lost. I need to work on another bug today, so if we want to get this done before the GNOME 2.17.3 deadline on Monday, it might need further help from yourself or Will. ---- (From Joanie): The good news: I think the attached diffs should handle the missing case. There are three instances (and hopefully no more!) when offsetDiff is 0: 1. We didn't start our search in this zone, but moved to it in the process of searching and just so happened to land on what we were looking for. This was already being handled explicitly. 2. We did start our search in this zone and are already on a match. We know that we are on a match because we just did a find (or find next or find previous). This isn't the one we want, so we're doing a(nother) find next/find previous. This was a known case, but being handled indirectly by the else. 3. We did start our search in this zone and are already on a match, but we don't know it. It's another case of happenstance (i.e. case #1). However, because offsetDiff is 0 and startedInThisZone is True, it was not being distinguished from #2. What distinguishes #2 from #3, I think, is that in #2 startedInThisZone is True, BUT we started in flat review. In #3 startedInThisZone is True, but we did NOT start in flat review. Hence, my proposed changes. With those changes, you will find "test." in the test case I provided. HOWEVER, Orca indicates that it's wrapping to bottom first. That isn't quite right, which brings me to... The bad news: We seem to be getting bad info about our present location *when we start the search from the very end of the current context.* Examples: 1. Type "This is a test." (no carriage return at the end) Position the cursor between the t and the period. From your very handy debug info we get: findQuery: original context line=3 zone=0 word=6 char=4 Position the cursor after the period. We get: findQuery: original context line=3 zone=0 word=0 char=0 * 2. Go back and add a carriage return at the end of "This is a test." Between t and the period: findQuery: original context line=3 zone=0 word=6 char=4 After the period: findQuery: original context line=3 zone=0 word=7 char=0 * wordIndex and charIndex aren't getting set when the caret is at the very end of the context. I just spent a couple of hours reacquainting myself with flat_review.py. Observations: In the __init__ for the Context class, foundZoneWithCaret (line 448) is failing to be true. The reason it is failing to become true seems to be that in this block (beginning with 442): ------ if (caretOffset >= zone.startOffset) \ and (caretOffset \ < (zone.startOffset + zone.length)): foundZoneWithCaret = True ------ caretOffset == zone.startOffset + zone.length *when the caret is at the very end of the context.* If you test for equality in that block, wordIndex and charIndex get set. However, setCurrent then fails (see line 1132): self.targetCharInfo = self.getCurrent(Context.CHAR) If you look at getCurrent, beginning with line 1192: elif type == Context.CHAR: if isinstance(zone, TextZone): words = zone.words if words: chars = zone.words[self.wordIndex].chars if chars: char = chars[self.charIndex] return [char.string, char.x, char.y, char.width, char.height] else: word = words[self.wordIndex] return [word.string, word.x, word.y, word.width, word.height] It seems that: 1. if words is evaluating to True 2. if chars is evaluating to False 3. the subsequent else clause is blowing up somehow.
I have found a test case that seems to indicate that the flat review problem in Comment #20 is independent of the "Find" command. Filed a bug here: http://bugzilla.gnome.org/show_bug.cgi?id=381391
Good work Joanie. Okay, then it looks like the only remaining thing to do it work out what the magic keyboard shortcuts for "Find", "Find Next" and "Find Previous" are when in laptop mode (currently there are none present). Mike, I understand you are working on this...
Created attachment 77516 [details] [review] patch to add laptop layout keybindings As per Mike's instructions, the laptop layout keybindings are: Find: Orca Modifier + [ Find Next: Orca Modifier + ] Find Previous: Orca Modifier + Control + ]
Looks good. Laptop binding patch checked into CVS HEAD. I reckon this is now complete and we can close this bug out as FIXED, but I'll let Mike and Will chirp in before I do that. Thanks again Joanie. Very nice work.
I played with this a bit. Cool stuff! Mike, you think we can close this? (We probably should let Joanie close it since she did the lion's share of the work and deserves the points).
I agree, this is a really nice job. I've tested it in several applications and it seems to be working great. Thanks a lot Joanie for such a useful adition.
Okay then. Closing as fixed. Thanks guys!!