Wrong implementations of forms _submit() functions.
| Project: | MySite |
| Version: | 5.x-3.3 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs review |
As per the forms api quick start guide, a _submit function should return the url to be redirected to and not use drupal_goto directly.
http://api.drupal.org/api/file/developer/topics/forms_api.html/5
Look under the section labeled "Submitting Forms", bullet number 4:
The return value of the _submit function will be the target of a drupal_goto; every form is redirected after a submit. If you return nothing, the form will simply be redirected to itself after a submit.
Wrong:
<?php
function myform_submit($form, $form_values) {
// do your thing
drupal_goto('somwhere');
}
?>Good:
<?php
function myform_submit($form, $form_values) {
// do your thing
return 'somwhere';
}
?>There are various forms _submit() function in the mysite module which are calling drupal_goto() instead of returning.
This means that I cannot use hook_form_alter() and override the destination by altering $form['#redirect'] like I am supposed to.
http://api.drupal.org/api/file/developer/topics/forms_api_reference.html...
#1
Here is proposed patch
#2
Looks good. Does it break anything?
#3
I did not take the time to do full suite of tests.
Does it even have a suite of unit test for d5?
#4
Good old fashioned form clicking is the only way to go. The only thing I could see breaking is the fake multi-step form stuff in edit and settings, but I don't see that affected here.
Those gotos(), btw, are a legacy of 4.7.x, I think.
#5
Looks good. I will roll a new release.
#6
Actually, there are instances of these all over the Type includes as well.
#7
Here's an extended patch. One good review and I'll roll a new realease.