The new mollom uses mollom_form_list to do per-form configuration and implement pluggability.

However the id shouldn't be the primary entity for this.
In case of webform.module we have N nodes - each with its own form_id. See

function webform_mollom_form_list...
    $form_id = 'webform_client_form_' . $node->nid;

With webforms you might have 100s of forms on a real platform.
For all mollom-configured form we'll execute a SELECT to fetch all per-form-id settings - here overhead might be huge. In current hook_form_alter the protected_forms is currently not being added to any persistent cache.

Also current situation is not friendly for site users. When normal users can add forms, they should also be able to set options about spam protection on a per form basis - without directly impacting system performance.

Finally - we'd like to define a rule named enable mollom for "all webforms that allow anonymous submission".

If we'd allow mollom to be activated on a per-form setting such as $form['mollom_tag'] = 'webform_anonymous' we would be able to define settings for webform_anonymous instead of the real form_id. Thus not all form_ids need per-form settings and {mollom_form} would be much smaller. It would be possible to build form categories to be configured as a group.

Is this API still open for update / refactoring?

CommentFileSizeAuthor
#16 mollom_base_form_id_16.diff2.54 KBLukyLuke_ch
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

miro_dietiker’s picture

BTW: Originally we've introduced questions about webform and mollom integration.
See there for the original case about general webform spam protection instead of per-form mollom.

#768872: Provide direct Mollom and CAPTCHA module support to allow authors to enable instead of administrators

Dave Reid’s picture

Are you referring to the most recent code with the two separate hooks?

miro_dietiker’s picture

I'm referring to DRUPAL-6--1 and checked the cvs again to be sure.

mauritsl’s picture

Webforms is not the only module which uses dynamic form id's, I thought Simplenews also do.

Supporting regexp for form id's would be an option, although not very user friendly. But another solution for such modules won't be easy..

miro_dietiker’s picture

Sure there are solutions, mauritsl - we need an fapi property to address/tag a mollom namespace.
Solution one:

// controle based on custom captcha id:
$form['captcha'] = array(
  'id' => 'webform_anonymous',
);
// hook_captcha_id to register captcha ids
function webform_captcha_id() {
  return array('webform_anonymous');
}

All Webforms that are anonymous submittable have this captcha id.

It might be better to tag forms so we're able to define overlapping rules (a webform might be anonymous submittable and is)

$form['captcha'] = array(
  'tag' => array('webform_anonymous', 'someother'),
);

All Webforms that are anonymous submittable have the tag webform_anonymous. There could be other tags, whatever general rule makes sense to controle captcha appearance.

Caveat is that we don't know which fields are present in the tag context.

Thinking about how this setup could be simplified:
We might introduce alternatively something like a "form type" (which might be "node" or "webform" or "simplenews subscribe") to configure them as a collection. So additionally we might choose form types in mollom instead of form ids.

Any further opinion?

mauritsl’s picture

I thought of regexp cause it doesn't require any changes to modules which generates the forms (like webforms). But a captcha id is - in my opinion - a good way to go.

Tagging is not very useful I think. Handling the difference between authenticated and anonymous users is already in the captcha modules. Duplicating this in the modules which generates the forms is overhead. Only one captcha id per form is enough to configure your captcha module. Adding multiple tags also will clutter the captcha module configuration pages (see issue from the post above :-).

The name of the array element (in the form array) should start with "#", since all items with a name starting with "#" are ignored in the form generation. Captcha modules are able to check for this value in the form_alter hook. The best name for this is - imo - "#captcha_category" (just "#captcha" does not exactly describe what it is for). When generating forms all you have to add (for integration with captcha modules) is:

$form['#captcha_category'] = 'webform';

All captcha modules have to implement this feature. They have to support this in addition to form_id's (for both legacy and to be able to enable antispam for a specific form (i.e. a specific webform, which has a nid in the form id). We should open issues on all captcha modules to have this implemented. I think it is a good idea to do so, but some more opinions would be highly appreciated.

Dries’s picture

Note that the API can be changed, assuming that we can come up with an elegant solution. I'm not a big fan of using a regex for this.

mauritsl’s picture

@Dries: Did you mean the Mollom module API or Form API?

I agree regex is not a wise idea, too difficult for an average user. But I came up with another solution in comment #6, the #captcha_category. What are your thoughts on that?

miro_dietiker’s picture

Mauritsl - kidding?
Sure we're talking about modification of mollom module API.
Changing fapi would be subject of Drupal 8...

Your "#captcha_category" is the same concept as my suggested "form type".
Note that we still need to announce available form types / captcha categories to make them configurable.

In the API i see that there's an "entity" for each form_id. This entity field might be identical to a "form type" or whatever you call "#captcha_category". We might be able to change the code to additionally allow per-entity configuration.
(See mollom_form_list in mollom.module. BTW: Shouldn't we cache this result [cache_set.. cache_get]?)

I'm still looking for a list of samples of categories to get some understanding of real live usecases and to be sure to have reasons enough to redefine the mollom api for a next stable featureproof api. We should start a use case collection as a wiki in the captcha group.
http://groups.drupal.org/capcha
See my current wiki entry:
http://groups.drupal.org/node/64978

Feel free to brainstorm overthere.

mauritsl’s picture

Status: Active » Postponed

(good idea to add this in D8)

The #captcha_category surely is the same concept as the captcha id, but using the name "id" for this would be incorrect.

Good to have this in the wiki. I will think about other use cases as well.

Marking this issue as postponed until we have an elegant solution.

sun’s picture

Status: Postponed » Active

I have troubles in understanding the original/underlying assumptions in this issue.

There are two integration types:

1) Single/dedicated, programmed forms, such as the comment form (D6) or the contact form.

2) Forms that can be configured and spawned into various instances, such as the node (type) form or webforms.

The first type of form always contains the same form elements/fields, the same structure of submitted values, and only some other business logic in the background, unrelated to the form submission itself, changes. Access permissions, to submit the form and to edit or delete the generated entity afterwards, keep the same, and such forms usually use a single $form_id for any possible instance of the form.

The second type is based on a pseudo-form and each form belongs to a certain "class" or configuration, i.e., the actual form elements/fields contained in the form depend on the class/type/configuration. This means that each form can be entirely different from class to class, although it still belongs to the same base form.

Since Mollom needs to know, which particular fields of the form should be analyzed/validated during textual analysis, you cannot really "protect all forms that can be submitted by anonymous users", since all these forms are different.

Most likely, this is where the Mollom module differs from other CAPTCHA modules; for a captcha-only protection, you do not necessarily need to know about the form elements/fields in the form, as you merely check whether the response for the CAPTCHA was correct.

For me, this issue does not contain any enhancement idea that would make sense and be doable, and I can't think of one myself. I'd therefore suggest to close as won't fix.

miro_dietiker’s picture

If however the exact field definitions are so important to mollom we need the other direction of integration:

A user creating a webform should be able to configure mollom directly in webform.
(same as a user could configure mollom captcha in the nodetype - instead of configuring it using the mollom config page.)

Captcha setup is a user task in user generated submittable entities - in worlds where form definition are data instead of system setup settings.
So we need mollom to be that kind of integration-friendly.

But still, if we have >1000 forms, the mollom backend implementation should NOT ask using a hook to enumerate all forms. This leads to huge dropdowns und un-usable interfaces.

I'm lucky other participants have understood this user need.

sun’s picture

Title: overhead from form_id in mollom_form_list » Administration incapable of handling large amounts of (sub-)forms of a module
Category: feature » task
Status: Active » Postponed (maintainer needs more info)

If however the exact field definitions are so important to mollom we need the other direction of integration:

A user creating a webform should be able to configure mollom directly in webform.
(same as a user could configure mollom captcha in the nodetype - instead of configuring it using the mollom config page.)

I think that Webform has sufficient ways to change its own implementation already, so as to allow users to configure fields being checked/validated against spam on a per-field basis.

But still, if we have >1000 forms, the mollom backend implementation should NOT ask using a hook to enumerate all forms. This leads to huge dropdowns und un-usable interfaces.

1) Why not? I'm currently not sure whether we implemented a pager, but that would be easy to do. Views integration using exposed filters perfectly doable, too.

2) I can see the point of a select form widget containing 1.000 options being cumbersome. So you are effectively proposing a new 'hidden' property for mollom form definitions that may be consumed by other modules?

Lastly, trying to improve this issue's title.

mauritsl’s picture

And what if you want to enable / disable Mollom on all 1000 webforms? Will you do that by hand?

Another option may be to implement actions in webforms module to enable / disable Mollom. Still not a solution which will also work for other captcha modules.

sun’s picture

Status: Postponed (maintainer needs more info) » Active

AFAICS, we have a few separate issues here:

1) There should be a pager on the form overview page.

2) If you have 1,000+ webforms (or similar), then the select list on the "Add form protection" form/page will likely #fail. It might be necessary to allow modules to mark their sub-forms as 'hidden', or we just need to implement optional integration with one of the better select user interface modules, such as http://drupal.org/project/hierarchical_select - i.e., if you experience this bug, you can simply install one of those modules to solve it, but for the other 99.9% of users, we don't need to do anything special.

3) Modules should be able to protect all forms that use a shared base form constructor; such as node_form() for any node type form, and webform_client_form() for any webform. Implementing support for handling shared base form IDs in addition to normal form IDs should not be hard. However, a real problem is that all of those sub-forms are vastly different and we don't know which additional fields should be sent for text analysis, unless the base form exposes the field already (so it's guaranteed to exist on all sub-forms). But what would be possible for shared base forms is a CAPTCHA-only protection, because that does not care for the actual form fields.

LukyLuke_ch’s picture

Issue summary: View changes
FileSize
2.54 KB

Currently we need the third case in one project to protect all forms with the same base_form_id. Not webforms but others (salsa_signup_page_form).

This can be done relatively easy with something like in the attached patch:
Just check for the base_form_id too in mollom_form_alter and use that one instead.

miro_dietiker’s picture

I postponed the patch at salsa_entity till this is in as we would end up with having many sites patched...
#2193829: Use hook_forms() to support multiple signup forms on the same page