See also bug 440490.

Created attachment 98844 [details] [review]
remove hardcoding of "/" and OrcaKey

Perhaps this is a silly question (as if that would stop me from asking it), but....  Why are there not two separate handlers for this command?  Seems to me that whereAmI can be thought of as two different beasts:

* Beast 1: Tell me about the thing I'm in/on.
  * Single-click: basic info
  * Double-click: detailed/custom info

* Beast 2: Tell me about the window I'm in.
  * Single-click: its title
  * Double-click: its status bar and/or default button

Making them two separate handlers solves the problem originally reported in this bug.  Plus it removes the hard-coding of the OrcaKey for the desktop layout which strikes me as being almost equally problematic.  Looking at the revision logs, they started out as two separate handlers, but got collapsed into one uber-handler in revision 1757.

My other potentially-silly question is about the inclusion of the following in the current doWhereAmI():

-----------
if inputEvent and orca_state.lastInputEvent \
   and isinstance(orca_state.lastInputEvent, input_event.KeyboardEvent):
    keyString = input_event.keyEventToString(inputEvent)
    debug.println(debug.LEVEL_FINEST, "default.doWhereAmI: %s" \
                  % keyString)
else:
    return self.whereAmI.whereAmI(obj, False, False)
-----------

What does the above block buy us?  Is there a case where doWhereAmI() could get called in which one of the conditions in the if would fail?  If not, and that's just there for debugging info, do we really need to keep it in there? (I took it out in this patch.)

Anyhoo, the attached seems to work.  Tested it with both desktop and laptop layouts with both the current keybindings and with modified ones.

Thoughts?

> that whereAmI can be thought of as two different beasts:
> 
> * Beast 1: Tell me about the thing I'm in/on.
>   * Single-click: basic info
>   * Double-click: detailed/custom info
> 
> * Beast 2: Tell me about the window I'm in.
>   * Single-click: its title
>   * Double-click: its status bar and/or default button
> 
> Making them two separate handlers solves the problem originally reported in
> this bug.

To answer all of your questions, including the one about the seemingly useless block of code: I like this.  It helps clean up a chunk of the code that has always a bit confusing to me.

Question I have for you, though: if you do a runall.sh of the gtk-demo tests, do any unexpected failures happen?  These would show up in the runall.sh output as lines that having the following form: 

  ******** %d FAILURES NOT EXPECTED FOR %s

So, you should be able to grep for "NOT EXPECTED" the resulting runall.out of the following command:

./runall.sh -a `pwd`/../keystrokes/gtk-demo 2>&1 runall.out

No -- this isn't a hint that I've done so and found anything.  I haven't tried it yet. :-)

Created attachment 98852 [details]
runall.out when the patch was applied

(In reply to comment #3)
> Question I have for you, though: if you do a runall.sh of the gtk-demo tests,
> do any unexpected failures happen?  

Excellent question. :-) And the answer is.... Yes, but it turns out that they are the same ones that happen without the patch applied :-).  Namely:  

******** 4 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_combo_box2.py
******** 3 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_combo_box.py
******** 2 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_page_tab.py
******** 3 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_table.py
******** 13 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_text_multiline_navigation.py
******** 1 FAILURES NOT EXPECTED FOR /home/jd/orca/test/keystrokes/gtk-demo/role_text_multiline.py

A quick glance through them shows a few to be whitespace related, such as:

-------------
Test 2 of 7 FAILED: /home/jd/orca/test/keystrokes/gtk-demo/role_table.py:Table Where Am I
EXPECTED:
     "BRAILLE LINE:  'gtk-demo Application Shopping list Frame ScrollPane Table Number ColumnHeader 3 bottles of coke '",
     "     VISIBLE:  '3 bottles of coke ', cursor=1",
     "SPEECH OUTPUT: ''",
     "SPEECH OUTPUT: 'cell'",
     "SPEECH OUTPUT: '3'",
     "SPEECH OUTPUT: 'bottles of coke'",
     "SPEECH OUTPUT: ''",
     "SPEECH OUTPUT: 'row 1 of 5'",
ACTUAL:
     "BRAILLE LINE:  'gtk-demo Application Shopping list Frame ScrollPane Table Number ColumnHeader 3 bottles of coke'",
     "     VISIBLE:  '3 bottles of coke', cursor=1",
     "SPEECH OUTPUT: ''",
     "SPEECH OUTPUT: 'cell'",
     "SPEECH OUTPUT: '3'",
     "SPEECH OUTPUT: 'bottles of coke'",
     "SPEECH OUTPUT: ''",
     "SPEECH OUTPUT: 'row 1 of 5'",
-------------

A few seem to be related to the fact that I have a printer installed and set as the default printer (unfortunately the test user inherited that without my realizing it), so it wants 'item 2 of 3' but finds 'item 2 of 6' and so on.

I've attached the one from the run with the patch.

I ran the tests pre-patch and post-patch on my Solaris Express Community Edition b75 machine that also has the latest at-spi/pyatspi (svnversion 966) and gail (svnversion 1297).  The differences didn't occur for me, so I think we might be able to say the differences might be OS distribution and runtime environment related.  I'm still working on setting up a nightly build/test machine, so we can wait for that to resolve those kinds of differences.

So, I say please commit.  This is a good fix that cleans up horrible nastiness.  Thanks!

Thanks and done.  Moving to [pending].

Created attachment 98926 [details] [review]
forgot to make changes in StarOffice.py

I hadn't realized that StarOffice.py had its own _processOrcaKey() and _handleOrcaKey(). These have been replaced by _speakTitleOrStatus(), and a corresponding change needs to occur here.

I tested this in both Calc and Writer and things seem to work as expected, so I committed this patch. But since Rich is the one with the familiarity with this script.... Rich, mind taking a look when you get a chance?  Thanks.

> Rich, mind taking a look when you get a chance?

Look fine. Tested with oocalc and oowriter on my Ubuntu 7.10
(Gutsy Gibbon - October 2007) system. 

Thanks.

Thanks Rich!  Re-moving to [pending] removing [rich]. :-)

Joanie - feel free to close.  Thanks!

Sounds good.  Thanks!