Closed Bug 564413 Opened 12 years ago Closed 12 years ago

Test for bug 503399 makes reftest require babysitting

Categories

(Core :: General, defect)

x86
macOS
defect
Not set
major

Tracking

()

RESOLVED FIXED
Tracking Status
blocking2.0 --- final+

People

(Reporter: bzbarsky, Assigned: mats)

References

Details

Attachments

(2 files)

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....
(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?
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?
(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.
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.
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....
Attached patch fix the testSplinter Review
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...
Assignee: nobody → matspal
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?
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
(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.
I filed bug 566671 on the remaining focus issue discussed above.
blocking2.0: ? → final+
You need to log in before you can comment on or make changes to this bug.