[PATCH] rails.js fix for submitting forms with multiple submit buttons

Hey Andrew,

It's a good idea to try to solve this, but I don't agree this is a good approach.

Prototype form serialization is done by the Form#serializeElements method, which—as you noticed—by default includes the first submit button because it can't know which one was pressed. However, it supports an optional argument submit with which you can specify the name of the submit button you want to include instead of the first button.

So, in your case we would want the serialize method in rails.js to get invoked like this:

params = element.serialize({ submit: 'delete' });

Now the problem remains how do we know which submit button was pressed? There are 3 ways a user can trigger form submit that I can think of:

1. hitting the Return key while in a text input element: unless it was a TEXTAREA, this submits the form as if the first submit button was pressed. because Prototype already serializes by this, we're OK.
   
2. pressing the submit button with the mouse or finger: a mouseup or touch event on a button means it got pressed.
   
3. activating the submit button with keyboard: a keypress of the Return (and possibly Spacebar?) key means the currently focused submit button got pressed.

My suggestion is to observe all these events on submit buttons which can trigger for submit, attach a piece of metadata on the form with Element#store indicating the name of the button, then use that information when calling serialize:

params = element.serialize({ submit: element.retrieve('rails-pressed-button') });

Great feedback. Some bits of prototype I wasn't aware of there. I'll rework this with your suggestions as soon as I can. Thanks.

I've attached a new version.

In my testing a mouse click, space bar press and enter key press all register the 'click' event. A mouseup event doesn't mean a button was pressed though. I can click anywhere on the page, hold the button down and move it over a button and release it for a mouseup. Either way, the 'click' event appears to be the only reliable event that is triggered for the space bar, enter key and mouse. I've also changed the code to use the 'store'/'retrieve' method in prototype.

Please let me know if any changes to method names, code style, logic, etc need to be changed.

Andrew,

Using your latest patch I am getting nothing returned. Line 23 should be: return submit.name;

Oops 79 not 23, was looking at the patch not the file but just to be clear...

function whichSubmitButton(form, reset) {
  var submit = form.select('input[type=submit]').detect(function(input){
    return input.retrieve('rails-submit-button') === true;
  });
  if (reset === true) {submit.store('rails-submit-button', null);}
  return submit.name; //added .name to submit
}

Thanks Larry. It appeared to be working for me locally, but looking at the code for serializeElements in prototype, it does seem to do a comparison with the 'name' value.

I've attached a diff with the newest commit. My change is slightly different and does a check for the submit element before returning the name attribute.

Please review and let me know if things are working ok.

Thanks

This wasn't exactly what I had in mind. If you store the name of the submit button on the form rather than on the button itself, you don't need the whichSubmitButton function.

document.on('click', 'form input[type=submit]', function(event, button) {
  // register the pressed submit button
  event.findElement('form').store('rails:submit-button', button.name || false);
});

// and then:
params = element.serialize({ submit: element.retrieve('rails:submit-button') });

The complete solution is much less complex and handles the case when the submit button didn't have a "name" to start with (no submit button value should be serialized in that case).

Storing on the form makes much more sense. I'll get another patch together soon. Thanks.

Here's a patch. Put Andrews name on it since he's been doing the work.

Yeah, funky stuff with tabs in textmate. Format correction attached.

Created a little rails app for testing in different browsers. It just returns the name and value from the button pressed pulled from the params in the controller.

http://ajax-button-test.heroku.com

iphone iOS4: working
safari 5.0.1: working
firefox 3.6.8: working

These are all the browsers I could test at the moment.

Also I had to perform a little hakery in rails.js to get the responseText. Tried using observe ajax:success but was only getting [object Event] returned which was probably fire... what is the best way to get the response from ajax:success using the current implementation of rails.js?

I'm away traveling so am a bit staged with getting online. I noticed Larry attached a new patch. I have a new patch as well, but don't do the "|| false" part as that stops any of the "submit" values from being serialized, is that what we are going for if nothing is found then?

I'll attach my patch as well just in case, but if we're leaving the || false part it then we can just use Larry's latest.

@Andrew: the || false fallback is what we want if the button doesn't have a name attribute. In this case, { submit: false } will be given to serialize method and no button value should be serialized. I'm pretty sure this is native browser behavior, too.

@Larry: rails.js fires custom events at all stages of an Ajax request. To access the responseText property, observe the "ajax:success" event and access its memo property:

document.on("ajax:success", function(e) {
  $('responseText').update(e.memo.responseText)
})

The memo property on a custom event is any object that the code which fired the event in the first place wished to pass on. In rails.js case, this is the Ajax request object.

Fair enough. I just wanted to make sure that's what the plan for the submit button was. Besides that, Larry's patch is what mine would be then too.

Pushed to prototype-ujs, thanks guys.

I noticed that the rails.js file that comes with rails 3 is still out of date and doesn't include this fix...

http://github.com/rails/rails/blob/master/railties/lib/rails/genera...

Shouldn't the original patch be committed to rails master to reflect this update? Otherwise why would the current rails.js be left as is in master?

To avoid any confusion, I didn't mean to say "original" patch there, just meant to say "patch" by itself... the one that was pushed to prototype-ujs.

The prototype-ujs project should be the source of edge rails.js for people that wish to update, not Rails master. I expect prototype-ujs to be updated much more frequently than it would make sense to merge back into Rails.

So everything committed to prototype-ujs will be merged for the next rails point release? I just thought it would have been merged for the final rails 3 release as well.

This ticket might as well be closed now.

Thanks