See http://drupal.org/node/331708.

I have found an edge case on the Mollom subscription creation: when you wanted to change the product of your subscription, which is done through dynamically added submit buttons (i.e. #ahah-powered buttons added through an #ahah callback themselves), FAPI wouldn't detect this properly and wouldn't call the proper submit handler.

Switching to the method used in poll_choice_js() fixed this, but introduced two even worse edge cases, both of which can be considered core bugs:

1) #id's don't match "op"s
First drupal_process_form() is called, during which form_builder() gets called. This generates #ids for all buttons. And because #process callbacks are called too, form_expand_ahah() is called, which results in Drupal.settings.ahah getting updated.
Then, drupal_rebuild_form() is called, during which form_builder() gets called again. This regenerates the #ids for all buttons, but if for example a button now no longer exists (due to the rebuilt form), the mapping will no longer be correct. Which means when pressing an #ahah-enabled button A, button B may actually be pressed through JS…
proper solution: requires changes to core: if a form gets rebuilt, it should be able to remove any previous Drupal.settings.ahah entries that have been set through drupal_add_js().
work-around: set #id manually in your forms

2) AHAH update triggered by something else than a clicked button makes FAPI pretend the first form button was clicked
If an AHAH update was triggered by something else than the clicking of a button (e.g. the change event of a select), $_POST['op'] wouldn't be set. FAPI sees this as the "form with a single button in IE" problem, and thus it inappropriately pretends the first button in the form was clicked. And thus the wrong validate and submit handlers are triggered.
proper solution: make FAPI less ignorant
work-around: implemented in ahah_helper:

  // If the form is being rebuilt due to something else than a pressed button,
  // e.g. a select that was changed, then $_POST['op'] will be empty. As a
  // result, Forms API won't be able to detect any pressed buttons. Eventually
  // it will call _form_builder_ie_cleanup(), which will automatically, yet
  // inappropriately assign the first in the form as the clicked button. The
  // reasoning is that since the form has been submitted, a button surely must
  // have been clicked. This is of course an invalid reasoning in the context
  // of AHAH forms.
  // To work around this, we *always* set $form_state['submitted'] to true,
  // this will prevent _form_builder_ie_cleanup() from assigning a wrong
  // button. When a button is pressed (thus $_POST['op'] is set), then this
  // button will still set $form_state['submitted'],
  // $form_state['submit_handlers'] and $form_state['validate_handlers'].
  // This problem does not exist when AHAH is disabled, because then the
  // assumption is true, and then you generally provide a button as an
  // alternative to the AHAH behavior.
  $form_state['submitted'] = TRUE;
  // Continued from the above: when an AHAH update of the form is triggered
  // without using a button, you generally don't want any validation to kick
  // in. A typical example is adding new fields, possibly even required ones.
  // You don't want errors to be thrown at the user until they actually submit
  // their values. (Well, actually you want to be smart about this: sometimes
  // you do want instant validation, but that's an even bigger pain to solve
  // here so I'll leave that for later…)
  if (!isset($_POST['op'])) {
    // For the default "{$form_id}_validate" and "{$form_id}_submit" handlers.
    $form['#validate'] = NULL;
    $form['#submit'] = NULL;
    // For customly set #validate and #submit handlers.
    $form_state['submit_handlers'] = NULL;
    $form_state['validate_handlers'] = NULL;
    // Disable #required and #element_validate validation.
    _ahah_helper_disable_validation($form);
  }
CommentFileSizeAuthor
#1 rewrite.patch30.06 KBwim leers

Comments

wim leers’s picture

Status: Active » Fixed
StatusFileSize
new30.06 KB

Will be committed in a sec.

wim leers’s picture

Version: 6.x-1.0 » 6.x-2.0

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.