497839 - Synthesizing events fail if browser window hasn't the focus

Status: RESOLVED WORKSFORME

(Core :: XUL, defect)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.2a1pre) Gecko/20090611 Minefield/3.6a1pre ID:20090611030757

Since the patch for refactoring the focus handling (bug 178324) has been checked in, it is not possible to send any synthesized events to the browser window. They aren't respected anymore.

I noticed this problem while using Mozmill from the command line and running a couple of our existing tests which make use of the synthesizeKey function from EventUtils.js. In such a case the browser is started in the background and is not focused or even the front-most window. Sending any key events will fail now. You have to explicitly focus the browser window.

I wonder if other mochitests are affected too but AFAIR the browser gets opened as front-most application.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I hope there should be an easier way to reproduce this bug. Would be nice to have a simple webpage which fires such an event after a timeout while the user can focus another application?

Comment 2

[Martijn Wargers (dead)]

Attachment: testcase (uses enhanced privileges)

This adds some text every 500ms when the keydown event is fired correctly. It indeed doesn't work when the window isn't focused, but afaik, this never worked when the window wasn't focused.

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Thanks Martijn! That works perfect on 1.9.1. Even with the browser in the background key events are getting processed. While running this testcase with a recent version of Minefield (after 06/11) nothing is added to the list.

Comment 4

[Martijn Wargers (dead)]

It doesn't work for me on a 1.9.1 build, either. It doesn't work when the url bar has the initial focus.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Oh, looks like we behave differently on Windows and OS X?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Yes, focus used to behave quite inconsistently across platforms...

Comment 7

[Mikeal Rogers [:mikeal] (mrogers@)]

I think a possible fix, or at least a good idea, would be to call controller.window.focus() before any controller action and before we do anything to elem object we call elem._document.defaultView.focus().

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Mikeal, that's a workaround for Mozmill but the problem is somewhere deeper in the focus refactoring.

Neil, would you have time to check what's wrong since your patch has been landed?

Comment 9

[Neil Deakin]

Incoming key events fire at whatever is focused. If nothing is focused, the key event does not get fired at anything.

I could add some means of firing key events at unfocused windows, but it would seem wrong to allow tests to send keys in a way that doesn't represent what could actually happen as it would not be as accurate a test.

Comment 10

[Mikeal Rogers [:mikeal] (mrogers@)]

That comment was incredibly old, let me clarify.

The way EventUtils simulates events requires the window in question to be in focus. There are others ways to simulate user events, like the way we were doing this in mozmill before we moved to EventUtils, but there are other tradeoffs and we've found that EventUtils get's us closer to 100% testability than any other method even if it has some usability tradeoffs like this focus issue.

I'm totally open to alternative event simulation methods but I don't think this is necessarily a product bug, just an implementation detail.

Comment 11

[Karl Tomlinson (:karlt)]

In bug 178324, the GTK version of nsIWidget::SetFocus() was changed so that when called on a toplevel window it always marks the toplevel as active (having toplevel focus) even though it is not yet active.  It seems that this is only done to make tests (that, rightly or wrongly, do not wait for focus) pass.

http://hg.mozilla.org/mozilla-central/annotate/74959aad851c/widget/src/gtk2/nsWindow.cpp#l1396

A number of tests are calling focus() on the window in the expectation that this will make the window respond to synthesizeKey(), which currently happens to be a functional workaround because of this code.  But I fear that this code may sometimes end up marking a toplevel window as active when it is not going to become active, so I'd like to remove it.

Either we need a way of sending key events to inactive windows, or tests need to wait for focus notification before using synthesizeKey.  More discussion in bug 498143 comment 44 and 45.

Being able run tests while a different application has toplevel focus sounds appealing to me.  I guess we may want some tests that check that the correct window becomes active, but that can't really be guaranteed on X11 as the window manager makes the final decision on which window is active.  If tests wait for focus on X11, there may be some situations where they never get it.

Comment 12

[Karl Tomlinson (:karlt)]

(In reply to comment #11)
> Either we need a way of sending key events to inactive windows, or tests need
> to wait for focus notification before using synthesizeKey.

Perhaps better than waiting for focus notification (for tests that are not trying to test which window is active):

Would it be an option for tests to tell the focus manager to behave as if a particular window is active?

Comment 13

[Neil Deakin]

I've considered the idea of a debugging/testing mode in which windows are never brought to the front, and are opened behind others. This would have mainly been used to allow one to run the tests in the background, which is very desirable and a requested feature. However, it would need a lot of work and coordination by the native widget code, the focus system and window managing code within Mozilla.

window.focus is supposed to have the effect of making the window active, but I guess the problem is sometimes it's asynchronous.

Comment 14

[Karl Tomlinson (:karlt)]

I filed bug 506175 on changing tests to wait for onfocus after window.focus().

Comment 15

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to comment #13)
> brought to the front, and are opened behind others. This would have mainly been
> used to allow one to run the tests in the background, which is very desirable
> and a requested feature. However, it would need a lot of work and coordination

Neil, do we already have a bug for this feature request?

Comment 16

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

I posted a patch to bug 519913. The key events and IME related events should be go to the last focused content in the window even if the window is deactivated. This issue broke some software keyboard's accessibility.

Comment 17

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

By the bug 532422's check-in, now nsIDOMWindowUtils::sendKeyEvent should work fine when all Fx's windows are deactive. However, when a Fx window is active, but you need to send the key events to another (deactive) window, it still doesn't work fine. It's going to be fixed in bug 519913 only on trunk.

Can we mark as FIXED this?

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Definitely not. Most of the Mozmill tests still fail when they get run in the background and I cannot say anything about Mochitests. I haven't time to check if the remaining failures are only synthesized mouse events or if still key events are failing.

Comment 19

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

Bug 519913 has been fixed, please check it.

Comment 20

[Andrew Sutherland [:asuth] (he/him)]

Speaking for Thunderbird, the bug 519913 landing the other day fixed the mozmill linux focus-related problems we were having relating to this bug.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Mozmill Test

So I have tested a todays Minefield build with our Mozmill tests for Firefox and it's definitely better as before but synthesizing a keypress like "VK_RETURN" doesn't still work. I have attached a Mozmill test which demonstrates the problem.

To run it do:
1. Install Mozmill (https://addons.mozilla.org/de/firefox/addon/9018)
2. Open Minefield and the Mozmill IDE
3. Load the test file and run the editor
4. Switch to another application

As you will be able to see, the location bar gets focused and 'Mozilla' entered but the I'm feeling lucky search is not triggered.

Comment 22

[Mark Banner (:standard8)]

(In reply to comment #20)
> Speaking for Thunderbird, the bug 519913 landing the other day fixed the
> mozmill linux focus-related problems we were having relating to this bug.

Andrew, can you take another look at the Linux builders, as the 32 bit ones are up and running again, and seem worse...

Comment 24

[cmtalbert]

(In reply to comment #13)
> I've considered the idea of a debugging/testing mode in which windows are never
> brought to the front, and are opened behind others. This would have mainly been
> used to allow one to run the tests in the background, which is very desirable
> and a requested feature. However, it would need a lot of work and coordination
> by the native widget code, the focus system and window managing code within
> Mozilla.
> 
> window.focus is supposed to have the effect of making the window active, but I
> guess the problem is sometimes it's asynchronous.

Neil, Karlt, I've got a contributor interested in working on this problem.  He's cc'd on this bug eran...at gmail, but he needs a pointer of where to start looking at this issue.  Do you have an mxr link or a place to put a breakpoint to start understanding what would need to change w.r.t. focus handling to make this a reality?  Ideally, a list of steps that you know would have to be done would be great.  Something like:
* refactor nsIFoo to handle focus better
* ensure nsIFoo integrates with native widgets 
* etc

I can help him decipher a list like that into actions, but without a deep understanding of this code, I'm afraid I'd start him out in the wrong area if I tried to compile such a thing.

Thanks for the help.

Comment 25

[Neil Deakin]

The approach you would need to take would depend on how you want this to be done. 

Do you want the tests to be able to run separately in the background without allowing real user interaction or not? An issue is that if real user interactions are allowed as well, it becomes notably more difficult as you would need to distinguish between real user interaction and input coming from tests.  

For example, the tests will often open new windows. You would need to distinguish between those coming from tests and those that the user opened. Modal dialogs could present other challenges.

Significantly easier though, is to assume that, while testing, the entire browser instance runs in the background. A global flag would be used to prevent windows from being raised and the focus system would emulate any needed behaviour. Any real user interaction would be allowed, but would probably cause tests to fail or the focus system to get confused.

Comment 26

[Eran Mes]

I am definitely aiming for the first option - not allowing user interactions during the test. In order to achieve that effect now, Firefox instances are created inside their own X server (either Xvfb or Xvnc) and a library is loaded to prevent the Focus in/out events from arriving to the different Firefox windows (unless the WebDriver test explicitly desires switching to another window).

The library is described here:
http://code.google.com/p/selenium/wiki/NativeEventsOnLinux#Over-coming_the_focus_problem

The global flag you described is just what's required. How do we go about adding that flag?

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Neil, could you please check Erans comment from beginning of November? Would be great to get feedback from you.

Comment 28

[Neil Deakin]

> I am definitely aiming for the first option
>   - not allowing user interactions during the test.

> The global flag you described is just what's required.

You contradict yourself here, but I'll describe the second option. I don't know how to do the first.

The basic change is that you would need have some state in which nsFocusManager::RaiseWindow skips calling nsIWidget::SetFocus and instead just calls WindowLowered/WindowRaised. Then, both WindowLowered/WindowRaised might need to be changed to ignore real attempts by the user to lower and raise the window.

Note that whatever you do, you won't be able to emulate an actual test, as some behaviour can vary depending on what the operating system does. We also don't have control over what plugins might do.

Comment 29

[Neil Deakin]

Is this still a problem, or is the focus test mode sufficient here?

Comment 30

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Neil Deakin from comment #29)
> Is this still a problem, or is the focus test mode sufficient here?

At least from our end for Mozmill it's fine for now. Lets also get feedback from David Burns and Jonathan Griffin for Marionette.

Comment 31

[Jonathan Griffin (:jgriffin)]

I'll defer to dburns here.

Comment 32

[David Burns :automatedtester]

I believe it has been sufficiently fixed