564413 - Test for bug 503399 makes reftest require babysitting

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Sometime recently reftest stopped running to completion in the background.  Specifically, layout/reftests/bugs/503399.html doesn't complete unless I click on the browser window.

It looks like the window.focus() call is ignored (because window flip is disabled in Firefox by default) and the focus call on the <p> doesn't fire the onfocus event until the window as a whole is focused.  So runTest doesn't run until the window as a whole is focused.

It's not clear to me that the focus() behavior here is correct (nominating for 1.9.3 blocking based on that), and I definitely think that not allowing reftest to run in the background is bad....

Comment 1

[Neil Deakin]

(In reply to comment #0)

> It's not clear to me that the focus() behavior here is correct (nominating for
> 1.9.3 blocking based on that)

What do you think is incorrect about it?

Comment 2

[Boris Zbarsky [:bzbarsky]]

Well it makes focus() calls on elements in unfocused windows/tabs not actually fire onfocus until some later time.  Is that something that will cause web compat issues?

Comment 3

[Neil Deakin]

(In reply to comment #2)
> Well it makes focus() calls on elements in unfocused windows/tabs not actually
> fire onfocus until some later time.  Is that something that will cause web
> compat issues?

Er, no. Focus events only fire on an element when it is focused and key events will target it.

Comment 4

[Mats Palmgren (inactive)]

Attachment: crash/reftest files that calls focus()

Fwiw, we have lot's of tests that probably does not test
what they are supposed to test without focus.

Point taken about 503399 which hangs without focus though.
I'll try to fix it so it completes at least, even if it
means it doesn't correctly test the bug.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Yeah, I can deal with a failure.  Usually I'm running reftests locally to see whether I hit certain asserts or to try to debug crash orange try server has pointed out....

Comment 6

[Mats Palmgren (inactive)]

Attachment: fix the test

This works for me locally (Fx debug on Linux), ie it fails without the fix
in bug 503399.  If I run it without window focus it times out after 1 sec
and runs the test anyway, which succeeds with or without the fix.
I'm pushing to TryServer now...

Comment 7

[Mats Palmgren (inactive)]

The test should be fixed now:
http://hg.mozilla.org/mozilla-central/rev/f8159af84c3b

Regarding the onfocus issue in comment 2; I tested it briefly:
Does onfocus fire when window does not have focus?
Chrome on Linux: yes; when focusing the window it fires it twice again.
Opera on Linux: yes; does not fire it again when focusing the window.
Webkit nightly on OSX: yes; and fires it again when focusing the window.
IE8 on XP (in VirtualBox VM): yes; and fires it again when focusing the window.

So it seems we're the only UA that delay the onfocus until the window
is focused.  Should I file a bug?

Comment 8

[Neil Deakin]

(In reply to comment #7)
> Regarding the onfocus issue in comment 2; I tested it briefly:
> Does onfocus fire when window does not have focus?
> Chrome on Linux: yes; when focusing the window it fires it twice again.
> Opera on Linux: yes; does not fire it again when focusing the window.
> Webkit nightly on OSX: yes; and fires it again when focusing the window.
> IE8 on XP (in VirtualBox VM): yes; and fires it again when focusing the window.

How odd. It would mean that the focus event was firing on an element even when it wasn't focused. That would have problems if a listener of the event was to draw something to indicate focus.

The Safari I have does the same Opera. Not firing when the window is focused is certainly not correct.

Oddly, IE doesn't even hide focus indicators when the window is blurred.

> So it seems we're the only UA that delay the onfocus until the window
> is focused.  Should I file a bug?

Well, Mozilla has always done focus this way, and changing this would certainly cause problems in ui and accessibility code. I think the current way is more correct.

Comment 9

[Mats Palmgren (inactive)]

I filed bug 566671 on the remaining focus issue discussed above.