Elements with #has_garbage_value (image buttons) are always set as a triggering element

Looks similar to

http://drupal.org/node/1342066

The workaround is to get remove #value property of image_button. If it exist, there is a bug in the Drupal form processing, specifically in the _form_button_was_clicked() function: According to this function, every element with non-empty '#has_garbage_value' property (like the image_button) and with non-empty #value is considered to be a "triggering element". If there are multiple elements of that kind in the form, the last one is picked up.

function _form_button_was_clicked($element, &$form_state) {
  // First detect normal 'vanilla' button clicks. Traditionally, all
  // standard buttons on a form share the same name (usually 'op'),
  // and the specific return value is used to determine which was
  // clicked. This ONLY works as long as $form['#name'] puts the
  // value at the top level of the tree of $_POST data.
  if (isset($form_state['input'][$element['#name']]) && $form_state['input'][$element['#name']] == $element['#value']) {
    return TRUE;
  }
  // When image buttons are clicked, browsers do NOT pass the form element
  // value in $_POST. Instead they pass an integer representing the
  // coordinates of the click on the button image. This means that image
  // buttons MUST have unique $form['#name'] values, but the details of
  // their $_POST data should be ignored.
  elseif (!empty($element['#has_garbage_value']) && isset($element['#value']) && $element['#value'] !== '') { // <== HERE
    return TRUE;
  }
  return FALSE;
}

I'm currently trying this fix:

- elseif (!empty($element['#has_garbage_value']) && isset($element['#value']) && $element['#value'] !== '') {
+ elseif (!empty($element['#has_garbage_value']) && isset($form_state['input'][$element['#name'] . '_x']) && isset($form_state['input'][$element['#name'] . '_y'])) {

So the current workaround is to not use the #value property. However this property is allowed in the FAPI docs and thus this is kind of a Drupal WTF - an unexpected behavior.

Fix for D7. I'll post D8 patch later. It seems that FAPI doesn't change in that case.

The first patch just extends the test and is expected to fail.

Title correction.

The code looks correct. Why do you except it to behave differently?

Patch against D8. The first patch file just extends the test module and is expected to fail.

See form_type_image_button_value(), that transform the (kind of browser-specific) submit (as far as I know you can find both element_name and element_name.x, element_name.y, the latter actually being the HTML5 pick). As far as I understand, this code is perfectly correct. If an image element has a value, it is considered to be submitted.

The code looks correct. Why do you except it to behave differently?

Maybe I've explained it wrong. The current - buggy - behavior of Drupal form processing is unexpected, because the "#value" is allowed property according to API, but the _form_button_was_clicked function doesn't count with that...

If an image element has a value, it is considered to be submitted.

Ok, I see your point. You're right, but it is not very DX friendly as the rest two types of "buttons" have that property. It is confusing. It happend to me today and to many people before (according to this and some other issue queues), plus there is no single word about that behavior in the Forms API.

So at least this is a bug in the documention and (IMHO) DX issue.

This is very similar to #873070: When an image button appears after another button in a form, the wrong triggering element and #submit handlers are detected, which is older. Both have patches (with similar code changes in them), although this one is farther along in some ways and the other is farther along in other ways.

Should we close this issue and consolidate all work over there?

Ok, I see your point. You're right, but it is not very DX friendly as the rest two types of "buttons" have that property. It is confusing. It happend to me today and to many people before (according to this and some other issue queues), plus there is no single word about that behavior in the Forms API.

I agree that this is confusing. But the real problem is that we are using #value for submit and button in the first place. We should really be #default_value or similar in that case...

One simple thing we could do (and backport) is to use #value as the value of the alt="" attribute for image buttons. That way the behavior of image buttons is the same as other types of buttons (aka. #value is the textual representation of the button).

Is this still a point of confusion? Should the issue be revived for Drupal 9?

This might be a duplicate of #2546700: Add support for detecting the triggering element when buttons are changed client-side?

Thank you everyone for this.

Closing as outdated as there hasn't been a follow up for #22

If still a bug please reopen updating issue summary for D10

Thanks!

This is absolutely still a bug in Drupal 11.

Button Field module documents:

Important note

Due to #1452894: Elements with #has_garbage_value and #value are always set as a triggering element you can not use image buttons on edit forms when there are any other buttons. Non-editable displays are unaffected.

And where i'm coming from, i confidently told Gin in #3539394: Image buttons cannot be kept in Gin's sticky form actions that core's image button form element is a valid alternative to a standard submit input form element:

As noted in #3359619: UX when using gin with Content Moderation and Workflows, Workflow Buttons module pairs very well with Gin for improving the experience of using content moderation workflows, and the 2.0.x branch of Workflow Buttons' "Trash workflow" submodule switches to using the image button to have a trash icon for the soft-delete, similar to the trash icon Claro/Gin add to core's delete link. […]

But in short swapping the submit button out and substituting an image button for it works, and Gin should support this core form element when adding Gin attributes and looking for the #gin_action_item flag.

And i get it looking perfect and then discover that no matter which button i press, the image button (soft delete!) is the one that gets incorrectly identified as the trigger button.

And it is not a duplicate with changing values client side; although that may be related, this is being caused with alteration in a Drupal form/widget alter.

Essentially the fix by wojtha in this issue 13 years ago and richthegeek and cvangysel in #873070: When an image button appears after another button in a form, the wrong triggering element and #submit handlers are detected 14 years ago remains correct and needed.

When image buttons are clicked, browsers do not set NAME in the POST, instead setting NAME_x and NAME_y (with x and y the coordinates of the click). To get the clicked button we need to test for these parameters instead of the presence of value— that is, equally . (Damien's suggestion of using #value for image buttons also does not work, whether the button is pressed or not, whether it has a value or not, the value gets set anyway, and set to "true", presumably because of how image buttons are treated by the browser— the only thing that is safe to check are if coordinates were set.)

And yes (replying to 13 year old comments from another issue now, but the same feedback will come up) these changes make the alternative condition specific to image buttons, but i'm fairly certain that is because that is all there is— the only other element in core which uses #has_garbage_value is Radios, which aren't ever going to be triggering a form submission.

So, yeah, image buttons are ridiculous and Drupal core could totally deprecate them and not support them since basically nobody has cared for a decade (especially if we finally do #1671190: Use <button /> form element type instead of <input type="submit" />), but as long as we have ImageButton extending Submit we need to support it properly as an alternative to Submit, that works alongside Submit buttons.

The merge request is ready and works, but leaving Needs work as it needs a test showing that this fails even when value is not set in the initial form element definition. As my use case is altering an existing button there is some possibility that something about that is adding 'value' back, but definitely unsetting value is not enough.