Problem/Motivation

This is a followup issue for #992928: Command line (Drush) install fails on SQLite ($form_state['input'] is not always populated for programmatic form submissions), with the part of the patch that wasn't committed there.

The bug is that it is possible to submit a form using drupal_form_submit() without Drupal ever identifying any button (or "triggering element") as the one that was clicked. Since code exists to make sure that Drupal always identifies a clicked button for browser-based form submissions, it doesn't make sense to allow a loophole for programmatic form submissions to get around that. For example, people writing forms may attach certain behaviors such as #limit_validation_errors to a button, and if it is the only button on the form, expect that that code will always run (that's the situation that came up in the above issue).

This patch makes it so the same code that runs for browsers also runs for programmatic form submissions and so that a triggering element is always identified. In the other issue it was suggested that perhaps this should be bumped to Drupal 8, but to me it seems like a straight-up bugfix. I don't see how anyone could legitimately write a programmatic form submission that relies on Drupal thinking the form was submitted without any of its buttons being clicked, when we already disallow that for regular form submissions.

Steps to reproduce

Proposed resolution

TBA

Remaining tasks

Update patch
Review
Commit

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#17 drupal-form-submit-no-buttons-1008644-17.patch1.57 KBMandakini_Kumari
drupal-form-submit-no-buttons.patch1.7 KBDavid_Rothstein

Comments

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Reviewed & tested by the community

Agree that is it is a bug fix. This is gonna pop up from time to time and it is very hard to spot.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs review

Seems sensible, but I'd love to get someone from #684846: AJAX triggered by non-submit element fails if any elements are validated to chime in here on this fix.

David_Rothstein’s picture

David_Rothstein commented

I added a comment to #684846: AJAX triggered by non-submit element fails if any elements are validated asking for a review of this issue.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

AFAIK, you always had to specify the form button for programmatic form submissions (also in D6).

Therefore, I'm reluctant to do this change. Especially, because browsers actually do not simply choose the "first" button — they are doing something along the lines of

var clickMe = $(triggeringElement, form).closest(':button');

and if they are not, then they are violating the HTML5 spec, which apparently defines that browsers should do this ugly fuzzy match.

Since there is no triggering element for programmatic form submissions, it gets even more weird: Is it

  1. the first button that exists in the form? (have a look at admin/config/development/performance, please)
  2. the first button of type #submit?
  3. the first button that executes form-level submit handlers?
  4. the first button that executes button-level submit handlers?

To be honest, I don't like this idea. Whatever you do programmatically, you should be explicit. Doing assumptions in this kind of functionality quickly turns into "black magic" and confusing behavior that no one understands.

David_Rothstein’s picture

David_Rothstein commented

If Drupal uses an incorrect algorithm to detect the clicked button, that's not an issue with this patch. The code already runs for Internet Explorer. (Or so it claims.) All this patch does is make things consistent.

If you absolutely need to ensure a particular button was clicked in a programmatic form submission, you should still pass it in, of course. This patch simply makes it impossible to wind up in a nonsensical situation where there is no clicked button [edit: triggering element] at all.

effulgentsia’s picture

effulgentsia commented

I'm not sure if this makes sense for D7 or not. Logically, I think it does, and I think the Drush use case is worth considering here. But this is old code from D6 or before, so I'm not sure how much we want to mess with it before D8.

In D6, you have:
form_builder():

if (!empty($form['#programmed'])) {
  $form_state['submitted'] = TRUE;
}

_form_builder_ie_cleanup():

if (empty($form_state['submitted']) && !empty($form_state['buttons']['submit']) && empty($form_state['buttons']['button'])) {

In #684846: AJAX triggered by non-submit element fails if any elements are validated, we changed this for D7 to not distinguish between 'button' and 'submit', because both are rendered as a submit button as far as the browser is concerned, so that was a clear bug fix and related to other things needed by that issue. But changing the logic for programmed form submissions wasn't seen as part of that issue's scope, since programmed form submissions can specify the button explicitly, so we don't have to work around browser quirks. That said, given that we do have an algorithm for picking the correct button when the browser fails to say what it is, I don't see it as all that risky to use the same algorithm for a programmatic form submission.

Sorry, I can't really give this a +1 or -1 at this time. I'll think about it more, and am interested in what chx and eaton think.

casey’s picture

casey commented
Version: 7.x-dev » 8.x-dev

sub

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Closed (works as designed)

I do not think that you should be able to submit a form programmatically without specifying the form button to be triggered.

I'm aware that we still have code for IE in core that tries to automatically determine the button you "intended" to press, but IMHO that's a dirty hack, which is a level of random guessing that is dangerous and risky on its own, which we should remove as soon as possible, and not something we want to advance on.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Just to provide some additional pointers to back up my position:

#1342066: Document that buttons with the same #value need a unique #name for the form API to distinguish them, or change the form API to assign unique #names automatically
#1452894: Elements with #has_garbage_value (image buttons) are always set as a triggering element

David_Rothstein’s picture

David_Rothstein commented
Status: Closed (works as designed) » Active

I do not think that you should be able to submit a form programmatically without specifying the form button to be triggered.

That might be a reasonable way to deal with this, but if so, we have to make it do that :) Right now, nothing enforces this at all.

So, we could, for example, throw a validation error in the case where a form was submitted programmatically with no button specified (and where the form has more than one button to choose from).

David_Rothstein’s picture

David_Rothstein commented
Status: Active » Needs work

Better status.

Mandakini_Kumari’s picture

Mandakini_Kumari commented
Status: Needs work » Needs review

drupal-form-submit-no-buttons.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, drupal-form-submit-no-buttons.patch, failed testing.

Mandakini_Kumari’s picture

Mandakini_Kumari commented

Patch no longer applies.

Getting below error:
ubuntu@ubuntu-Lenovo-G570:/var/www/drupal$ git apply -v drupal-form-submit-no-buttons.patch
Checking patch form.inc...
error: form.inc: No such file or directory
ubuntu@ubuntu-Lenovo-G570:/var/www/drupal$

star-szr’s picture

star-szr
he/him
Primary language English
commented

@Mandakini_Kumari: It's a CVS patch, so git won't be able to apply it. You can find some tips on applying old patches here:
http://drupal.org/node/60116

If you're able to provide a new patch created with git that might be helpful:
http://drupal.org/patch/reroll
http://drupal.org/patch/create

Mandakini_Kumari’s picture

Mandakini_Kumari commented
Assigned: Unassigned » Mandakini_Kumari
Status: Needs work » Needs review
StatusFileSize
new drupal-form-submit-no-buttons-1008644-17.patch1.57 KB

Thanks Cottser attached here is re-rolled patch.

alansaviolobo queued 17: drupal-form-submit-no-buttons-1008644-17.patch for re-testing.

Status: Needs review » Needs work

The last submitted patch, 17: drupal-form-submit-no-buttons-1008644-17.patch, failed testing.

shwetap_07’s picture

shwetap_07 commented
Issue summary: View changes

I want to validate textfield using ajax before submitting form. i.e. if i write something in textfield it should get validate after tab or enter. please help.

$form['add_form']['text_user']=array(
'#title'=>'Enter the UserName for your profile',
'#type'=>'textfield',
'#size'=>20,
'#required'=>TRUE,
'#ajax'=>array(
'callback'=>'user_add_form_validate',
),
);


function use_add_form_validate($form, &$form_state) {
db_set_active('mydb');

///some db code....


$val1 = $form_state['values']['text_user'];
if(in_array($val1,$user))
{
form_set_error('text_user', t('username is already define'));
}

}

db_set_active();
}

please let me known where going wrong :(

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.9.x-dev » 9.2.x-dev

Drupal 8 is end-of-life as of November 17, 2021. There will not be further changes made to Drupal 8. Bugfixes are now made to the 9.3.x and higher branches only. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.15 was released on June 1st, 2022 and is the final full bugfix release for the Drupal 9.3.x series. Drupal 9.3.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.4.x-dev branch from now on, and new development or disruptive changes should be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

quietone’s picture

quietone commented
Assigned: Mandakini_Kumari » Unassigned
Category: Bug report » Task
Issue summary: View changes
Issue tags: +Bug Smash Initiative, +Needs reroll

This was discussed at a bugsmash group triage meeting. Discussed with Pandaski , catch and myself. catch pointed out "there are still use-cases for programmatically submitting a form like combining multiple form definitions into one (multiform, entity embed etc.)". He also said that this is a task, there is no bug here.

I am updating the issue.

catch’s picture

catch
he/him
Primary language English
commented
Related issues: +#2910320: Validating managed files should account for null triggering elements

This conflicts with #2910320: Validating managed files should account for null triggering elements

Version: 9.5.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.