When two buttons are added to a form, one vanilla HTML button and one image button, $form_state['triggering_element'] is always the image button regardless of which button is clicked. If both buttons are HTML buttons then the triggering element is set properly. If both buttons are image buttons, the last button in the form will be the triggering element. This is true when using both ajax and normal submit.

In order to demonstrate, I have created a content type called "test" and used the following code for the buttons and their ajax callbacks.

<?php
/**
* Implements hook_form_FORM_ID_alter().
*/
function mymodule_form_test_node_form_alter(&$form, &$form_state, $form_id) {
 
$form['html_button'] = array(
   
'#type' => 'button',
   
'#value' => 'HTML button',
   
'#ajax' => array(
     
'callback' => 'mymodule_button_html_callback',
     
'wrapper' => 'mymodule_replace',
    ),
  );
 
$form['image_button'] = array(
   
'#type' => 'image_button',
   
'#value' => 'Image button',
   
'#src' => drupal_get_path('module', 'mymodule') . '/image.jpg',
   
'#ajax' => array(
     
'callback' => 'mymodule_button_image_callback',
     
'wrapper' => 'mymodule_replace',
    ),
  );
 
$form['#prefix'] = '<div id="mymodule_replace">';
 
$form['#suffix'] = '</div>';
}
/**
* Callback function for the HTML button's ajax event.
*/
function mymodule_button_html_callback(&$form, &$form_state) {
 
drupal_set_message('HTML button');
  return
$form;
}
/**
* Callback function for the image button's ajax event.
*/
function mymodule_button_image_callback(&$form, &$form_state) {
 
drupal_set_message('Image button');
  return
$form;
}
?>
Files: 
CommentFileSizeAuthor
#6 1452894-5_trigerring_element_fix_test_fail.patch645 byteswojtha
FAILED: [[SimpleTest]]: [MySQL] 46,189 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#6 1452894-5_trigerring_element_fix.patch1.35 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 46,183 pass(es).
[ View ]
#3 1452894-3_trigerring_element_fix_test_fail.patch564 byteswojtha
FAILED: [[SimpleTest]]: [MySQL] 39,546 pass(es), 3 fail(s), and 0 exception(s).
[ View ]
#3 1452894-3_trigerring_element_fix.patch1.25 KBwojtha
PASSED: [[SimpleTest]]: [MySQL] 39,535 pass(es).
[ View ]

Comments

Title:Image button is always the triggering elementElements with #has_garbage_value and #value are always are always set as triggering element
Version:7.12» 7.x-dev
Issue tags:+DrupalWTF

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.

<?php
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.

Status:Active» Needs review
StatusFileSize
new1.25 KB
PASSED: [[SimpleTest]]: [MySQL] 39,535 pass(es).
[ View ]
new564 bytes
FAILED: [[SimpleTest]]: [MySQL] 39,546 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

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:Elements with #has_garbage_value and #value are always are always set as triggering elementElements with #has_garbage_value and #value are always set as a triggering element

Title correction.

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

Version:7.x-dev» 8.x-dev
StatusFileSize
new1.35 KB
PASSED: [[SimpleTest]]: [MySQL] 46,183 pass(es).
[ View ]
new645 bytes
FAILED: [[SimpleTest]]: [MySQL] 46,189 pass(es), 3 fail(s), and 0 exception(s).
[ View ]

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?

Status:Needs review» Needs work

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).