normally things like node_form_validate get ran on node_save. _form_validate calls form_execute_handlers with $type set to "validate". The issue is mb_content_changed_validate gets put in the $form_state['validate_handlers'] array and everything else inside of the $form['#validate'] array does not get executed due to the way form_execute_handlers() works.

The solution to this is to make the validation and submit callbacks smarter and apply this globally instead of to each button.

This is a bad idea.

function mb_content_form_alter(&$form, &$form_state, $form_id) { 
...
  switch ($form_id) {
...
    case $node_type . '_node_form':
...
        // Define the "Save and continue" form element.
        $form['actions']['sac'] = array(
          '#type' => 'submit',
          '#value' => isset($mb_content_values['sac']) ? t('@sac-value', array('@sac-value' => t($mb_content_values['sac']))) : t($mb_default_values['sac']),
          '#weight' => $weight_sac,
          '#validate' => array('mb_content_changed_validate'),
          '#submit' => array('node_form_submit', 'mb_content_sac_submit')
        );
...
      /**
       *  Extra validation for the standard buttons.
       *  Without this break the normal form handling.
       *  @see mb_content_cancel_validate(), mb_content_changed_submit()
       *  Call edit form from admin/content is no needed this callbacks.
       */
      if ($settings['sac'] > 0 || $destination['destination'] != 'admin/content') {
        $form['actions']['preview']['#validate'][] = 'mb_content_changed_validate';
        $form['actions']['submit']['#validate'][] = 'mb_content_changed_validate';
        $form['actions']['delete']['#validate'][] = 'mb_content_changed_validate';
        // Corresponding submit callbacks.
        $form['actions']['preview']['#submit'][] = 'mb_content_changed_submit';
        $form['actions']['submit']['#submit'][] = 'mb_content_changed_submit';
      }
...
}
CommentFileSizeAuthor
#9 mb-fix-validation-hooks-1414898-9.patch3.93 KBronino
#7 mb-1414898-7-fix-validation-hooks.patch817 bytestheapi
#1 mb-1414898-1-fix-validation-hooks.patch3.61 KBmikeytown2

Comments

mikeytown2’s picture

mikeytown2 commented
Status: Active » Needs review
StatusFileSize
new mb-1414898-1-fix-validation-hooks.patch3.61 KB
pglatz’s picture

pglatz commented

Thanks for the fix Mike - I was at my wits end trying to figure this one out, and finally discovered how the validation hooks weren't being called. This saved me the time of writing my own patch.

Hmmm, this has been around for five months and hasn't been fixed? I love this module, it's very useful - and now that it doesn't ignore my validators, I'm very happy

mikeytown2’s picture

mikeytown2 commented
Status: Needs review » Reviewed & tested by the community
Renee S’s picture

Renee S commented

Another vote here. Thanks so much mikeytown2. Several hours of many bad words were said until I found this thread. You've saved my neighbours' delicate ears.

Renee S’s picture

Renee S commented

Also, maintainers, this is a year old now. Consider committing? :)

jonraedeke’s picture

jonraedeke commented
Priority: Normal » Major

I think that validation not running is a biggie. Patch works for me. Thanks mikeytown2!

theapi’s picture

theapi commented
Component: Documentation » Code
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new mb-1414898-7-fix-validation-hooks.patch817 bytes

Since "Save & Continue" is a multi-part form it may not be wise to have the normal validation before the form is formally submited, which may be the reason for taking over the validation.

However the form alter does put it's overriding validation functions on all node forms whether mb buttons are used or not. That's what's killing node validation on other forms.

Here's a simple patch that changes the relevant line from
if ($settings['sac'] > 0 || $destination['destination'] != 'admin/content') {

to
if ($settings['sac'] > 0 || $settings['cancel'] > 0) {

ronino’s picture

ronino commented
Priority: Major » Critical

#1 works fine and should be committed rapidly as a myriad of users out there are like me seeking for the reason why their form handlers are not run. I just found this by accident and would never have thought that a contrib module causes this issue.

ronino’s picture

ronino commented
StatusFileSize
new mb-fix-validation-hooks-1414898-9.patch3.93 KB

I tested a bit more and noticed that I get this error when saving a node after applying #1:

Notice: Undefined index: nid in mb_content_changed_submit() (line 406 of /path/to/drupal/sites/all/modules/contrib/mb/mb_content/mb_content.module).

My changes were not saved. Instead I was redirected to /node and got the above message only.

I attached an enhanced patch from #1 which fixes this issue. I didn't incorporate #7 as I couldn't test that scenerio.

seanr’s picture

seanr
he/him
commented
Title: Form validation hooks are not ran » Form validation hooks are not run

Patch in #9 appears to work for me. I concur with the critical status of this bug as it prevents all hook_node_validate implementations from running, including the conflict module, for example. I am continuing to evaluate this patch, but so far it appears to be working.

seanr’s picture

seanr
he/him
commented

I now have been added as a maintainer for this, so if we can get a couple more reviews of this patch, I'll get it committed.

  • 6733c76 committed on 7.x-1.x 
    Issue #1414898 by Ronino: Form validation hooks are not run
seanr’s picture

seanr
he/him
commented
Status: Needs review » Closed (fixed)

Fixed in dev. Will keep testing a bit more before making a release; expect that in a day or two.

seanr’s picture

seanr
he/him
commented

Just updated the commit with proper attribution. Sorry about that, mikeytown2. :-)