After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 354463 - [requirement] "Find" command
[requirement] "Find" command
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
1.0.x
Other All
: Normal minor
: 2.18.0
Assigned To: Rich Burridge
Orca Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-09-05 16:10 UTC by Willie Walker
Modified: 2008-07-22 19:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch that *mostly* works. (42.97 KB, patch)
2006-11-28 21:06 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review
Sample text file I've used to test find and refer to below. (3.62 KB, text/plain)
2006-11-28 21:12 UTC, Joanmarie Diggs (IRC: joanie)
  Details
Simpler test file. (93 bytes, text/plain)
2006-11-29 22:59 UTC, Rich Burridge
  Details
New revision of the find patch. (47.13 KB, patch)
2006-11-30 00:22 UTC, Rich Burridge
none Details | Review
Patch to fix the second problem (gnome-terminal). (47.03 KB, patch)
2006-11-30 16:41 UTC, Rich Burridge
none Details | Review
Fixup diffs to find.py (1.85 KB, patch)
2006-11-30 23:02 UTC, Rich Burridge
none Details | Review
Patch to fix the oowriter find problem. (1.52 KB, patch)
2006-12-01 01:00 UTC, Rich Burridge
none Details | Review
Patch from Joanie to fix the problem described in comment #13 (2.13 KB, patch)
2006-12-01 17:22 UTC, Rich Burridge
none Details | Review
patch to add laptop layout keybindings (1.72 KB, patch)
2006-12-01 22:31 UTC, Joanmarie Diggs (IRC: joanie)
none Details | Review

Description Willie Walker 2006-09-05 16:10:14 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
Comment 1 Rich Burridge 2006-09-05 16:49:55 UTC
Tracking.
Comment 2 Willie Walker 2006-10-15 00:25:42 UTC
Add accessibility keyword.  Apologies for spam.
Comment 3 Joanmarie Diggs (IRC: joanie) 2006-11-28 21:06:58 UTC
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.
Comment 4 Joanmarie Diggs (IRC: joanie) 2006-11-28 21:12:47 UTC
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??
Comment 5 Joanmarie Diggs (IRC: joanie) 2006-11-28 21:16:52 UTC
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):
  • File "/usr/lib/python2.4/site-packages/orca/input_event.py", line 178 in processInputEvent
    consumed = self._function(script, inputEvent)
  • File "/usr/lib/python2.4/site-packages/orca/default.py", line 3295 in findPrevious
    self.find(lastQuery)
  • File "/usr/lib/python2.4/site-packages/orca/default.py", line 3263 in find
    location.wordIndex, location.charIndex)
  • File "/usr/lib/python2.4/site-packages/orca/flat_review.py", line 1132 in setCurrent
    self.targetCharInfo = self.getCurrent(Context.CHAR)
  • File "/usr/lib/python2.4/site-packages/orca/flat_review.py", line 1198 in getCurrent
    char = chars[self.charIndex]
IndexError: list index out of range

I see a similar problem when using Find Previous to locate icons on the Desktop.
Comment 6 Rich Burridge 2006-11-28 21:22:59 UTC
Thanks Joanie. I'll start to have a look at these problems.
Comment 7 Rich Burridge 2006-11-29 22:59:48 UTC
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.
Comment 8 Rich Burridge 2006-11-29 23:01:51 UTC
s/I not need to determine/I now need to determine/
Comment 9 Rich Burridge 2006-11-30 00:22:54 UTC
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.
Comment 10 Joanmarie Diggs (IRC: joanie) 2006-11-30 03:53:25 UTC
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?
Comment 11 Joanmarie Diggs (IRC: joanie) 2006-11-30 04:07:23 UTC
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
Comment 12 Rich Burridge 2006-11-30 16:41:51 UTC
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.
Comment 13 Joanmarie Diggs (IRC: joanie) 2006-11-30 17:25:13 UTC
'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
Comment 14 Rich Burridge 2006-11-30 18:22:08 UTC
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
Comment 15 Rich Burridge 2006-11-30 18:44:08 UTC
I've just filed issue #72155
http://www.openoffice.org/issues/show_bug.cgi?id=72155
against OOo oowriter for their problem.
Comment 16 Rich Burridge 2006-11-30 19:13:16 UTC
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.

Comment 17 Rich Burridge 2006-11-30 23:02:41 UTC
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'
Comment 18 Joanmarie Diggs (IRC: joanie) 2006-11-30 23:07:22 UTC
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).
Comment 19 Rich Burridge 2006-12-01 01:00:32 UTC
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.
Comment 20 Rich Burridge 2006-12-01 17:22:00 UTC
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.
Comment 21 Joanmarie Diggs (IRC: joanie) 2006-12-01 20:14:08 UTC
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
Comment 22 Rich Burridge 2006-12-01 21:37:39 UTC
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...
Comment 23 Joanmarie Diggs (IRC: joanie) 2006-12-01 22:31:53 UTC
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 + ]
Comment 24 Rich Burridge 2006-12-01 23:01:22 UTC
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.
Comment 25 Willie Walker 2006-12-11 15:35:32 UTC
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).
Comment 26 Mike Pedersen 2006-12-12 01:49:56 UTC
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.  
Comment 27 Joanmarie Diggs (IRC: joanie) 2006-12-12 14:05:16 UTC
Okay then.  Closing as fixed.  Thanks guys!!