Bruno 'Aqualon' Escherl Reporter
	Description • 14 years ago

Probably the result from bug 599258. We don't even get to the first check in that test, so the hang occurs somewhere else.

	Justin Wood (:Callek)
	Comment 1 • 14 years ago

Does this fail locally, and if so can you check if there are any existing observers for invalidformsubmit registered. [I don't see how]

But this test shouldn't call |waitForExplicitFinish| if there are no observers and thus shouldn't time out.

	Mounir Lamouri (:mounir) Assignee
	Comment 3 • 14 years ago

The test is run when onload is called. waitForExplicitFinish should have been called before. I don't see how that might break SeaMonkey but that was a mistake. So, now finish() is explicitly call if there is no observer.

	Justin Wood (:Callek)
	Comment 4 • 14 years ago

Comment on attachment 478592 [details] [diff] [review]
Patch v1

Unable to test locally right now, so feedback+ If you want an official review before being willing to push this, re-request and I'll be sure to do just that [with local testing]. code-inspection-wise this looks good though.

	Justin Wood (:Callek)
	Comment 5 • 14 years ago

Interesting... I didn't fail this locally.... :/

	Bruno 'Aqualon' Escherl Reporter
	Comment 6 • 14 years ago

The test hang for me and passes with the patch. So looks good.

	Mounir Lamouri (:mounir) Assignee
	Comment 7 • 14 years ago

Justin, according to comment 5, you were able to test this. May I have you r+? :)

	Justin Wood (:Callek)
	Comment 8 • 14 years ago

(In reply to comment #7)
> Justin, according to comment 5, you were able to test this. May I have you r+?
> :)

c#5 was about with_out_ this patch, I didn't fail locally....  but r+ anyway based on Aqualon's c#6

	Mounir Lamouri (:mounir) Assignee
	Comment 9 • 14 years ago

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

	Serge Gautherie (:sgautherie)
	Comment 10 • 14 years ago

(In reply to comment #9)
> http://hg.mozilla.org/mozilla-central/rev/d0d3ac2ce08c

    1.32 +  if (!observers.hasMoreElements()) {

Nit: adding an explicit todo(false, ...) or ok(true, ...) would be perfect.

    1.33 +    SimpleTest.finish();
    1.34 +    return;
    1.35 +  }

	Mounir Lamouri (:mounir) Assignee
	Comment 11 • 14 years ago

The current mochitest behavior is to add a todo if there is no test. Isn't that enough?

	Justin Wood (:Callek)
	Comment 12 • 14 years ago

(In reply to comment #11)
> The current mochitest behavior is to add a todo if there is no test. Isn't that
> enough?

Yes it is. And I have not seen this on any runs of the test for SeaMonkey machines since this landed--> Verified