277890 - disabling BUTTON after form.submit() prevents submission

Status: RESOLVED FIXED

(Core :: DOM: Events, defect)

Description

[Andrew Schultz]

This appears to be a close cousin of bug 242709, for BUTTON elements instead of
INPUT/submit buttons.  A BUTTON whose javascript action submits the form and
then disables the BUTTON does not successfully submit the form.  State Farm's
website seems to use this type of structure quite a bit.

This regressed between linux trunk 2004042807 and 2004050105 (bug 60212).

Comment 1

[Andrew Schultz]

Attachment: testcase

testcase exhibits the bug with linux trunk 2005010905
clicking "Submit" disables the button and does not submit the form.

Comment 2

[Boris Zbarsky [:bzbarsky]]

I think doing the clickOrExtActivate thing a la bug 242494 should fix this....

mrbkap, feeling bored?  ;)

Comment 3

[Phil Ringnalda (:philor)]

*** Bug 307679 has been marked as a duplicate of this bug. ***

Comment 4

[Adam Guthrie]

*** Bug 311105 has been marked as a duplicate of this bug. ***

Comment 5

[codelogic]

(In reply to comment #4)
> *** Bug 311105 has been marked as a duplicate of this bug. ***

Almost exactly the same, except that, in that case, the button is disabled
before submitting the form, and not after.

Comment 6

[codelogic]

BTW, the bug seems to appear only when using the <BUTTON> tag and not the <INPUT type=button> tag.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Yes, of course.  See comment 0 and the bugs listed therein.

Comment 8

[Martijn Wargers (dead)]

*** Bug 322503 has been marked as a duplicate of this bug. ***

Comment 9

[Martijn Wargers (dead)]

Attachment: patch?

So, appr. something like this?
It seems to make the testcase work.
I'm getting an assertion in a debug build, btw, with https://bugzilla.mozilla.org/attachment.cgi?id=197421 (from bug 309348), I've filed this as bug 322908.

Comment 10

[Boris Zbarsky [:bzbarsky]]

That doesn't look much like the <input> code... might be equivalent, but ask bryner to review?

Comment 11

[Martijn Wargers (dead)]

Comment on attachment 208072 [details] [diff] [review]
patch?

I don't really know this code, so this might be totally wrong.

Comment 12

[Mike Schroepfer]

Bryner - can you take a look at the patch

Comment 13

[Mike Schroepfer]

Removing from blocking list as we likely wouldn't block the release for this - but would really like this if patch it suitable for the branch

Comment 14

[Brian Ryner (not reading)]

Comment on attachment 208072 [details] [diff] [review]
patch?

It's been awhile since I've looked at this code, but won't this break invoking form submission by dispatching a DOMActivate event to the button?

Comment 15

[Boris Zbarsky [:bzbarsky]]

bryner, that patch just sets up the same code in nsHTMLButtonElement as we had in nsHTMLInputElement (although the patch has completely rotted due to the event landing on trunk).  The nsHTMLInputElement code is code you wrote in bug 242494.    You said it "does not regress any of the DOMActivate tests", so I assume you have some.  Can you point them out, so that someone could test the patch for this bug against them (and hence maybe answer your question)?

Comment 16

[Brian Ryner (not reading)]

(In reply to comment #15)
> bryner, that patch just sets up the same code in nsHTMLButtonElement as we had
> in nsHTMLInputElement (although the patch has completely rotted due to the
> event landing on trunk).  The nsHTMLInputElement code is code you wrote in bug
> 242494.    You said it "does not regress any of the DOMActivate tests", so I
> assume you have some.  Can you point them out, so that someone could test the
> patch for this bug against them (and hence maybe answer your question)?
> 

That's the problem I don't think the code is the same.  In nsHTMLInputElement, clickOrExtActivate is set to true if we're in a click, _or_ if we're in DOMUI_ACTIVATE, as long as it's not the DOMActivate _caused_ by the click.  This patch to nsHTMLButtonElement seems to only perform the submit if the event was a click.

Comment 17

[Boris Zbarsky [:bzbarsky]]

Ah, ok.  Martijn, do you have time to update the patch to trunk and the review comments?

Comment 18

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Martijn, will you be able to fix this for 1.9?

Comment 19

[Martijn Wargers (dead)]

Sorry, I looked at this code, but I don't really know this code. It's hard for me to do the similar thing for nsHTMLButtonElement as is done for nsHTMLInputElement, because the code is quite different (not only named differently).

Comment 20

[Adam Guthrie]

Attachment: Mochitest test

I also added a testcase for bug 242709 which is exactly the same as the one for this bug, except it uses an <input> instead of a <button>.

Comment 21

[Adam Guthrie]

Comment on attachment 267181 [details] [diff] [review]
Mochitest test

Smaug, since you're a content peer, could you review?

Comment 22

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

But we can't take that test yet, because it hangs mochitest.
Testcase for 277890 doesn't fail or pass.

Comment 23

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Comment on attachment 267181 [details] [diff] [review]
Mochitest test

So could you make the todo() to actually work. At least for me mochitest just hangs.

Comment 24

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Taking.
Jonas, if you already have the patch, feel free to take this back :)

Comment 25

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Attachment: make button to work like input type="submit"

This is almost copy-paste from nsHTMLInputElement.

Comment 26

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

Better to fix this first before starting to change when DOMActivate is 
dispatched (bug 127903)

Comment 27

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276899 [details] [diff] [review]
make button to work like input type="submit"

Shouldn't we only flush the pending submission if we're in an outer activate?  Or am I misreading the input code?

Comment 28

[Olli Pettay [:smaug][[e-mail]] (holiday -> Aug30)]

(In reply to comment #27)
> (From update of attachment 276899 [details] [diff] [review])
> Shouldn't we only flush the pending submission if we're in an outer activate? 

NS_IN_SUBMIT_CLICK is set only when in outer activate, so flush happens
only in outer activate when event was prevented.
And if the event wasn't prevented, then dispatch 'submit' to form.
I could add a comment about NS_IN_SUBMIT_CLICK.

Comment 29

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 276899 [details] [diff] [review]
make button to work like input type="submit"

With comment, looks ok, though I admit that I didn't double-check every niggling detail....