457672 - window blur event is not fired when opening a new tab

Status: VERIFIED FIXED

Description

[Michael Tamm]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9.0.3) Gecko/2008092417 Firefox/3.0.3

The window blur event is not fired, when a new tab is opened (and sometimes - not always - also when you switch to another tab).

This behavior can be verified with the provided test page at http://www.michaeltamm.de/window_blur_test.html

The blur event will be fired, if another window is focused.

Reproducible: Always

Steps to Reproduce:
1. Go to http://www.michaeltamm.de/window_blur_test.html
2. Open a new tab.
3. Switch back to the tab displaying http://www.michaeltamm.de/window_blur_test.html
Actual Results:  
There is no paragraph with the text 'lost focus at ...'

Expected Results:  
The page should have two more paragraphs (added via JavaScript):
'lost focus at ...' followed by 'got focus at ...'

Comment 1

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b1pre) Gecko/20080928084201 Minefield/3.1b1pre

Confirmed with latest trunk on Windows XP.

Comment 2

[Ria Klaassen (not reading all bugmail)]

This regressed on 3 July 2007 probably from Bug 238987.

Comment 3

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Created attachment 340949 [details]
testcase

Note, blur and focus don't bubble per DOM Events spec.
Bug 238987 just revealed a bug that we don't always dispatch the blur event to
|window|, only to |document|. But that bug was there already in 1.8 (FF2).

Comment 4

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

For some reason this wasn't implemented in bug 27936 (yes, that is ~8 years ago).
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/content/events/src/nsEventStateManager.cpp&rev=1.439&mark=436#416

Comment 5

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Created attachment 340957 [details] [diff] [review]
possible patch

Changing focus/blur handling is always regression-risky.
I think the patch does what I'd like to see happening, but
something may rely on the old behavior.
Needs lots of testing.

Comment 6

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Created attachment 340993 [details] [diff] [review]
passes mochitest, chrome and browser-chrome

I still need to write testcases for this

Comment 7

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Comment on attachment 340993 [details] [diff] [review]
passes mochitest, chrome and browser-chrome

Doesn't regress bug 200735, bug 203117, bug 194104 nor bug 201996.
(bryner fixed those bugs when modifying this code.)
Still need to write mochitests

Comment 8

[Johnny Stenback (:jst)]

Smaug, how risky do you think this is? Do you think you could get the tests written in time for 1.9.1? If so, it'd be awesome to get this in if we feel comfortable about this patch...

Comment 9

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Changing focus handling is always a bit risky.
The current code is clearly wrong, there is even the XXX comment.
I'll try to write the mochitest ASAP.

Comment 10

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Created attachment 350001 [details] [diff] [review]
with mochitesttest

Sorry, my asap wasn't really asap.

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 350001 [details] [diff] [review]
with mochitesttest

Looks good to me. r+sr=jst

Comment 12

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Comment on attachment 350001 [details] [diff] [review]
with mochitesttest

If we want this for 1.9.1, this should land asap to get testing.

Comment 13

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 350001 [details] [diff] [review]
with mochitesttest

a191=beltzner

Comment 14

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

Fixed on trunk and 1.9.1

Comment 15

[Olli Pettay [:smaug] (r- if the bug doesn't explain what the change(s) are about.)]

I disabled the test on 1.9.1 for now because SM is also running mochitest
and the test relies on FF's new-window handling.

Comment 16

[Serge Gautherie (:sgautherie)]

(In reply to comment #15)

See bug 457672.

Comment 17

[Serge Gautherie (:sgautherie)]

(In reply to comment #16)
> See bug 457672.

Argh: bug 468261 !

Comment 18

[Daniel Veditz [:dveditz]]

will look at a patch approval if you feel strongly about getting this into a 3.0.x release (and think the patch is safe enough), but not "wanted" in the sense of "release-drivers need to make sure this gets in sometime".

Comment 19

[Aakash Desai [:aakashd]]

verified FIXED on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090331 Minefield/3.6a1pre

and

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090401 Shiretoko/3.5b4pre