Add support for detecting the triggering element when buttons are changed client-side

Doesn't Drupal 7 already try to identity the triggering element by a combination of both #name and #value, rather than just #value? See _form_button_was_clicked() and _form_element_triggered_scripted_submission(). So it's not obvious to me what scenario this patch would help in.

Also if this is the essentially the same as a Drupal 8 issue, it should be closed in favor of that one so we don't have duplicated discussions. (And #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 is already marked for Drupal 7 backport.)

_form_button_was_clicked() relies on both #name and #value matching - but yes, I think I see what you're saying, and how your patch addresses it.

So basically:
1. If a developer creates a unique #name for their form submit button, and
2. It's not the first button on the form, and
2. If the label of the submit button has been changed client-side

Then:
1. Currently Drupal is not smart enough to ever detect that it was clicked.
2. But with your patch, it will be, since it will do a final check to see if something matching the button's #name was submitted (regardless of #value) and use it in that case rather than just blindly falling back on the first form button.

At first glance this makes a lot of sense and I agree it sounds backwards compatible also.

It also sounds different from the other issue, so I agree with keeping them separate. But this would need to be considered for Drupal 8 first and then backported - as far as I can see, Drupal 8 has the same fallback logic in https://api.drupal.org/api/drupal/core!lib!Drupal!Core!Form!FormBuilder.... as Drupal 7 currently does.

Has this patch been backported to Drupal7? If not where can I see the progress of the backport?

Thanks IRuslan, It was working fine for me also.

@IRuslan, Thanks too. I will get back here and #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 and #1150006: How to add 'Delete' button using Ajax to a form themed as a table with small module for d8 that reproduce this bug. Confirm that patch works!
About review:

1. +++ b/core/lib/Drupal/Core/Form/FormBuilder.php
   @@ -1084,7 +1084,22 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state
   +            // Try to find an ajax-submitted element by name matched to _triggering_element_name.
   

   Split into two lines to avoid a long string (don't take this file as example of standards)) )

2. +++ b/core/lib/Drupal/Core/Form/FormBuilder.php
   @@ -1084,7 +1084,22 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state
   +              || (!empty($input[$button_element['#name']]))) {
   

   Move ) { on new line

Ok, I know

When in Rome, do as the Romans do.

So, maybe we can write some tests for that?

:)

This fixed an issue with D7 and seems to be the same patch for D8, Still applies and is quite well documented as to what it's attempting to do.

+++ b/core/lib/Drupal/Core/Form/FormBuilder.php
@@ -1084,7 +1084,22 @@ public function doBuildForm($form_id, &$element, FormStateInterface &$form_state
+        // If there was no specific triggering button detected by an element value,

Line exceeding 80 characters
There is one line which is exceeding 80 characters, applying the patch, please review.

Still RTBC

This needs some test coverage

I am working on a module that desperately needed the correct triggering_element to be called because I didn't know the names of the fields as they were being created. #6 worked for my problem, and it's a great relief. Is there any estimate on implementing this into 7 core?

Patch in #6 solved the issue for me on a D7 site where AJAX buttons were not behaving properly, the first one was always getting triggered if the buttons were clicked in a specific sequence.

I'd like to try writing the test coverage for this but I'm wondering if anyone has a really simple repeatable set of form elements that trigger the problem? If not I'll try and come up with something.

For me the probem was solved by adding '#name' => 'unique_id' to the button-element ...

The test case is having 2 submit buttons with a same value and different names?

Updated Drupal 7 patch to apply properly to 7.64.

#9 describes the steps needed to write a test.

From a D7 maintainer I am okay with the patch now.

I had this problem in Drupal 8 in a multi-step webform. I happened to use the same value in 2 blocks of 6 buttons each. Each block of 6 buttons I listed it from 1 to 6. Finally, I solved it by adding the attribute #name.

I have added a test where the submit button is cloned, unique names have been assigned and a unique payload to test for something different than the name.

During submit the value (label) of the submit button is changed.

I think this looks ok?

At a minimum, this needs a re-roll. I can confirm the bug still exists in core. I have a form with multiple 'button' elements to handle triggering actions other than submitting/saving the form. However, the ajax callback always receives the same triggering element, regardless of which one is actually triggered. Changing the form element to something else, like a 'radio' fixes the issue.

I can confirm patch #39 works with D8.8.5. Thanks!

andyg5000 Could you please explain what has to be re-rolled? The patch still applies to all relevant Drupal versions (I messed up with the PHP versions...). If you were referring to the test-only file; that was supposed to fail.

Setting back to needs review for now. Please change it back if I've overlooked something.

Patch #46 needs to be the version for further D7 testing and review.

Patch #34 causes a fatal error with Views Data Export batch processing, and returns a 403 Access Denied error. The update in patch #46 fixes this problem.

Thank you to @jefuri for identifying this change.

This came up as the Bug Smash Initiative daily triage target.

I don't think this is documented as something that the form API supports, so in my mind it is a feature request.

Adding a new title.

Tagging for issue summary update to detail the remaining tasks.

@larowlan, If I am not mistaken this also applied to dropbuttons (possibly generated via AJAX). There is no way to tell what button was pressed. I would say that is a bug.
I will see if I can reproduce a test case for this.

Thanks

I can't replicate the issue I was having with dropbuttons anymore, so that must have had another cause.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

So I've been debugging this in 9.5.10 and none of the patches above seem to identify the triggering element properly.

I tracked the issue down to the buttonWasClicked() method in FormBuilder. It specifically checks to see if the name and value of a button is found in the getUserInput() array, but buttons do not submit values. Their value is used as the text to display in the button, if if they are added to the getUserInput() array the value will be an empty string and never match to criteria for marking the button as clicked.

I also noticed that the key for any button on the form will only show up in the getUserInput() array if it was the button used to actually trigger the form submission.

I wrote a patch that adds another check to buttonWasClicked() that checks to see if a button's name is a key in the $input array, and if it is, it flags the button as the one that was clicked and results in the proper triggering element being flagged.

As a note: I know this needs some work, just wanted to get a patch posted to build upon.

I have updated the patch in 57 to fix an issue causing views apply/cancel/remove buttons to not submit properly.

The AJAX call _was_ adding a value to the "op" input, so the logic I had in 57 was flawed. We should only assume the input was clicked if the value matches exactly or if the value is exactly empty. Anything other than those 2 outcomes means we can't assume that button was clicked.

Further testing of both my patch and the previous patch has revealed that properly identifying the clicked button works in some scenarios, but not others.

In the case of my patch, it actually was breaking layout builder's submit buttons because they all have the same name- so disregard my patch.

Hi, is there a patch for d10 available?