Synthesizing events fail if browser window hasn't the focus

RESOLVED WORKSFORME

Status

()

Core
XUL
--
major
RESOLVED WORKSFORME
9 years ago
4 years ago

People

(Reporter: whimboo, Assigned: Neil Deakin)

Tracking

({regression, testcase})

Trunk
regression, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.2 -
wanted1.9.2 -

Firefox Tracking Flags

(blocking2.0 -)

Details

(Whiteboard: [mozmill])

Attachments

(2 attachments)

(Reporter)

Description

9 years ago
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.
Flags: blocking1.9.2?
(Reporter)

Comment 1

9 years ago
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?
Keywords: testcase-wanted
Created attachment 382943 [details]
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.
(Reporter)

Comment 3

9 years ago
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.
Keywords: testcase-wanted → testcase
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.
(Reporter)

Comment 5

9 years ago
Oh, looks like we behave differently on Windows and OS X?
Yes, focus used to behave quite inconsistently across platforms...
Assignee: nobody → enndeakin
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().
Blocks: 504586
(Reporter)

Comment 8

8 years ago
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?
(Assignee)

Comment 9

8 years ago
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.
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.
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.
Blocks: 499438, 498805, 299673
(Reporter)

Updated

8 years ago
Blocks: 503480
(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?
(Assignee)

Comment 13

8 years ago
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.
Flags: wanted1.9.2-
Flags: blocking1.9.2?
Flags: blocking1.9.2-
I filed bug 506175 on changing tests to wait for onfocus after window.focus().
Blocks: 506175
No longer blocks: 299673, 498805, 499438
(Reporter)

Comment 15

8 years ago
(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?

Updated

8 years ago
Blocks: 498704
Depends on: 519913
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.

Updated

8 years ago
Blocks: 532422
(Reporter)

Updated

8 years ago
blocking2.0: --- → ?
Whiteboard: [mozmill]
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?
(Reporter)

Comment 18

8 years ago
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.
blocking2.0: ? → -
Bug 519913 has been fixed, please check it.
Speaking for Thunderbird, the bug 519913 landing the other day fixed the mozmill linux focus-related problems we were having relating to this bug.
(Reporter)

Comment 21

7 years ago
Created attachment 451743 [details]
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.
(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...
(Assignee)

Updated

7 years ago
Duplicate of this bug: 590315

Comment 24

7 years ago
(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.
(Assignee)

Comment 25

7 years ago
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

7 years ago
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?
(Reporter)

Comment 27

7 years ago
Neil, could you please check Erans comment from beginning of November? Would be great to get feedback from you.
(Assignee)

Comment 28

7 years ago
> 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.
No longer blocks: 504586
(Assignee)

Comment 29

4 years ago
Is this still a problem, or is the focus test mode sufficient here?
(Reporter)

Comment 30

4 years ago
(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.
Flags: needinfo?(jgriffin)
Flags: needinfo?(dburns)
I'll defer to dburns here.
Flags: needinfo?(jgriffin)
I believe it has been sufficiently fixed
Flags: needinfo?(dburns)
(Assignee)

Updated

4 years ago
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Resolution: FIXED → WORKSFORME
You need to log in before you can comment on or make changes to this bug.