Jump to:

I've added a hook_captcha_placement_alter() that can be used to customize placement of captcha elements. This may be useful in situations where the form is complex and the captcha module's default placement needs to be overridden.

I've also added documentation showing how to add a CAPTCHA to a webform using the forms API. This would address the issue raised in Document how to add a CAPTCHA programmatically, but since my patch is against Drupal 7, I'm creating a separate ticket here for it.

Cool! Thanks for the work. Also nice you thought to update the API documentation :)

One remark: you placed the hook_alter trigger after caching of the placement map (variable_set('captcha_placement_map_cache', $placement_map);). Any reason for this? Wouldn't it be better for performance to cache the altered placement map?

Note that I already provided place to put the hook:
around line 245 in captcha.inc:

// This is the place to (pre)fill the placement map with some hard coded default entries. // However, probably all Drupal core forms are correctly handled with the best effort // guess based on the 'actions' element (see below). So not much to here at the moment.

// TODO: provide a hook here so other modules can inject placements here? // TODO: also make the placement 'overridable' from the admin UI?}?>

Hm, well for starters the way you're caching the placement map is by setting a Drupal variable, which means it is a cache that never expires, and there is no settings page or other hook that enables it to be altered. This means that if the captcha module sets a value for a particular form ID, thereafter it will always skip over the place in your code where you are suggesting inserting a drupal_alter function. Effectively, therefore, it will still be impossible for the hook to modify the placement once the captcha module has defined it.

Also, the hook I wrote modifies placement for a single form ID, whereas placing the hook in the location you suggested would require modifying the entire placement map. I think in practice people who want to have this kind of hook will likely only want to modify placement on one or two forms on their website, so modifying the $placement array rather than the $placement_map array seems to me like an easier hook to understand and implement.

About the caching: there is a button on the CAPTCHA settings page to flush the placement cache, precisely for this (see attachment).
The caching is there because CAPTCHA placement is not something that changes very often once a website is set up. It would be a waste to do all the form array traversing/processing every time for every CAPTCHA protected form, when the result is probably the same as last time.
Conceptually speaking, I think that the result of placement altering by other modules should be included in this caching.
You might have a point that cache invalidation is currently not optimal (only a button on the settings page). Alternatives: invalidate it when new modules are enabled/disabled, always invalidate it when there is CAPTCHA configuration activity (e.g. the CAPTCHA settings form is submitted), ...

Also, the hook I wrote modifies placement for a single form ID, whereas placing the hook in the location you suggested would require modifying the entire placement map. I think in practice people who want to have this kind of hook will likely only want to modify placement on one or two forms on their website, so modifying the $placement array rather than the $placement_map array seems to me like an easier hook to understand and implement.

Hm, that's interesting. I didn't understand how your caching worked. I guess I didn't read the settings page very closely. Also, when I see the word "caching" I think of Drupal's cache tables and expect to be able to clear caches using "flush all caches," "flush theme cache," etc. I wonder if it would be better to use a phrase such as "reset to defaults" to avoid confusion. In any case, I see the logic to your suggested changes. Do you want to reroll the patch, or should I?