David Baron :dbaron: Reporter
	Description • 15 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251134962.1251140722.13052.gz
Linux mozilla-central unit test on 2009/08/24 10:29:22  

...
91512 INFO TEST-PASS | /tests/toolkit/content/tests/widgets/test_menubar.xul | Deactivate menubar with f10 key DOMMenuItemInactive fired
91514 INFO Running /tests/toolkit/content/tests/widgets/test_menuchecks.xul...

command timed out: 300 seconds without output, killing pid 11248

	David Baron :dbaron: Reporter
	Comment 1 • 15 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251192456.1251198718.6998.gz
Linux mozilla-central unit test on 2009/08/25 02:27:36

	Serge Gautherie (:sgautherie)
	Comment 2 • 15 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251302339.1251311021.1197.gz
Linux mozilla-central unit test on 2009/08/26 08:58:59

	Jonathan Watt [:jwatt]
	Comment 3 • 15 years ago

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251336413.1251342720.31267.gz
Linux mozilla-central unit test on 2009/08/26 18:26:53

	Nick Thomas [:nthomas] (UTC+12)
	Comment 4 • 15 years ago

Also 

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1251337584.1251341368.16640.gz
Linux mozilla-central test mochitests on 2009/08/26 18:46:24

91580 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_menuchecks.xul | Test timed out.
91583 INFO Error: Unable to restore focus, expect failures and timeouts.
91584 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_menulist_keynav.xul | cursor down command event fired - got false, expected true
91585 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_menulist_keynav.xul | cursor down selectedItem - got [object XULElement], expected [object XULElement]
91586 ERROR TEST-UNEXPECTED-FAIL | /tests/toolkit/content/tests/widgets/test_menulist_keynav.xul | cursor down skip disabled command event fired - got false, expected true
leading to 
92764 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | 4 test timeouts, giving up.
92765 ERROR TEST-UNEXPECTED-FAIL | (SimpleTest/TestRunner.js) | Skipping 36 remaining tests.

And 
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1251338845.1251341963.23129.gz
Linux mozilla-central test mochitests on 2009/08/26 19:07:25

	Nochum Sossonko [:Natch] Assignee
	Comment 5 • 15 years ago

Might be worth trying this, sending it to the tryserver now. This passes on my machine.

	Neil Deakin
	Comment 6 • 15 years ago

This isn't likely to work once bug 506173 is fixed.

	Nochum Sossonko [:Natch] Assignee
	Comment 7 • 15 years ago

(In reply to comment #6)

In won't ever work any less than it works now, the reason I changed it is because this test is essentially just waiting for onload, not onfocus. It _wants_ to be the active window when the test runs, but onfocus doesn't help with that anyhow. It should be the active window regardless, I just put the |window.focus()| call in there as an attempt to set it as active. Once bug 506173 is fixed, there will inevitably have to be a fix for all other instances of this hack, this one can be included.

Tryresults: (Mac, with a warning, unrelated) http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1251346831.1251354655.32266.gz

(Win, clean) http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1251346831.1251353952.24753.gz

(Linux, clean) http://tinderbox.mozilla.org/showlog.cgi?log=MozillaTry/1251346831.1251353157.15987.gz

	Neil Deakin
	Comment 8 • 15 years ago

I don't understand. The test relies on the window being focused right? window.focus() should be asynchronous and doesn't guarantee that the window is active after it returns.

	Nochum Sossonko [:Natch] Assignee
	Comment 9 • 15 years ago

Comment on attachment 396949 [details] [diff] [review]
maybe?

Ah bug 506175 indeed! I was a bit confused about the implications of bug 506173. So, summing up, |window.focus()| is the correct way to activate a window, only it's asynchronous, and you have to wait for onfocus.

	Johnathan Nightingale [:johnath]
	Comment 10 • 15 years ago

This is pretty active right now - can we get a fix together?

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1251814846.1251818386.9832.gz&fulltext=1
Linux mozilla-central test mochitests on 2009/09/01 07:20:46

	Nochum Sossonko [:Natch] Assignee
	Comment 11 • 15 years ago

Is there any reason this has to be done with a separate window? Seems to run just fine on my machine.

	Neil Deakin
	Comment 12 • 15 years ago

Comment on attachment 397889 [details] [diff] [review]
don't open another window

The basic idea of this change is ok as this test was written when the iframe displaying tests was only 100 pixels high, but now it is 300 pixels high which is enough space now.

The patch here doesn't cause the test to actually run though, as runTest isn't called.

Also, remove the margins on the hbox.

Finally, the test will need to be updated for bug 513707.

	Nochum Sossonko [:Natch] Assignee
	Comment 13 • 15 years ago

Comments 1 (stupid qref crap) & 2 addressed. Comment 3, not really sure what needs to be addressed. Bug 513707 is really only necessary for tests that open new windows, tests that don't open windows at all expect the focus to be on their window (the testing window, that is), where it should be. At least so it seemed to me.

	Neil Deakin
	Comment 14 • 15 years ago

Comment on attachment 397897 [details] [diff] [review]
rev. 2, comments addressed

>+  function runTest()
>+  {
>+    SimpleTest.waitForExplicitFinish();

waitForExplicitFinish should be called outside a function rather than in a load handler.

	Nochum Sossonko [:Natch] Assignee
	Comment 15 • 15 years ago

Thanks. Hopefully this will quiet t-boxes a bit!

	Chris Pearce [:cpearce (Not reading bugmail)]
	Comment 16 • 15 years ago

Linux mozilla-central unit test on 2009/08/31 18:03:34
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1251767014.1251777779.5139.gz

Natch: did you mean to request review on your patch? Would be good to quiet the t-boxen down!

	Nochum Sossonko [:Natch] Assignee
	Comment 17 • 15 years ago

Comment on attachment 397912 [details] [diff] [review]
patch
[Checkin: Comment 18]

Chris, this patch has r+ from Enn already, it's good to go, just need someone to check-in for me.

	Robert O'Callahan (:roc) (email my personal email if necessary)
	Comment 18 • 15 years ago

http://hg.mozilla.org/mozilla-central/rev/e7aa4e5ec9c0

Should we request 1.9.2 approval for this?

	Nochum Sossonko [:Natch] Assignee
	Comment 19 • 15 years ago

(In reply to comment #18)

I don't know, for the past 36 hours this hasn't shown up once on 1.9.2 tinderboxen, perhaps this needs to be investigated more thoroughly?

Does anyone have a timeframe of when this began? Does anyone know if this happened at all on 1.9.2?

	(no longer active)
	Comment 20 • 15 years ago

Here is a 1.9.2 instance:

Linux mozilla-1.9.2 test mochitests
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6-Unittest/1252840440.1252844153.18232.gz

	Jeff Muizelaar [:jrmuizel]
	Comment 21 • 15 years ago

And another 1.9.2:

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox3.6-Unittest/1252922522.1252926470.29317.gz

	Nochum Sossonko [:Natch] Assignee
	Comment 22 • 15 years ago

Did something land recently on 1.9.2 that might cause this (focus code, window code, event code, etc.)?

Perhaps it will be easier to figure this out now by cross-referencing the 1.9.2 start and m-c start, unless this has been happening on 1.9.2 for a while and I've been missing it?

	Nochum Sossonko [:Natch] Assignee
	Comment 24 • 15 years ago

Comment on attachment 397912 [details] [diff] [review]
patch
[Checkin: Comment 18]

From the duped bug it seems this is happening pretty frequently on branch, requesting approval.

	(not currently active) Ted Mielczarek
	Comment 25 • 15 years ago

Test changes have blanket approval to land on branches. Land away!

	Nochum Sossonko [:Natch] Assignee
	Comment 26 • 15 years ago

Thanks. I'll add the keyword then.

	(no longer active)
	Comment 27 • 15 years ago

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/aa4e39e3394a

	Dão Gottwald [:dao]
	Comment 28 • 15 years ago

1.9.2 is burning. Ehsan, are you around?

	Dão Gottwald [:dao]
	Comment 29 • 15 years ago

backed out

	(no longer active)
	Comment 30 • 15 years ago

Thanks for covering for me, Dão, and sorry I caught this too late.

The problem was that I mistakenly removed test_menuchecks.xul instead of window_menuchecks.xul from Makefile.in.  Nochum, it'd be great if you can attach a patch which applies cleanly on 1.9.2.

	Nochum Sossonko [:Natch] Assignee
	Comment 31 • 15 years ago

I'll update this later today, guess I figured that the tests wouldn't bitrot or change...

	Nochum Sossonko [:Natch] Assignee
	Comment 32 • 15 years ago

The last patch applies perfectly on 1.9.2 as of now (just updated and tested and it applied fine, no fuzz).

	(no longer active)
	Comment 33 • 15 years ago

Landed again on 1.9.2:

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/2a496c6df8c2