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 530368 - Only move focus if the event is for the focused/active window.
Only move focus if the event is for the focused/active window.
Status: RESOLVED FIXED
Product: orca
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: 2.24.0
Assigned To: Rich Burridge
Orca Maintainers
Depends on:
Blocks:
 
 
Reported: 2008-04-28 14:46 UTC by Rich Burridge
Modified: 2008-06-04 18:09 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Revision #1 - debug version. (1.10 KB, patch)
2008-05-22 17:15 UTC, Rich Burridge
none Details | Review
Revision #2. (3.16 KB, patch)
2008-05-22 17:42 UTC, Rich Burridge
none Details | Review
Revision #3. (620 bytes, patch)
2008-05-22 18:39 UTC, Rich Burridge
none Details | Review
Revision #4. (696 bytes, patch)
2008-05-22 19:47 UTC, Rich Burridge
rejected Details | Review
Fix traceback. (607 bytes, patch)
2008-05-24 16:59 UTC, Rich Burridge
rejected Details | Review
Another Orca debug log. (57.73 KB, text/plain)
2008-05-29 17:00 UTC, Rich Burridge
  Details
Revision #5. (3.83 KB, patch)
2008-06-02 16:33 UTC, Rich Burridge
none Details | Review
debug output from trying line navigation in FF (50.06 KB, text/plain)
2008-06-02 20:54 UTC, Joanmarie Diggs (IRC: joanie)
  Details
output exhibiting the problem (25.74 KB, application/octet-stream)
2008-06-02 21:00 UTC, Mike Pedersen
  Details
Revision #6. (2.12 KB, patch)
2008-06-02 22:19 UTC, Rich Burridge
committed Details | Review

Description Rich Burridge 2008-04-28 14:46:16 UTC
This came from comment #10 of bug #523731
Opening as a separate issue so that the other bug 
can be progressed separately.

------- Comment #10 from Willie Walker  2008-04-28 13:25 UTC -------
(In reply to comment #8)
> > I do wonder though if what we do in default.onTextDeleted() is what we want to
> > be doing.  Maybe we want to check to see if we event is for a focused text area
> > *that's in a focused/active window*???

This seems like it might be a good thing to do.  Without it, the side effect
might be that we get a lot of verbose context output:

1) App "a" has focus
2) App "b" issues event that causes locus of focus to be in app "b"
3) App "a" gets an event that requires speech output -- the hierarchical
context is completely different, so we might get lots of speech output.

script.py:processObjectEvent has some rough filtering if "presentIfInactive" is
False, but the default is True.   :-)   So, maybe we need to do a more global
search for orca.setLocusOfFocus calls and make them only if self ==
orca_state.activeScript?
Comment 1 Rich Burridge 2008-05-21 16:50:41 UTC
Listed below are all the places under .../orca/src/orca
where orca.setLocusOfFocus() is called. 43 of them. Do
we want to just adjust each of them to something like:

        if self == orca_state.activeScript:
            orca.setLocusOfFocus(event, event.source, False)

?

There are currently three places where we are doing:

        # We don't always get focus: events for text areas, so if we
        # see caret moved events for a focused text area, we silently
        # set them to be the locus of focus.
        #
        if event and event.source and \
           (event.source != orca_state.locusOfFocus) and \
            event.source.getState().contains(pyatspi.STATE_FOCUSED):
            orca.setLocusOfFocus(event, event.source, False)

These are in onCaretMoved(), onTextDeleted() and onTextInserted()
in default.py.

Does that still need to be done if we do the first change
suggested above?

Just checking before I start in on this.

Thanks.

default.py:1987:            orca.setLocusOfFocus(None, context.obj, False)
default.py:3105:        orca.setLocusOfFocus(event, newFocus)
default.py:3389:            orca.setLocusOfFocus(event, event.source, False)
default.py:3421:            orca.setLocusOfFocus(event, event.source, False)
default.py:3515:            orca.setLocusOfFocus(event, event.source, False)
default.py:3619:            orca.setLocusOfFocus(event, child)
default.py:3621:            orca.setLocusOfFocus(event, event.source)
default.py:3651:        orca.setLocusOfFocus(event, event.source)
default.py:3933:            orca.setLocusOfFocus(event, newFocus)
default.py:3969:        orca.setLocusOfFocus(event, event.source)
default.py:4019:            orca.setLocusOfFocus(event, None)

scripts/apps/acroread.py:432:                    orca.setLocusOfFocus(event, oldLocusOfFocus, False)
scripts/apps/acroread.py:435:                orca.setLocusOfFocus(event, oldLocusOfFocus, False)
scripts/apps/acroread.py:444:            orca.setLocusOfFocus(event, newLocusOfFocus, False)
scripts/apps/acroread.py:459:            orca.setLocusOfFocus(event, newLocusOfFocus, False)
scripts/apps/acroread.py:476:            orca.setLocusOfFocus(event, newLocusOfFocus, False)
scripts/apps/acroread.py:557:                orca.setLocusOfFocus(event, event.source, False)

scripts/apps/gdmlogin.py:54:        orca.setLocusOfFocus(event, event.source)
scripts/apps/gdmlogin.py:60:            orca.setLocusOfFocus(event, obj)

scripts/toolkits/J2SE-access-bridge.py:244:                orca.setLocusOfFocus(event, selectedItem)
scripts/toolkits/J2SE-access-bridge.py:248:                orca.setLocusOfFocus(event, event.source)
scripts/toolkits/J2SE-access-bridge.py:258:            orca.setLocusOfFocus(event, event.source)
scripts/toolkits/J2SE-access-bridge.py:296:                                orca.setLocusOfFocus(event, item)
scripts/toolkits/J2SE-access-bridge.py:320:            orca.setLocusOfFocus(event, event.source.parent)
scripts/toolkits/J2SE-access-bridge.py:352:                    orca.setLocusOfFocus(event, event.source)
scripts/toolkits/J2SE-access-bridge.py:438:                orca.setLocusOfFocus(event, obj)

scripts/apps/evolution/script.py:496:            orca.setLocusOfFocus(None, obj, False)

scripts/apps/gedit/script.py:102:            orca.setLocusOfFocus(None, obj, False)
scripts/apps/gedit/script.py:528:            orca.setLocusOfFocus(event, event.source)

scripts/apps/soffice/script.py:1561:            orca.setLocusOfFocus(event, event.source, True)
scripts/apps/soffice/script.py:1711:            orca.setLocusOfFocus(event, event.source, False)
scripts/apps/soffice/script.py:1728:            orca.setLocusOfFocus(None, event.source.parent, False)

scripts/apps/Thunderbird/script.py:104:            orca.setLocusOfFocus(event, acc)

scripts/toolkits/Gecko/script.py:1739:            orca.setLocusOfFocus(None, context.obj, False)
scripts/toolkits/Gecko/script.py:2029:            orca.setLocusOfFocus(event, event.source, False)
scripts/toolkits/Gecko/script.py:2118:                    orca.setLocusOfFocus(event, event.source, False)
scripts/toolkits/Gecko/script.py:2204:            orca.setLocusOfFocus(event, event.source, False)
scripts/toolkits/Gecko/script.py:2266:                    orca.setLocusOfFocus(event, obj)
scripts/toolkits/Gecko/script.py:2318:                orca.setLocusOfFocus(event, child)
scripts/toolkits/Gecko/script.py:2472:                    orca.setLocusOfFocus(event, obj, False)
scripts/toolkits/Gecko/script.py:2567:            orca.setLocusOfFocus(event, obj, False)
scripts/toolkits/Gecko/script.py:6489:            orca.setLocusOfFocus(None, obj, False)
scripts/toolkits/Gecko/script.py:6903:            orca.setLocusOfFocus(None, comboBox)
Comment 2 Rich Burridge 2008-05-22 17:15:14 UTC
Created attachment 111353 [details] [review]
Revision #1 - debug version.

We discussed this on IRC this morning. Will suggested a different
approach (see comment n code patch).

Here's a version (with debug messages included) for Joanie to try.
To be tidied up (and pylinted, reg. tested etc.) later.
Comment 3 Rich Burridge 2008-05-22 17:42:54 UTC
Created attachment 111354 [details] [review]
Revision #2.

This seems to work nicely. So much so, that I've moved all that
code I'd previous added to the gaim/pidgin script.

Please test.

When I pylint orc.py I get:

C0301:185: Line too long (89/80)
C0301:186: Line too long (96/80)
C0301:187: Line too long (103/80)

Maybe I'm tireder then I think, but I can't see what I'm doing wrong here.
Please let me know what I missed.

Thanks.
Comment 4 Joanmarie Diggs (IRC: joanie) 2008-05-22 18:19:16 UTC
Rich I'm not seeing the pylint errors on my Hardy box either, so I'm not sure.

As for ripping out the Pidgin changes.... If you do that, your fix for this bug will solve the problem in bug 525644 *if the app with focus is not pidgin*.  

However, if I'm in a chat window (i.e. the active script is still pidgin), changes that take place in the buddy list cause us to display "cell."
Comment 5 Rich Burridge 2008-05-22 18:39:26 UTC
Created attachment 111356 [details] [review]
Revision #3.

Okay. Thanks Joanie. Then here is an adjusted version.
Comment 6 Willie Walker 2008-05-22 19:07:50 UTC
(In reply to comment #5)
> Created an attachment (id=111356) [edit]
> Revision #3.
> 
> Okay. Thanks Joanie. Then here is an adjusted version.
> 

In the patch, please make sure that activeScript and event are not None before using them.  If I recall, we used to run into these situations.  Ask me where, and I cannot recall exactly, but I have a voice in my head telling me to be concerned.
Comment 7 Rich Burridge 2008-05-22 19:47:46 UTC
Created attachment 111361 [details] [review]
Revision #4.

Okay. Thanks Joanie. Then here is an adjusted version.
Comment 8 Mike Pedersen 2008-05-23 17:57:26 UTC
I've been running with this patch all morning.  I haven't seen any bogus messages that I didn't expect on the braille display.  I think this one is good.  
Comment 9 Rich Burridge 2008-05-23 19:52:12 UTC
Thanks Mike. Patch committed to SVN trunk. Moving to "[pending]".
Comment 10 Joanmarie Diggs (IRC: joanie) 2008-05-24 04:09:23 UTC
Rich, I'm seeing the occasional traceback.  I'm working on another elusive bug atm so for now, I'm just pasting it.  I'll look at it later if you don't beat me to it. ;-)

Traceback (most recent call last):
  • File "/usr/lib/python2.5/site-packages/orca/focus_tracking_presenter.py", line 630 in _processObjectEvent
    s.processObjectEvent(event)
  • File "/usr/lib/python2.5/site-packages/orca/script.py", line 319 in processObjectEvent
    self.listeners[key](event)
  • File "/usr/lib/python2.5/site-packages/orca/default.py", line 3969 in onWindowActivated
    orca.setLocusOfFocus(event, event.source)
  • File "/usr/lib/python2.5/site-packages/orca/orca.py", line 185 in setLocusOfFocus
    if currentApp != event.host_application.name and \
AttributeError: 'NoneType' object has no attribute 'name'

Comment 11 Rich Burridge 2008-05-24 16:59:24 UTC
Created attachment 111466 [details] [review]
Fix traceback.

Okay. Thanks Joanie. Here's a small tweak to hopefully prevent
that. Patch committed to SVN trunk.
Comment 12 Rich Burridge 2008-05-25 16:50:55 UTC
Patch removed. As Daniel Dalton reports on the Orca mailing list,
this breaks gnome-screensaver. Password is no longer announced.
Comment 13 Rich Burridge 2008-05-27 15:43:52 UTC
Here's what's happening with gnome-screensaver in setLocusOfFocus()
with the previous patch:

vvvvv PROCESS OBJECT EVENT object:state-changed:focused vvvvv
OBJECT EVENT: object:state-changed:focused             detail=(1,0)
    app.name='gnome-screensaver-dialog' name='None' role='password text' state='editable enabled focusable focused sensitive showing single line visible' relations='labelled by'
Finding top-level object for source.name=
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=
--> obj.name=
onStateChanged: could not get frame of focused item
XXX: event.host_application.name:  gnome-screensaver-dialog
XXX: event.source.getApplication().name:  gnome-screensaver-dialog
XXX: currentApp:  gnome-screensaver
XXX: just RETURNING.
^^^^^ PROCESS OBJECT EVENT object:state-changed:focused ^^^^^

As you can see, this is for an object with a role of "password text".
How should we best handle this? Is it simple enough to also add a check to
make sure the role isn't pyatspi.ROLE_PASSWORD_TEXT?
Comment 14 Willie Walker 2008-05-29 15:29:09 UTC
(In reply to comment #13)
...
> onStateChanged: could not get frame of focused item
...

Looking at the code, this seems to be the result of default.py:getTopLevel failing to find a top level window.  In the case of the screen saver, I'm guessing there is no top level window. 

If this is the case, I wonder if we didn't get any window activation events.  If so it may be possible we never activated the script for the screen saver.
Comment 15 Willie Walker 2008-05-29 15:56:50 UTC
(In reply to comment #14)
> (In reply to comment #13)
> ...
> > onStateChanged: could not get frame of focused item
> ...
> 
> Looking at the code, this seems to be the result of default.py:getTopLevel
> failing to find a top level window.  In the case of the screen saver, I'm
> guessing there is no top level window. 
> 
> If this is the case, I wonder if we didn't get any window activation events. 
> If so it may be possible we never activated the script for the screen saver.

In looking at the debug log from bug #529655, it looks like we are getting window activated events when the screen saver appears.
Comment 16 Rich Burridge 2008-05-29 17:00:20 UTC
Created attachment 111748 [details]
Another Orca debug log.

Here's a little more information. I'm using the following patch+debug.

Index: src/orca/orca.py
===================================================================
--- src/orca/orca.py	(revision 3937)
+++ src/orca/orca.py	(working copy)
@@ -180,9 +180,26 @@
     - notifyPresentationManager: if True, propagate this event
     """
 
+    print "XXX: setLocusOfFocus called."
     if obj == orca_state.locusOfFocus:
         return
 
+    print "XXX: orca_state.activeScript.app.name: ", orca_state.activeScript.app.name
+    print "XXX: event.host_application.name: ", event.host_application.name
+    print "XXX: event.source.getApplication().name: ", event.source.getApplication().name
+
+    # If this event is not for the currently active script, then just return.
+    #
+    if event and event.source and \
+       event.host_application and orca_state.activeScript:
+        print "XXX: Comparing currentApp name."
+        currentApp = orca_state.activeScript.app.name
+        if currentApp != event.host_application.name and \
+           currentApp != event.source.getApplication().name:
+            print "XXX: RETURNING."
+            return
+
+    print "XXX: setLocusOfFocus continuing."
     oldLocusOfFocus = orca_state.locusOfFocus
     try:
         # Just to see if we have a valid object.
Index: src/orca/script.py
===================================================================
--- src/orca/script.py	(revision 3937)
+++ src/orca/script.py	(working copy)
@@ -500,6 +500,7 @@
 
     def activate(self):
         """Called when this script is activated."""
+        print "XXX: script: activate called. self.name: ", self.name
         pass
 
     def deactivate(self):

Interesting messages start with "XXX".

I started Orca, then used the little green running man (with my mouse
to start the lock screen). After it had all gone dark (speaking "window"
at line 964), I pressed the left shift key (line 982) to bring up the 
password dialog.

Note that we get a traceback at line 1012:

Traceback (most recent call last):
  • File "/usr/lib/python2.5/site-packages/orca/focus_tracking_presenter.py", line 630 in _processObjectEvent
    s.processObjectEvent(event)
  • File "/usr/lib/python2.5/site-packages/orca/script.py", line 319 in processObjectEvent
    self.listeners[key](event)
  • File "/usr/lib/python2.5/site-packages/orca/scripts/apps/gnome-screensaver-dialog.py", line 55 in onStateChanged
    default.Script.onStateChanged(self, event)
  • File "/usr/lib/python2.5/site-packages/orca/default.py", line 3675 in onStateChanged
    keyString = orca_state.lastNonModifierKeyEvent.event_string
AttributeError: 'NoneType' object has no attribute 'event_string'

We certainly need to add some bullet-proofing there, but i don't think 
that's the real problem here.

The interesting thing is right at the end (line 1260) for the "focus:"
event:

vvvvv PROCESS OBJECT EVENT focus: vvvvv
OBJECT EVENT: focus:                                   detail=(0,0)
    app.name='gnome-screensaver-dialog' name='None' role='password text' state='editable enabled focusable focused sensitive showing single line visible' relations='labelled by'
XXX: setLocusOfFocus called.
XXX: orca_state.activeScript.app.name:  gnome-screensaver
XXX: event.host_application.name:  gnome-screensaver-dialog
XXX: event.source.getApplication().name:  gnome-screensaver-dialog
XXX: Comparing currentApp name.
XXX: RETURNING.
^^^^^ PROCESS OBJECT EVENT focus: ^^^^^

Our active script name is different from the name being returned by
event.host_application.name or event.source.getApplication().name.
Comment 17 Willie Walker 2008-05-30 13:09:32 UTC
(In reply to comment #16)
> Note that we get a traceback at line 1012:

Filed as bug #535747.

+        print "XXX: Comparing currentApp name."
+        currentApp = orca_state.activeScript.app.name
+        if currentApp != event.host_application.name and \
+           currentApp != event.source.getApplication().name:

Not directly related to this particular problem with screen saver, but related to this bug, I'm wondering if we should actually be comparing the application objects and not the names of those objects.  There are two main reasons: I believe the application objects should be sufficient for the compare and referencing the name has the potential of forcing a round trip.

> The interesting thing is right at the end (line 1260) for the "focus:"
> event:
> 
> vvvvv PROCESS OBJECT EVENT focus: vvvvv
> OBJECT EVENT: focus:                                   detail=(0,0)
>     app.name='gnome-screensaver-dialog' name='None' role='password text'
> state='editable enabled focusable focused sensitive showing single line
> visible' relations='labelled by'
> XXX: setLocusOfFocus called.
> XXX: orca_state.activeScript.app.name:  gnome-screensaver
> XXX: event.host_application.name:  gnome-screensaver-dialog
> XXX: event.source.getApplication().name:  gnome-screensaver-dialog
> XXX: Comparing currentApp name.
> XXX: RETURNING.
> ^^^^^ PROCESS OBJECT EVENT focus: ^^^^^
> 
> Our active script name is different from the name being returned by
> event.host_application.name or event.source.getApplication().name.

In digging through the debug logs, I see that these are probably separate objects as indicated by the "NEW SCRIPT" output:

NEW SCRIPT: gnome-screensaver (module=orca.scripts.toolkits.GAIL)

NEW SCRIPT: gnome-screensaver-dialog (module=orca.scripts.apps.gnome-screensaver-dialog)

As such, it does seem as though the script that needs to be the active script never becomes the active script.  I wonder if this is due to some different behavior of the screen saver screen (e.g., no window manager running to cause a window activate event for the gnome-screensaver-dialog)?

Further down in the debug log, I see this event:

OBJECT EVENT: object:state-changed:showing             detail=(1,0)
    app.name='gnome-screensaver-dialog' name='None' role='panel' state='active enabled modal resizable sensitive showing visible' relations='

So....I wonder if we might consider a couple different options:

1) Instead of the proposed "is this a password field" check in the setLocusOfFocus mod for this bug, we do a "is the app for this event a panel with the modal state set" check.

or

2) In focus_tracking_presenter.py, we include an additional check for object:state-changed:showing events of modal panels when determining if we need to set the active script.  This would be in the same location as the check for window:activate.

I suspect #2 is likely to be more correct thing to do, but I'm not sure how difficult it would be to implement or what the performance impact would be.
Comment 18 Rich Burridge 2008-06-02 16:33:19 UTC
Created attachment 111968 [details] [review]
Revision #5.

Thanks Will.

This seems to do the right thing now. Checks no longer use
.name comparisons, and the additional code in _processObjectEvent()
now catches the "showing" event for the modal panel for
gnome-screensaver-dialog and set the active script accordingly.

I've left the debug messages in for now, so that if any problems
are found, then supplying a debug.out will contain that useful
information.

Please test.
Comment 19 Mike Pedersen 2008-06-02 19:38:58 UTC
This patch seems to break navigating by line in both firefox 3 and thunderbird trunk.  
to reproduce:
Open firefox and attempt to down arrow.  Notice that you get no speech or braille.  
Notice also that attempting to install a clean trunk does not correct the problem.  In order to get back the desired functionality one must also remove orca from site-packages before reinstalling trunk.  
Comment 20 Rich Burridge 2008-06-02 20:10:18 UTC
Hi Mike. Would you be good enough to turn debugging on, and attach a 
debug.out of your results?

Thanks!
Comment 21 Joanmarie Diggs (IRC: joanie) 2008-06-02 20:54:21 UTC
Created attachment 111998 [details]
debug output from trying line navigation in FF

Here you go.
Comment 22 Mike Pedersen 2008-06-02 21:00:01 UTC
Created attachment 112000 [details]
output exhibiting the problem 

requested output
Comment 23 Rich Burridge 2008-06-02 22:19:12 UTC
Created attachment 112008 [details] [review]
Revision #6.

Thanks Joanie and Mike.

Well it was actually the debug messages that I'd added that were causing
the problem. How ironic is that!

Here's a version of the patch with them removed.

I seem to be able to nicely down arrow in FF3 beta5 now.
Comment 24 Mike Pedersen 2008-06-03 18:43:35 UTC
This one now seems to be working nicely.
Comment 25 Rich Burridge 2008-06-03 19:18:02 UTC
Patch (revision #6) committed to SVN trunk. Moving to "[pending]".