Firefox not firing focus event on application switching

RESOLVED FIXED

Status

()

Core
Disability Access APIs
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Scott Haeger, Assigned: Aaron Leventhal)

Tracking

({access})

Trunk
x86
Linux
access
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

11 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070220 Minefield/3.0a3pre
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070220 Minefield/3.0a3pre

Firefox is not firing a focus event on application switching when the focused page does not have any controls in focus.   Firefox should indicate its focus when it becomes the foreground window again.

Reproducible: Always

Steps to Reproduce:
1.  Bring up a simple html page with only text.
2.  Switch to a different application.
3.  Switch back to Firefox and observe that there are no focus events.
Actual Results:  
No, focus events seen.

Expected Results:  
Firefox should fire a focus event indicating what is in focus.
(Assignee)

Updated

11 years ago
Assignee: nobody → aaronleventhal
Blocks: 368881
Component: Disability Access → Disability Access APIs
Keywords: access
Product: Firefox → Core
QA Contact: disability.access → accessibility-apis
Version: unspecified → Trunk

Comment 1

11 years ago
fixed perhaps by bug 261074?  (just fixed a few days ago)
If not, you might enlist someone's help from that bug.
Shouldn't this be OS=ALL?
(Reporter)

Comment 2

11 years ago
On Linux (not tested under Windows), during application switching, I am not getting a focus event unless there is a control with focus WITHIN the document frame.  In this case a focus(document frame) followed by a focus(control) are triggered.  All other cases, including when focus is within the chrome, there are no focus events.
(Assignee)

Comment 3

11 years ago
Is it a regression? I suspect the bug mentioned in comment 1 actually may have broken this. Will someone download a build from the 19th and check? 
See http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/

Under Windows we suppress focus events for the top level window because MSAA fires focus events for the top level window automatically. However that suppression is #ifdef'd and is not built for our ATK support.

On Linux we fire the event here:
http://lxr.mozilla.org/seamonkey/source/accessible/src/atk/nsRootAccessibleWrap.cpp#180

(Reporter)

Comment 4

11 years ago
Same behavior is seen on Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a3pre) Gecko/20070219 Minefield/3.0a3pre
(Assignee)

Updated

11 years ago
Assignee: aaronleventhal → ginn.chen

Comment 5

11 years ago
Firefox did call atk_focus_tracker_notify().
But atkutil.c prevents an application emit focus event again for previous focused object.

It's consistent with gnome application's behavior.

Willie and Peter, what's your opinion? 
If it's not good, maybe we want to delete the filter in atkutil.c
File a bug in bugzilla.gnome.org then.

Firefox 2.0 seems work, just because it had another serious bug.
It emits focus event for window object before "window:activate" event.
It may cause "window:activate" event discarded by at-spi registryd.
See Bug 341463.
But since it emits focus event for window, the focus event of the focused widget in the window gets a pass.

Not a bug for FIrefox for now.
Feel free to reopen if you disagree.
Status: UNCONFIRMED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → INVALID

Comment 6

11 years ago
Hm. It seems focus events in Firefox are inconsistent then:

1) If focus is previously on a chrome widget, when you switch back from another app, you do not get a focus event.
2) If focus is on a widget within the document frame, when you switch back from another app, you *do* get a focus event, on the document frame, on the widget in the frame, and sometimes on the top level window frame too.

The behavior in FF should at least be consistent. Either never fire focus or always fire it.

You're right that you do not get a focus: event when you return to an application in gtk. However, you do get an object:state-changed:focus on the last focused control indicating where focus was restored.
Status: RESOLVED → UNCONFIRMED
Resolution: INVALID → ---
(Assignee)

Comment 7

11 years ago
> You're right that you do not get a focus: event when you return to an
> application in gtk. However, you do get an object:state-changed:focus on the
> last focused control indicating where focus was restored.

So would you be alright with either a focus or state-changed event to indicate where focus is restored?

I guess you need some event, and right now you get none.

Comment 8

11 years ago
OK, so the fix could be
1) kill the code in atkutil.c that preventing us firing focus: event, it will affect all gnome-app. too.
2) fire state-changed:focus event when Firefox fires focus: event

Peter, do you like 2) or 1)+2) ?
(Assignee)

Comment 9

11 years ago
Is that atkutil.c check there to preven duplicate focus events? E.g. 2 focus events on the same checkbox? If so we might want to fix the check to be smarter instead of removing it.

Comment 10

11 years ago
We should ask Will Walker for his opinion.

Going back to the "old way" (circa GNOME 2.14) of receiving focus: events when
switching windows sounds tantalizing. However, I don't know what side effects
that might have and I don't want to destabilize the entire desktop. 

We can definitely handle the state-changed:focus events fired in addition to
focus: since that's what most gtk apps do. So I guess my conservative vote is
for #2.
(Assignee)

Comment 11

11 years ago
Just curious, what does the cvs comment / bug for that change in atkutil.c say?

Comment 12

11 years ago
When I use at-poke, GEdit and GAIM to experiment with our reference implementation of GTK+/gail, I see that we get window activation and deactivation events when Alt+Tab'ing between windows.  I don't see any "focus:" events, but I do see "state-changed:focus" events for the appropriate object.

Getting some sort of focus event when a window is activated is indeed nice because it allows us to more easily announce the window you just moved to as well as where you just moved.  Without the focus event, doing this is a little more difficult.

As for doing just one of "focus:" or "state-changed:focus", my opinion is that when you get one, you should get the other.  But, that's not the current way life is, and we just deal with it.  We maintain state to make sure we work in the face of receiving focus: and state-changed:focus events from the same object at the same time.

So...I'm guessing that some version of #2 is the thing to do.  Thanks!  :-)

Comment 13

11 years ago
Sounds like we agree on #2.

The lack of any focus indicator is also noticible when using Ctrl-Tab to switch between Firefox tabs when the focus is on the URL bar. No focus event. In this case, it's more difficult to track where the focus was for N tabs versus 1 application window.

Comment 14

11 years ago
(In reply to comment #13)
> Sounds like we agree on #2.
> 
> The lack of any focus indicator is also noticible when using Ctrl-Tab to switch
> between Firefox tabs when the focus is on the URL bar. No focus event. In this
> case, it's more difficult to track where the focus was for N tabs versus 1
> application window.

I think we'll generally be in agreement.  :-)
(Assignee)

Comment 15

11 years ago
Ginn, to get around the focus event swallowing in Gnome 2.14, our ATK code needs to fire a state change event for the focus in addition to focus events. If you can, just fire it when we're switching back to a window.

nsRootAccessible::FireCurrentFocusEvent() might be where we fire focus events when switching back to a window.

Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 16

11 years ago
The tab switch case should be filed as a separate bug.
(Assignee)

Comment 17

11 years ago
Created attachment 257746 [details] [diff] [review]
Wild untested guess. Ginn, this might provide some ideas. Can you take it from here?

Comment 18

11 years ago
Shouldn't we fire state-changed:focus:1 event for every focus event?
Not only for switch-back case, right?

When it loses focus, we should fire state-changed:focus:0.
(Assignee)

Comment 19

11 years ago
> Shouldn't we fire state-changed:focus:1 event for every focus event?
> Not only for switch-back case, right?
Technically we could, but no one needs it. They only need it for this case. If it doesn't affect performance go ahead and fire it every time. It's redundant except in this one case, so if affects performance don't fire it.

> When it loses focus, we should fire state-changed:focus:0.
No one needs that, from what I understand. It will just clog the event queue and affect performance for no good reason. If the Orca team asks for it you can add it. The LSR team doesn't need it.

Comment 20

11 years ago
Here's some relevant information from the AT-SPI IDL specification for Accessibility_Event:

focus:   an object has received keyboard focus.  This event has no type or subtype.

As such, it appears as though focus: is issued when a new object gets focus, and it is not issued when an object loses focus.  

As for state-changed:focus events, there doesn't seem to be a whole lot written up, but exploration with at-poke can be useful.  When I run at-poke and look for state-changed events on a GTK+ application while I tab around, I see state-changed:focus:0 and state-changed:focus:1 events occur in a logical and predictable fashion.   That is, state-changed:focus:0 are issued by an object every time focus is lost by the object and state-changed:focus:1 are issued by an object every time focus is gained.

Orca has some checks for the detail1 value of state-changed:focus events, but they are mostly to filter out detail1 values of 0.  :-P  I've had some thoughts for some time, however, that detail1 values of 0 can be useful in that we can let the user know that nothing has focus.  That kind of information is often desired by users, and we infer it using other means at the moment.

Finally, I think the right thing to do here is not assume the "LSR way" or the "Orca way" are the only way and let one or the other or both dictate what to do.  Making decisions like that will only lead to strife and problems down the road.  The right thing to do is make sure you follow the spec and the reference implementation of GTK+/gail where the spec falls down.  at-poke is a very useful tool for this.  In the event that there isn't a GTK+/gail precedent for something, then turning to the predominant assistive technologies for discussion makes a lot of sense.
(Assignee)

Comment 21

11 years ago
I agree with not doing things the "LSR way" or "Orca way" when everything makes sense.

What doesn't make sense to me is that we have events that are redundant. If focus and state change for focus mean exactly the same thing, why do we have them both. We have enough issues already.

We can do the state change every time, but I'm not really happy with the unnecessary redundancy.
(Assignee)

Updated

11 years ago
Attachment #257746 - Attachment is obsolete: true
(Assignee)

Comment 22

11 years ago
Created attachment 258419 [details] [diff] [review]
Fire state change event when focus state is enabled

Spun off bug to fire state change for clearing of focus state -- bug 373766.
Assignee: ginn.chen → aaronleventhal
Status: NEW → ASSIGNED
Attachment #258419 - Flags: review?
(Assignee)

Comment 23

11 years ago
Comment on attachment 258419 [details] [diff] [review]
Fire state change event when focus state is enabled

Ginn, will you test this for me as well?
Attachment #258419 - Flags: review? → review?(ginn.chen)

Comment 24

11 years ago
I'm a bit confused, maybe my brian doesn't work at midnight,

what's
+                stateData.enable = (stateData.state & (STATE_CHECKED | STATE_SELECTED)) != 0;
+                stateData.state = STATE_CHECKED;
for?

It will always overwritten by
+            stateData.enable = PR_TRUE;
+            stateData.state = STATE_FOCUSED;

right?
(Assignee)

Comment 25

11 years ago
You are right. Those 2 lines got into the patch by accident.
(Assignee)

Comment 26

11 years ago
Created attachment 258434 [details] [diff] [review]
Remoe extra lines we didn't need
Attachment #258419 - Attachment is obsolete: true
Attachment #258434 - Flags: review?(ginn.chen)
Attachment #258419 - Flags: review?(ginn.chen)

Updated

11 years ago
Attachment #258434 - Flags: review?(ginn.chen) → review+
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Blocks: 374115
You need to log in before you can comment on or make changes to this bug.