Default validate/submit handlers not added if any form-level validate/submit handlers added in hook_field_attach_form()

Aye and then you would override the defaults how?

@chx: If the default function exists it should always be added if it isn't already, e.g. changing this:

  if (!isset($form['#validate'])) {
    // Ensure that modules can rely on #validate being set.
    $form['#validate'] = array();
    // Check for a handler specific to $form_id.
    if (function_exists($form_id . '_validate')) {
      $form['#validate'][] = $form_id . '_validate';
    }
    // Otherwise check whether this is a shared form and whether there is a
    // handler for the shared $form_id.
    elseif (isset($form_state['build_info']['base_form_id']) && function_exists($form_state['build_info']['base_form_id'] . '_validate')) {
      $form['#validate'][] = $form_state['build_info']['base_form_id'] . '_validate';
    }
  }

to this:

if (!isset($form['#validate'])) {
  // Ensure that modules can rely on #validate being set.
  $form['#validate'] = array();
}
// Check for a handler specific to $form_id.
if (function_exists($form_id . '_validate') && !in_array($form_id . '_validate', $form['#validate'])) {
  $form['#validate'][] = $form_id . '_validate';
}
// Otherwise check whether this is a shared form and whether there is a
// handler for the shared $form_id.
elseif (isset($form_state['build_info']['base_form_id']) && function_exists($form_state['build_info']['base_form_id'] . '_validate')) {
  $form['#validate'][] = $form_state['build_info']['base_form_id'] . '_validate';
}

Here's a patch following my idea from #7 above applied to both #validate and #submit.

I think what chx is asking is how to replace a default submit handler, rather than add another one. The answer would be hook_form_alter , but then you're asking those that want to replace a submit handler to do the same workaround as you're currently doing to add one.

Really, it would be much handier if the #validate/#submit attributes is already set when hook_field_attach_form is invoked, but that moves the responsibility to the original form builder (which might come from who knows where) or field_attach_form (which is an odd place).

@Xen/@chx: the patch above doesn't do anything strange and completely allows you to override the default validation/submit handlers, it only adds the default ones to the process list if the functions actually exist.

Lets turn this around: please explain why the patch does not provide what most people would expect to happen.

@DamienMcKenna: It changes how you can override the default handler, in the slightly odd case of hook_field_attach_form() (and possibly others) where you have third party code that get the form handed over *before* drupal_prepare_form has processed it. Before the patch you simply set the attribute in the relevant function, after the patch you will have to add an form_alter to do it (which seems odd, as your module was already handed the complete form once).

Not saying it is the right behavior now, but it is a change of behavior, and I wasn't as surprised over this as Dave obviously was, as I thought it was a known thing. We have the same kind of issue with #element_validate, which is not available in form_alters, as it's added later in form_builder(). That's another case where you currently have to add in the default explicitly, if you want it, because it will only be added if not set explicitly.

What we have now is "defaults is applied if nothing else has set it", with the patch it's "the defaults are always set". Not saying it shouldn't be changed, but then #element_validate should be fixed to (which is not as trivial), or it will be confusing as hell that two handlers that's so alike, behave very differently.

@Dave Reid: I don't think that's a good reason to diverge them further.

Needs a re-roll, and a non-suffixed patch.

Here's a straight reroll.

[ignore]

Ignore me don't know what I was thinking.

Back to needs work as #16 still failed.

Had a quick look at the test failures. Looked at the block availability test and the problem is quite obvious.

Previously, you were able to override the default behavior by adding an explicit submit callback. node_form() for example does some crazy things by adding the generic submit callback on the submit button and then maybe add a node type specific one and always initialize #submit to an empty array to prevent the default behavior.

Because what happens now is that the same submit callback is re-added globally on the form, and because that one does also execute global submit callbacks, for some weird backwards compatible thingy, this means we're running into an endless loop.

This would be quite a change, not sure if that qualifies as bug fix and certainly not something that's backportable. The only backportable bugfix for this problem that I can think of is adding the same check at the beginning of field_attach_form() to make sure that submit/validate callbacks are initialized at that point. I've been running into the same problem with privatemsg and organic groups a while ago.

Is this still relevant? Our entity forms have changed a lot.

@tim.plunkett: Maybe it should be changed to be just a D7 issue then?

Yes, if its not relevant that's what would happen.

Now form action handlers are consistently attached at button level, which is the recommended way also for attaching custom handlers. I don't think this is still an issue in D8 and would be ok to moving it to D7. OTOH if we change the logic in drupal_prepare_form(), we should do that consistently in both branches.

Fixing PHP tags

Back to D7 as per #22, #23, and #25.

Well, I just wasted half a day wondering why my form_alter submit/validate handlers weren't being called.

I can't imagine how much pain this has caused developers. Shouldn't this be a priority rather than a 4 year old bug?

This patch makes a small change to form_execute_handlers() to include both sets of submit/validate callbacks.