If some other module alters a form and sets $form_state['no_cache']=TRUE and Mollom has captcha on it, Mollom captcha never succeeds with error "The word verification was not completed correctly. Please complete this new word verification and try again."

function example_form_alter(&$form, &$form_state, $form_id) {
  if ($form_id == 'user_register') {
    $form_state['no_cache'] = TRUE;
  }
}

This problem removes from service a big chunk of core Form API functionality if Mollom is used and makes Mollom a bad citizen. It was not a problem in D6 (for $form['#no_cache']), judging by Mollom's D6 code that forced form to be not cached.

Comments

sun’s picture

Status: Active » Postponed (maintainer needs more info)

Can you explain why you're overriding and enforcing no_cache in your hook_form_alter()?

iva2k’s picture

distinctgrey’s picture

Is there any workaround available?
Same problem as iva2k, i need $form_state['no_cache']=TRUE but Mollom refuses to accept the word verification.

sun’s picture

Category: bug » support

Well, the Form API cache is - architecturally - not supposed to be disabled after a module enabled it.

We should probably remove the ability to disable it (once enabled) for D8.

For D7 (and below), Mollom module relies on the form cache as a storage that is unique and bound to 1) the current user, 2) the current form, 3) the current build of a form, and 4) retaining and maintaining security. Resembling all of that in custom code and a custom storage would be nuts. ;) That is, because the Form API cache was designed for exactly that job.

The no_cache option will not only break with Mollom module. it will also break any kind of module that adds #ajax functionality to affected forms, because #ajax equally relies on the Form API cache.

Long story short, Mollom module isn't doing something wrong. It is leveraging Drupal core APIs in exactly the way they have been designed for.

On the contrary, I yet have to see a module that cannot be made compatible with the Form API cache. Most often, it merely means to move processing logic from hook_form_alter() into a #process, or in most cases even better, #pre_render step.

In short, I've never seen a (valid) reason for force-disabling the Form API cache via no_cache.

iva2k’s picture

Maybe we need to employ some other method for Botcha, which basically makes all form builds unique, and therefore it needs a method that disables form reuse from the cache if for example validation fails.

The history of Botcha's "$form_state['no_cache'] = TRUE;" goes all the way back to Captcha in D6 - I looked how Captcha does unique captchas and done the same in Botcha. When doing D7 port that did break and I found elsewhere (maybe in Captcha? don't remember) the "$form_state['no_cache'] = TRUE;".

If someone can point me to a better implementation than this, I will be glad to change Botcha and close 3 linked issues (this, Botcha's Ajax incompatibility and Botcha's issue on Mollom compatibility) at once.

And last, it should be clearly documented in Form API documentation when it is ok and when it is not ok to change $form_state['no_cache'].

sun’s picture

Can you clarify some more what exactly Botcha tries to achieve (or work around) by disabling the form cache?

iva2k’s picture

@sun,
Botcha ensures that each form is built with unique protection fields so no two builds of the same form have the same protection. Botcha always blocks second submission of the form with the same build id. If validation fails, spambot scripts (or spambot writers) cannot keep guessing what are the entries should be to pass Botcha test. That method means that Form API should be prevented from pulling the form from the cache upon validation failure.

Does that make sense?

sun’s picture

Status: Postponed (maintainer needs more info) » Fixed

Well... if that's all... then:

Instead of force-disabling the form cache, that logic is as simple as this:

function botcha_form_alter(&$form, &$form_state, $form_id) {
  if (!empty($form_state['cache'])) {
    if (!empty($form_state['botcha_was_here'])) {
      form_set_error('', t('You are not allowed to preview a post, use Ajax, or submit a form twice.'));
      return;
    }
    $form_state['botcha_was_here'] = TRUE;
  }
  else {
    // ...do whatever you do if the form is not cached...
  }
}

In other words: Simply leverage the form cache instead of force-disabling it.

iva2k’s picture

Status: Fixed » Needs work

[EDITED] I don't think that the proposed code will fix the case when form validation fails and form is retrieved from cache and served again - Form API does not invoke hook_form_alter() in this case. It will bypass the code where Botcha replaces old recipe with a new one and the subsequent submission will trip Botcha's "no resubmit" rule that is always active.

sun’s picture

Status: Needs work » Fixed

Well, it depends on how exactly the form is processed and what exactly Botcha wants to do.

To run the code on every (re)build of the form, move the entire code from hook_form_alter() into a #process callback and only register that #process callback in hook_form_alter().

I think that's all advice I can provide right now... Given that Botcha's usage of no_cache breaks all #ajax and other entity form interactions, I'm a bit surprised that the problem space is entered from a Mollom-module-conflict perspective — AFAICS, the impact on much more common form/entity/ajax handling functionality should have triggered many other bug reports for Botcha already...?

Status: Fixed » Closed (fixed)

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

Anonymous’s picture

Issue summary: View changes

D6 behavior detail