#633 FormTagHelper#submit_tag with :disable_with option doesn't submit the button's value when clicked - Ruby on Rails

Type	To find
responsible:me	tickets assigned to you
tagged:"@high"	tickets tagged @high
milestone:next	tickets in the upcoming milestone
state:invalid	tickets with the state invalid
created:"last week"	tickets created last week
sort:number, importance, updated	tickets sorted by #, importance or updated
Combine keywords for powerful searching.
Use advanced searching »

This project is archived and is in readonly mode.

#633 ✓committed

FormTagHelper#submit_tag with :disable_with option doesn't submit the button's value when clicked

Reported by Jose Fernandez | July 16th, 2008 @ 05:03 PM | in 2.x

When the :disable_with option is used in the FormTagHelper#submit_tag, the button's value isn't passed along in the parameters, { :commit => 'Foobar' }.

This occurs because when :disabled_with is used, we overwrite the onclick event and manually submit the form executing 'this.form.submit()'. Thus, the submit button acts as if it was never clicked. Also, disabled input fields do not get submitted.

This is a problem when you have forms with more than one submit_tag's, and your update/create logic is depending on the 'commit' value to react accordingly.

What the patch does is add some js at the beggining of the onclick event, so that it clones the submit_tag into a hidden input element with the original values. This way way when the form is finally submitted, it still passes along the submit button value.

The FormTagHelper submit_tag tests have also been refactored to support this new feature.

Comments and changes to this ticket

You flagged this item as spam.
Jose Fernandez July 16th, 2008 @ 05:09 PM
Clarification: The submit_tag original value is the one that gets submitted, not the disable_with one.
You flagged this item as spam.
Jonathan Dance July 16th, 2008 @ 09:57 PM
If this is a bug, the patch would be more awesome if it had a test that would fail without this patch, e.g. the value of the submit button should be passed when using :disable_with.
You flagged this item as spam.
Jose Fernandez July 16th, 2008 @ 10:58 PM
- Tag changed from “actionpack, helper, patch, tested” to “actionpack, bug, helper, patch, tested”
The problem with that is how to simulate a click on the submit_tag and a mocked form submission (in order to check that the hidden input fields gets successfully created and passed long the request).
I think as long as the modified submit_tag tests do a dom check that includes the extra js, its all good.
You flagged this item as spam.
Jonathan Dance July 16th, 2008 @ 11:02 PM
Hm, I guess it would require Selenium (or something similar). Oh well, it was a good thought.
+1
Pratik July 17th, 2008 @ 01:27 AM
- Title changed from “[PATCH] FormTagHelper#submit_tag with :disable_with option doesn't submit the button's value when clicked” to “FormTagHelper#submit_tag with :disable_with option doesn't submit the button's value when clicked”
You flagged this item as spam.
Lawrence Pit July 21st, 2008 @ 02:51 AM
I think you need to test whether the hidden field already exists or not. This is because the form can have a custom onsubmit handler (doing some ajax stuff for example). When that fails, the submit button is enabled again. When you press submit again, the current patch will again add a hidden field.
You flagged this item as spam.
Jose Fernandez July 21st, 2008 @ 04:10 AM
I was actually thinking about that issue recently, will update patch tomorrow
Jose Fernandez July 21st, 2008 @ 06:02 PM
- no changes were found...
- submit_tag_and_disable_with_option_original_value_submission.diff 3.6 KB
You flagged this item as spam.
Jose Fernandez July 21st, 2008 @ 06:02 PM
Updated the patch with a a check to see if the reference to the dynamically created (hiddenCommit) element exists in the window object before creating and appending it again.
Tests have also been updated, all passed too.
You flagged this item as spam.
spovich August 18th, 2008 @ 10:32 PM
+1

We were bitten by this bug. Would love to see this fixed in Rails 2.1.1
Pratik August 18th, 2008 @ 10:45 PM
- Assigned user set to “Pratik”
You flagged this item as spam.
Ryan Bates August 19th, 2008 @ 12:26 AM
+1, I've run into this problem before too.

But does this patch handle the case where a different button is pressed after the original onsubmit failed? Maybe add an "else" condition if the hidden field exists and override the value there.
You flagged this item as spam.
Jose Fernandez August 19th, 2008 @ 02:42 AM
Good idea Ryan, updating the patch atm.
You flagged this item as spam.
Jose Fernandez August 19th, 2008 @ 11:21 PM
Here's the updated patch. It will check the window DOM object for the hiddenCommit element and update the value if it already exists, or create it otherwise.

I also stopped using the array of strings + joining with ';' way of writing the javascript since it was causing problems with the if/else js statements.

Tests were also updated, both passed.
- updated_submit_tag_and_disable_with_option_original_value_submission.diff 5.2 KB
You flagged this item as spam.
Jose Fernandez August 19th, 2008 @ 11:25 PM
Oh yeah, and I also switched the += operator with << at line 363.
DHH September 10th, 2008 @ 06:05 AM
- State changed from “new” to “committed”
You flagged this item as spam.
Han Kessels March 27th, 2009 @ 07:20 AM
This patch breaks the :disable_with option on all versions of Internet Explorer.

IE does not allow the modification of the type attribute of an element. This can be fixed by replacing cloneNode on the submit input element in the original patch by the creation of a new hidden input element. A small patch with updated tests is attached.
- disable_with_patch.diff 6.3 KB