Opened 12 years ago

Closed 12 years ago

Last modified 8 years ago

#4982 closed defect (fixed)

dijit.form.Button ignores form onsubmit

Reported by: guest Owned by: Douglas Hays
Priority: high Milestone: 1.1
Component: Dijit - Form Version: 0.9
Keywords: Cc: matt@…
Blocked By: Blocking:

Description (last modified by bill)

I've noticed a change in behaviour when clicking a button with type="submit" between 0.9 beta and later versions. Say you have the following form:

<form action="foo.php" method="POST"
                     onsubmit="return confirm('Are you sure?')">
  <button dojoType='dijit.form.Button' type="submit">Save</button>
</form>

In the beta version of 0.9 the onsubmit event fired when the button was clicked. In later versions it does not.

Looking at the code, I found the culprit in the onClick method (refactored to _onButtonClick since r11107), below a comment saying that "for some reason type=submit buttons don't automatically submit the form; do it manually".

I'm not sure this is true: the buttons did submit the form correctly in Firefox and IE previously - I suspect that it is caused by the dojo.stopEvent(e) just above. Is this needed?

Either way, I do not think the button should bypass the form's onsubmit.

Attachments (3)

Button.diff (1.3 KB) - added by guest 12 years ago.
buttonSubmit.diff (1.1 KB) - added by tk 12 years ago.
patch to fix form.onsubmit events from dijit Buttons
4982.patch (11.4 KB) - added by Douglas Hays 12 years ago.
Form widget now understands onsubmit

Download all attachments as: .zip

Change History (15)

comment:1 Changed 12 years ago by guest

I traced down the dojo.stopEvent(e) getting included in the the button class to: http://trac.dojotoolkit.org/changeset/9485/dijit/trunk/form/Button.js

This basically moved the _onButtonClick() method from the combobox class to the button class unaltered - something the 'summary: callback when the user mouse clicks the button portion' should've flagged up for me.

I can see how you'd want to stop the combo box's event propagating. The attached change leaves that intact, but removes the stopEvent(e) from the base Button class. I've given this a try on the test_Button.html page, and it seems to work OK.

Changed 12 years ago by guest

Attachment: Button.diff added

comment:2 Changed 12 years ago by bill

Component: GeneralDijit
Description: modified (diff)
Owner: changed from anonymous to Douglas Hays

Hi Matt, have you filed a CLA? We're very strict about that.

I'm not against that change per se, as long as the Form widget still works.

comment:3 Changed 12 years ago by guest

Hi - just sent our company CLA to cla@…

comment:4 Changed 12 years ago by Douglas Hays

Milestone: 1.0.1
Status: newassigned

Seems to be a regression introduced in 1.0.

comment:5 Changed 12 years ago by Douglas Hays

Resolution: fixed
Status: assignedclosed

(In [11502]) Fixes #4982 for 1.1. Call the form onsubmit to see if the form should be submitted.

comment:6 Changed 12 years ago by Douglas Hays

(In [11503]) Fixes #4982 for 1.0.1. Refer to [11502].

comment:7 Changed 12 years ago by tk

Resolution: fixed
Status: closedreopened

Attaching patch to allow dijit.form.Button type="submit" to still build the onsubmit event that forms pass to their onsubmit functions, current implementation if you connect to form.onsubmit with dojo.connect you will not get an event object which breaks form handlers.

Changed 12 years ago by tk

Attachment: buttonSubmit.diff added

patch to fix form.onsubmit events from dijit Buttons

comment:8 in reply to:  7 Changed 12 years ago by guest

Replying to tk:

Attaching patch to allow dijit.form.Button type="submit" to still build the onsubmit event

Think line 76 should be: sb.type = 'submit';

It seems to me that the real problem is the stopEvent(e) that my original patch removed. Maybe I've misunderstood what your patch actually does, but if Button.diff _can_ address your problem it's considerably simpler and avoids creating a whole new hidden submit button.

Matt

comment:9 Changed 12 years ago by tk

Correct, should be = 'submit', as for your patch... I think it will still have the same problem.... that being the buttons manually call form.onsubmit(); which does not create an onsubmit event object and that is what I'm trying to do so that form handlers (dojo.connect(form, 'onsubmit', 'doSomethingWithTheSubmitObject');) will still have an event object to work with.

comment:10 Changed 12 years ago by bill

Milestone: 1.0.11.1

The idea is to remove both the stopEvent() call and also remove the manual call to form.onsubmit(), and rather let onSubmit() be called naturally.

Just needs to be architected such that the Form widget (and widgets that extend it) work, in addition to a native <form> element.

Changed 12 years ago by Douglas Hays

Attachment: 4982.patch added

Form widget now understands onsubmit

comment:11 Changed 12 years ago by Douglas Hays

Resolution: fixed
Status: reopenedclosed

(In [12044]) Fixes #4982. !strict Added native form attributes to Form attributemap. Deprecated execute() and onExecute() for Form - replaced with onSubmit. Moved form submit code out of Button and made use of native form onsubmit handling. Replaced existing Form execute() code with new onsubmit handling.

comment:12 Changed 8 years ago by bill

Component: DijitDijit - Form
Note: See TracTickets for help on using tickets.