I get the following error each time the form is submitted when Mollom is configured to protect a form without specifying any fields to analyze.
Invalid argument supplied for foreach() in /home/lovvik/Acquia/gardener/branches/gardens-beta-14/docroot/modules/acquia/mollom/mollom.module on line 969. (GardensError)
The mollom_form_get_values function is being passed a boolean value for $fields rather than the expected array in this case.
My simple fix detects the $fields value is the incorrect type and sets it to an empty array. The function doesn't expect other types for that parameter.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | mollom.form-fields.6.patch | 2.55 KB | sun |
| #5 | mollom.form-fields.5.patch | 1.67 KB | sun |
| #2 | mollom-DRUPAL-6--1.form-fields.2.patch | 1.81 KB | sun |
| mollom_bad_field_type.patch | 800 bytes | paul.lovvik |
Comments
Comment #1
sunHm. Right now, the administrative form configuration should prevent you from protecting a form with text analysis, if no fields are selected. You should get a form validation error.
Is it possible that you got this error after updating from an earlier version, perhaps? Because, the only way I say to get past that validation would be to programmatically save a form protection (which does not perform that validation of fields)
Comment #2
sunThe added unit test in attached patch proves that it is not possible to configure a form for text analysis without enabling fields to analyze. Therefore, I'm not really sure how you were able to get this error.
Comment #3
sun#2: mollom-DRUPAL-6--1.form-fields.2.patch queued for re-testing.
Comment #5
sunComment #6
sunThe patch in #5 was identical to #2.
This might have been caused by our own module update, so fixing that.
However, we should not "fix" the low-level data type mismatch issue, because $fields should never be FALSE, and thus, all that a change for this error message would do is to hide an actual error caused by some other module. In turn, you'd never ever notice that something is actually wrong on your site.
Comment #7
dries commentedThis looks like a good fix to me. I'll ping Paul to see if he can help test this or so.
Comment #8
paul.lovvik commentedThis fix looks good to me also. I think this patch is right however I don't believe that it fixes my specific issue on our existing site because our updates have already run and therefore this new code has no effect on our site. That said, we are no longer experiencing the issue on our site. It appears that the point of this patch (to ensure that the enabled_fields field in the mollom_form table has an array) directly addresses the issue we were having.
At any rate, we are no longer having this problem, and I believe this patch would have prevented the problem we experienced.
Comment #9
dries commented@sun: let's get this committed then!
Comment #10
sunThanks for reporting, reviewing, and testing! Committed to all branches.
A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.