596636 - [SeaMonkey 2.1] mochitests-1/5: "TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug589696.html | submit event should not be sent"

Status: VERIFIED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Bruno 'Aqualon' Escherl]

http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1284558919.1284562508.24896.gz#err1

WARNING: There is no observer for "invalidformsubmit". One should be implemented!: file /builds/slave/comm-central-trunk-linux-debug/build/mozilla/content/html/content/src/nsHTMLFormElement.cpp, line 1730
NEXT ERROR 50440 ERROR TEST-UNEXPECTED-FAIL | /tests/content/html/content/test/test_bug589696.html | submit event should not be sent

After that the test times out.

Mounir: Is this a failure with the test or do we have to port some patches from Firefox?

Comment 1

[Mounir Lamouri (:mounir)]

Hmm, my bad I think. I should check if there is an observer for invalidsubmit event. If there is no, I shouldn't expect the form submission to be canceled.

I'm assigning the bug to myself to keep it in my radar. If someone wants to fix it, please do, it's quite simple. (I've another dom test in the same directory doing that)

Comment 2

[Ian Neal]

Attachment: Patch with whitespace changes v0.1

Not had chance to test yet...

Comment 3

[Ian Neal]

Attachment: Fix with whitespace changes hidden to help review v0.1 [Checked in: Comment 5]

This adds check for observer before trying the test.

Comment 4

[Mounir Lamouri (:mounir)]

Comment on attachment 476440 [details] [diff] [review]
Fix with whitespace changes hidden to help review v0.1 [Checked in: Comment 5]

>+if (observers.hasMoreElements()) {
>+  SimpleTest.waitForExplicitFinish();
>+

You should not add a call to waitForExplicitFinish(). There is already one.

>- * failure if we got one when unexpected. All of them should be catched if sent.
>+   * failure if we got one when unexpected. All of them should be caught if
>+   * sent.

I guess that was part of the code shift.
By the way, it would be better to have all this code correctly indented for the patch to push. And make sure to not add two spaces for these lines when you will re-indent the code.

r=me with those fixed

Comment 5

[Ian Neal]

Comment on attachment 476440 [details] [diff] [review]
Fix with whitespace changes hidden to help review v0.1 [Checked in: Comment 5]

http://hg.mozilla.org/mozilla-central/rev/87b8633ad8ca
Checked in with correct indentation and no additional waitForExplicitFinish
a=testing

Comment 6

[Serge Gautherie (:sgautherie)]

V.Fixed, per
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1284826967.1284829315.13788.gz
Linux comm-central-trunk debug test mochitests-1/5 on 2010/09/18 09:22:47