Created attachment 85718 [details]
Orca debug,.out generated whilst reproducing this problem.

I just looked into this. I tried to investigate
the problem using the following steps:

1/ Start Orca
2/ Start gedit.
3/ Type the following two lines into gedit.

The quick
brown fox.

4/ Position the text caret below the second line.
5/ Hit the Up key. "brown fox." is spoken/brailled. This is around line
   389 in the debug file.

6/ Use the mouse to select "brown", selecting from right to left.
   Nothing is spoken/brailled.

   I added a few debug lines to the Orca code, and this is what appears
   to be happening.

* At line 434, we are creating a MouseButtonEvent (pressed) event for the
  initial mouse selection (to the right of the "n" in "brown").

* At line 437, we get an "object:text-caret-moved" (detail=16,0) event. 
  Because of the following logic in _presentTextAtNewCaretPosition() in 
  default.py:

    if isinstance(orca_state.lastInputEvent, input_event.MouseButtonEvent):
        print "default: last event was a mouse event."
        if not orca_state.lastInputEvent.pressed:
            print "default: last event was not pressed."
            self.sayLine(event.source)
        return

  we don't speak/braille anything.

* We get similar "object:text-caret-moved" for various new text caret 
  positions as the mouse is dragged left over the word "brown":

  - Line 469 (detail=15,0)
  - Line 536 (detail=14,0)
  - Line 593 (detail=13,0)
  - Line 660 (detail=12,0)
  - Line 717 (detail=11,0)

  Because the mouse button is still pressed, nothing is spoken/brailled.

* Finally at line 795, we create a MouseButtonEvent (released) event, but we
  don't get anymore "object:text-caret-moved" after that, so we don't 
  speak/braille the new text selection.

Will, any suggestions on how this should be reworked?

>     if isinstance(orca_state.lastInputEvent, input_event.MouseButtonEvent):
>         print "default: last event was a mouse event."
>         if not orca_state.lastInputEvent.pressed:
>             print "default: last event was not pressed."
>             self.sayLine(event.source)
>         return
> 
>   we don't speak/braille anything.

I did a little archeology on the code above, and it was added by me on 17-Jan-2006:

* src/orca/orca.py, src/orca/default.py, src/orca/input_event.py: keep track of mouse button events as another means for watching caret moved events.

So...now we know.  Looks like it is silly code that should be reworked.  I think we need to take a step back a figure out what the desired user behavior is for mouse actions.  These include knowing what to present to the user when:

1) They click the mouse to position the caret - do we speak/braille the line?

2) They paste text using MB2 - do we speak the pasted text and braille the line?

3) They drag the mouse to select text - do we speak the selected text and braille the line?

4) They double/triple click the mouse to select text - do we speak the selected text and braille the line?

We need to investigate deeper into what events are delivered to us (and maybe ones we should be listening for). For example, we have object:text-selection-changed handled as a noOp in default.py.  Maybe we could do more with it?

We do get (multiple) "object:text-selection-changed" events, but the 
same scenario applies there. We do not get an object:text-selection-changed" 
event after the mouse button is released.

Trying something like:


    def onTextSelectionChanged(self, event):
        """Called whenever the text selection is changed for an object.

        Arguments:
        - event: the Event
        """

        print "default: onTextSelectionChanged called."
        if isinstance(orca_state.lastInputEvent, input_event.MouseButtonEvent):
            self.sayLine(event.source)

just results in "brown fox selected" being spoken numerous times.
It's a pity the at-spi isn't designed to associate the accessibility events
with the real keyboard and mouse events that caused them.

A possible solution for this might be to do something like the following, though I admit this is not well thought out:

1) Extend orca.py:_onMouseButton to manage a new field in orca_state.py: mouseButtonState.  This state could be as simple as a dictionary where the key is the button number and the value is the state (False=not pressed, True=pressed).  Or, it could be whatever you want.  

2) If text selection occurs while a mouse button is pressed, don't do anything, but record the fact that there's a pending selection announcement.  Maybe "self._pendingTextSelection = True" or something.

3) Make a new listener in default.py for "mouse:button", call it onMouseButton.  When a button release event is detected by onMouseButton and self._pendingTextSelection == True, then output the text selection information and set self._pendingTextSelection = False.

Created attachment 90887 [details] [review]
First cut at a fix. 

Thanks Will. Here's a first cut at a patch to fix this bug.

There are three problems. The first two might be related.

1/ Now that we care about "mouse:button" events, we are getting

ERROR in Accessible.__get_app: top most parent (name='main') is of role unknown

in ___get_app() in atspi.py. Here's the full stack trace.

+ Trace 144665

2/ I could not get this to work with:

   if len(self._pendingTextSelection) > 0:
       ... handle possible text selection ...

in onMouseButton() in default.py.  I had to use:

   if len(orca_state.activeScript._pendingTextSelection) > 0:
       ... handle possible text selection ...

Thanks to Joanie for this workaround. Why self._pendingTextSelection
doesn't just work is currently a mystery. As I said, it might be related
to problem #1.

3/ Occasionally, the start offset for the text selection is off my one.
   So for "The quick brown fox", where "quick" has been selected, then
   it just thinks "uick" is selected.

  The solution here might be to add some similar logic to what's in
  onTextSelectionChanged() in onCaretMoved(). I'll experiement.

Yup, problem #1 and problem #2 are related. The first occurs when
trying to do:

        ...
        try:
            s = self._getScript(event.source.app)
        except:
            ...

in _processObjectEvent() in focus_tracking_presenter.py.

For now, I'm just going to work on finding a better solution to
problem #3, as I'm not sure what the appropriate change is to 
make to __get_app() in atspi.py to handle this (obj.role in 
there for a "mouse:button" event in "unknown").

Created attachment 90906 [details] [review]
Second cut at a fix.

This improves the situation. We also can potential hear the
current line being spoken multiple times as well. But this is
apparently now always followed by the correct speaking of the
selected text.

Still need to work out what to do in __get_app() in atspi.py too.

Created attachment 91012 [details] [review]
Third cut at the fix.

Includes in the call to "self.updateBraille(event.source)"
in onTextSelectionChanged() in default.py (see comment #17
of bug #382601 for more details -- thanks Joanie!)

Changing latest patch status to "needs-work" as we still need to work
out the best solution to the:

ERROR in Accessible.__get_app: top most parent (name='main') is of role unknown

error.

Rich, I'm curious:  Are you still getting that error?  I don't believe I have ever gotten it and I'm not getting it with the latest patch.  If so, I wonder what we're doing differently.

Yup, I'm still seeing it. This is with latest Orca from SVN HEAD
on my Feisty box. I'll work with you offline to try to determine
why you are getting something different then I am.

Aha!  Based on a quick glance in comment 5 I was searching in my debug.out for the traceback rather than looking at each event.  I am getting the error; it just wasn't jumping out at me.

vvvvv PROCESS OBJECT EVENT mouse:button:1r vvvvv
OBJECT EVENT: mouse:button:1r                          detail=(1412,186)
Finding app for source.name='main'
ERROR in Accessible.__get_app: top most parent (name='main') is of role unknown
    app=None name='main' role='unknown' state='' relations=''
Finding app for source.name='main'
ERROR in Accessible.__get_app: top most parent (name='main') is of role unknown
^^^^^ PROCESS OBJECT EVENT mouse:button:1r ^^^^^

Sorry about that!

If you monitor mouse button events using Accerciser, it seems that they are always associated with role unknown and app main rather than the actual application with focus. (i.e. it ain't us that's causing the problem)

Agreed. We just need to work out the best way to prevent the
error happening. My guess is we will need to add some new code to
_processObjectEvent() in focus_tracking_presenter.py to handle this,
but I want to talk with Will when he gets back, to see how he'd like
this done.

Will, what's the best way to handle the "ERROR in Accessible.__get_app ..."
errors we are now getting (see previous comments)?

Thanks.

> Will, what's the best way to handle the "ERROR in Accessible.__get_app ..."
> errors we are now getting (see previous comments)?

I'm not sure.  The problem is that we're getting mouse button events as AT-SPI object events as opposed to the way we get keyboard events as device events.  This seems to be a strangeness of the AT-SPI.  :-(

I think you're right in that we probably need special code in _processObjectEvent to handle this.

Created attachment 91577 [details] [review]
Patch to fixup the ERROR messages for mouse events.

Hopefully this patch is now complete. Please test (not you Mike!)
Thanks.

(In reply to comment #17)
> Created an attachment (id=91577) [edit]
> Patch to fixup the ERROR messages for mouse events.
> 
> Hopefully this patch is now complete. Please test (not you Mike!)
> Thanks.
> 

With this part of the patch, I believe the only thing that will ever handle the mouse event will be an instance of the default script:

+            app = None
+            if not event.type.startswith("mouse:"):
+                app = event.source.app
+            s = self._getScript(app)

I think the patch might be better if it did something like this:

+            if not event.type.startswith("mouse:"):
+                s = self._getScript(event.source.app)
+            else:
+                s = orca_state.activeScript
+            if not s:
+                return

The reason for this is that it would cause mouse button events to be handled similar to key events (i.e., they go to the active script, if there is one), and it would also allow scripts to provide their own custom onMouseButton methods.  

Created attachment 91579 [details] [review]
Revision patch based on Will's comments (thanks).

Rich, try the following it Gedit:

1. Write the following line:

This is a test.

2. With the caret just after the ".", select "is" with the mouse.  Works as expected.

3. Click just before the "i" of "is".  Now select "is" with the mouse from left to right.  Orca just says "s".

4. Click just after the "s" of "is".  Now select "is" with the mouse from right to left.  Orca just says "i".

Disclaimer:  Gutsy box. :-)

Turns about the above seems to be the rule (across multiple apps, and present also in Feisty).  If the caret is already at the point where the selection begins, that initial character will not be spoken.

My wrist is officially worn out now.  Back to the keyboard. :-)

Created attachment 91585 [details]
Orca debug.out running Joanie's tests.

Yup. Confirmed. Sad but true. I added in some code earlier
in onCaretMoved() to potentially fix this, but the click/release
is clearing this. I'll try adjusting how this works to see
if I can improve things.

Thanks.

I'm afraid this doesn't work at all in gnome-terminal.

Created attachment 91686 [details] [review]
Patch to fix the problem - take #2

Will suggested a much simpler approach to fixing the problem.
When we get a mouse release button, we now just look to see 
if the current locus of focus is a text object and if it has
any selections. If it has, then speak them, followed by "selected".
This seems to work nicely with the minimal testing I gave it in
gedit, OOo Writer and Evolution.

gnome-terminal is not giving any text selections back when I call:

self.whereAmI._getTextSelections(obj, True)

I also noticed that doing a "where am I" command in gnome-terminal doesn't
work, so I'm going to file a separate bug for that. Fix that, and this
problem should go away too.

This approach seems to buy us something else:  Now if you select a word by double-clicking on it, Orca speaks it.  With the previous approach I don't believe it did.  Cool!

However, now that we're no longer looking at text-selection-changed events and calling updateBraille() when we get one, the problem you pointed out here http://bugzilla.gnome.org/show_bug.cgi?id=382601#c8 is back:  If the user causes text that was selected to become unselected by clicking with the mouse at the caret offset at the beginning of the selection, we're not updating the braille display to reflect the unselected state.  Should onMouseButton() call updateBraille() to handle this "edge case"?

> Should onMouseButton() call updateBraille() to handle this "edge case"?

Yes indeed. I'll update the patch now. thanks.

Created attachment 91693 [details] [review]
Revised patch based on feedback from Joanie.

Thanks Rich! 

I think you have to outdent the call however so that it's outside of the "if textContents:" block.  We're only going to have textContents if there's nSelections right?  And there won't be if you've just unselected all the text by clicking.

Created attachment 91699 [details] [review]
Further revision to updateBraille() call in onMouseButton.

Okay. Does this look okay now?

More than okay.  It's absolutely fabulous. :-)  Thank you Rich.

Created attachment 91743 [details] [review]
Adding in the code to prevent the ERROR message.

Needed to add back in the code in focus_tracking_presenter.py
to prevent the "ERROR in Accessible ..." messages being displayed
when using the mouse.

Even though this seems to be working now (see Joanie's reply in
comment #30), it's still not checked in. This is because there is
one new localized string.

Do we want to check this in for the next Orca release and send a 
message to the GNOME i18n people, or shall we let it wait until 
the next new GNOME/Orca cycle?

> Do we want to check this in for the next Orca release and send a 
> message to the GNOME i18n people, or shall we let it wait until 
> the next new GNOME/Orca cycle?

The string added is "selected" (right?).  If so, then this isn't a new string for translation and we're safe.  Just make sure to put a comment prior to it and check it in.  The exact same stuff from line-ish 5900 (~6000 lines? WOW!) in default.py would probably suffice:

            # Translators: when the user selects (highlights) text in
            # a document, Orca lets them know this.
            #

Alternatively, would there be a way for the new code to call default.py:speakTextSelectionState?

> The string added is "selected" (right?). 

Correct.

> Just make sure to put a comment prior to it and check it in.

Okay. Done.

> Alternatively, would there be a way for the new code to call
> default.py:speakTextSelectionState?

No. That routine expects the last event to be a keyboard event.

Changing the Summary line to include "[pending]".

Thanks.

Could someone who is actually a mouse user please verify this one?

Yup! verified.  I think we're good.

Thanks Joanie. Closing as FIXED.