mollom_field_extra_fields() only works for nodes, needlessly loads $mollom_forms

wow, what a mess.

If it is such a mess, does it need to be removed altogether? Or what needs to be done so we don't have to guess?

It sounds like a reasonably common use case to me so it is hard to believe Mollom is the only module that has this problem.

Hm. Now I see that http://api.drupal.org/api/function/hook_field_extra_fields/7 does not even get those arguments I mistakenly thought it would get...

Problem space being that

1) entity modules registering their _forms_ for possible protection with Mollom

2) user enables/configures any of those registered entity forms

3) Mollom appears on the entity forms as extra field, but only because we integrate with the form_id

4) There is no reverse-relationship from entity -> bundle -> form_id...

On a second thought. Let's try this. Gotta get some dinner now, will come back to this later.

I'll postpone testing this given that it has a @todo and per the comment in #6 it might just be a work in progress.

Resolved that @todo

um, slightly strange structure expected in that hook. Manually tested attached patch, works.

Speaking of tests, do we want/need to test this? Hm. Let's defer that to #674692: Node module integration tests, so RTBC.

As for the caching issue, see #870292: hook_field_extra_fields() results should be cached by language

Committed to CVS HEAD. Thanks!

+++ mollom.module	31 Jul 2010 14:40:28 -0000
@@ -1580,16 +1580,22 @@ function mollom_get_statistics($refresh 
 function mollom_field_extra_fields() {
...
+  foreach (mollom_form_list() as $form_id => $info) {
+    if (isset($info['entity']) && isset($forms[$form_id])) {

blargh. :) #775444: API implementor documentation just made me recall and revisit this patch again.

The code now assumes that every form that registers an 'entity' is a Field API entity.

That's not necessarily the case -- it may be true for all core entities, but technically, contributed and custom modules could register forms to protect by Mollom that are neither using entity API nor field API.

We can keep this issue fixed until someone actually reports a problem about this.

Powered by Dreditor.

Umph, but I agree that it is acceptable for now. Might be worthy a code comment ... ?

Right. Defering tests to #674692: Node module integration tests

Thanks for the documentation. Committed to CVS HEAD.