I "think" the _form_button_was_clicked function logic is broken. Here is how I noticed the situation:

I have a form with multiple submit buttons one of which is a image_button and validation handlers are called for my image_button even though I click on other buttons.

Here is what I observed while tracing through the code (I'm a newbie to Dupal so my comprension could be all wrong). The _form_button_was_clicked function (commented code is bellow) is called for every button on my form. The function returns true in the case of an image_button even when it is not clicked.

// Note that the parameter name should be something like $element not $form

function _form_button_was_clicked($form) {
  if (isset($form['#post'][$form['#name']]) && $form['#post'][$form['#name']] == $form['#value']) {
    return TRUE;
  }
  elseif (!empty($form['#has_garbage_value']) && isset($form['#value']) && $form['#value'] !== '') { 
    // wrong. I think you have to test for isset($form['#post'][$form['#name']])
    // other conditions are probably useless (and even wrong)
    return TRUE;
  }
  return FALSE;
}

// Here is the logic I would have used

function _form_button_was_clicked($form) {

  if (isset($form['#post'][$form['#name']])) {
     if($form['#post'][$form['#name']] == $form['#value']) {
        return TRUE;
     }
     if(!empty($form['#has_garbage_value'])) {
       return TRUE;
    }
  }
  return FALSE;
}
  • Please note that I have not tried this (I'm away from work).
  • Note that the original logic (nor this one) does not support arrays of submit buttons (could be an enhancment). For example, if I want to have a delete button for every row of a rowset, I could name my buttons "delete[1]", "delete[2]". In this case the buttom name would be "delete[1]" while the post formed name would be an array and comparison would fail.

Comments

damien tournoud’s picture

damien tournoud commented
Status: Active » Postponed (maintainer needs more info)

Due to the logic of image button posting (the browser will post the (x,y) coordinates where the user clicked, not the value parameter), you simply can't use the same name for two image buttons or for one image button and a button.

I see no flaw in the original code, so I guess you *are* using the same name ('op' by default) for two image buttons or for one image button and a true button.

cquezel’s picture

cquezel commented

I completly edited this post because I now understand things a little better. The original problem remains:

If I have a form with a mix of image_buttons and submit elements, when I click on one of the submit elements, this element is flagged as having been clicked and so are all the image_buttons whose ‘#value’ properties is set (all names being different). If I do not set the ‘#value’ property in image_button elements all works fine.

Basically there are three solutions:
1) Alert the programer that he must not set the '#value' property of image_buttons (it can't be used anyway).
2) Fix the code so that even if he sets this '#value' property, the code does not break.
3) both 1 and 2.

The first solution is trivial:

if (!empty($element['#has_garbage_value']) && isset($element['#value']) && $element['#value'] !== '') {
  // warn somehow here
}

For the second solution, I thought that just writing:

function _form_button_was_clicked($element) {

if (isset($element['#post'][$element['#name']])) {
	if ($element['#post'][$element['#name']] == $element['#value']) {
		return TRUE;
	}
	if (!empty($element['#has_garbage_value'])) {
		return TRUE;
	}
  } 
  return FALSE;
}

would fix the problem but this introduces other problems elsewhere (like not behaving the same on IE and FF).

And contrary to what I posted originally, array seem well supported.

damien tournoud’s picture

damien tournoud commented
Status: Postponed (maintainer needs more info) » Closed (duplicate)

This is a duplicate of #206955: image_button: clicked_button value is wrong. The short answer is: don't use #value value in your image buttons.

Sid_M’s picture

Sid_M commented

Having just fought with this for a while, and not having found clear documentation, I'm adding what seem like the key points for using multiple image_buttons in a form:

1) Do give each image_button a unique #name (unlike regular buttons, you can't have several image_buttons all named op).
2) Don't give an image_button a #default_value.
3) Don't give an image_button a #value
4) Do give each image_button a #return_value. This is the same as what you would assign to #value if IE6 problems hadn't forced Drupal to implement some unusual element handling. If you don't assign this, it will default to true.
5) Do use $form_state['clicked_button']['#value'] to find the #value of, well, the clicked image_button. This field will contain the value of #return_value for the clicked image_button.