When you fill in a captcha incorrectly (either from mollom or from the captcha module), you get an error (as expected). But the captcha is not visible anymore, so you cannot fix this and retry.

This happens only if AHAH is enabled.

CommentFileSizeAuthor
#6 mollom_fbss_ahah-12632685-6.patch1.57 KBcodesidekick
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

IceCreamYou’s picture

Status: Active » Closed (won't fix)

There is very little that FBSS does to integrate with Mollom -- it just implements 2 hooks. Mollom does the rest. If there is a problem, it's probably on Mollom's side.

That said, I don't know very much about Mollom or CAPTCHA, and FBSS wasn't designed with CAPTCHAs in mind. I personally don't use either module with FBSS and I'm not convinced that there's a good reason to do so. The integration exists because some people wanted it and it doesn't add any overhead for non-Mollom users.

codesidekick’s picture

Hi there, old issue but one that I've managed to fix without any hacks. In a custom module include the following hook:

function MY_MODULE_facebook_status_form_ahah_alter(&$new_form, $old_form) {
  $new_form['mollom'] = $old_form['mollom'];
}

This is the same method that FBSS Twitter, FBSS Privacy and FBSMP use in order to include fields in the AHAH fragmented response. This is a must for all uses of Drupal Commons who wish to have Mollom active on AHAH Facebook Style Statuses.

IceCreamYou’s picture

Version: 6.x-3.0-rc3 » 6.x-3.x-dev
Status: Closed (won't fix) » Needs review

I must have been grumpy last September. :-P If this works it needs to be ported to D7 too and ideally in patch form.

IceCreamYou’s picture

Project: Facebook-style Statuses (Microblog) » Statuses (Social Microblog)
Version: 6.x-3.x-dev » 7.x-1.x-dev
Component: Code - Functionality » Integrations
Status: Needs review » Patch (to be ported)

Committed to dev, thanks! Needs porting to D7

codesidekick’s picture

We discovered two additional issues with this solution unfortunately.

  1. The status message isn't carried over (no form state) between the submit
  2. Mollom only works the first time (it seems Mollom may need to be forced to regenerate).

We are actively working to fix these for a site launch and if anybody has any suggestions or input then please download the dev build and give us a hand! I think we're pretty close to coming up with a solution and we'll make sure to post it here once we have something.

Thanks

codesidekick’s picture

Attached is a patch for 6.x-3.x-dev that fixes the following issues for Mollom AHAH integration:

  • Carries over status value and default value and disables textarea box input during Mollom verification. This prevents the user from changing the status to a blacklisted item and verifying it.
  • Regenerates Mollom form state so that Mollom gets triggered on each status submit, not just the first.

I'm not sure if the doubling up on the submit functions is really necessary, in our case only the submit handler on $form['fbss-submit']['#submit'] gets called however we've also added the submit handler to $form['#submit'][].

There is still one outstanding issue for us: FBSMP uploads bypass Mollom all together. . From what we've seen so far, FBSMP implements it's own AHAH responses and submit handlers and this is going to take us a while to debug. We are working to fix this however the solution to this may be more fitting as a patch to the FBSMP module rather than the facebook_status module.

If you have any feedback on the quality of the patch please let me know and I'll improve it. We will be continually testing this over the next couple of months and will keep you up to date if we find any bugs.

IceCreamYou’s picture

Status: Patch (to be ported) » Needs work

Thanks for the patch. Basically looks good, just a few points of feedback/questions:

+++ b/submodules/fbss_mollom/fbss_mollom.module
@@ -79,3 +79,35 @@ function fbss_mollom_facebook_status_form_ahah_alter(&$new_form, $old_form) {
+ * Implements hook_form_alter().

Technically hook_form_FORM_ID_alter().

+++ b/submodules/fbss_mollom/fbss_mollom.module
@@ -79,3 +79,35 @@ function fbss_mollom_facebook_status_form_ahah_alter(&$new_form, $old_form) {
+    // Value for the status box is set as the previous (to prevent users
+    // from doing javascript injection).
+    $form['fbss-status']['#value'] = $form_state['values']['fbss-status'];

What exactly is the issue this is solving?

+++ b/submodules/fbss_mollom/fbss_mollom.module
@@ -79,3 +79,35 @@ function fbss_mollom_facebook_status_form_ahah_alter(&$new_form, $old_form) {
+  $form['#submit'][] = 'fbss_mollom_facebook_status_box_submit';
+  $form['fbss-submit']['#submit'][] = 'fbss_mollom_facebook_status_box_submit';

I forget exactly how this works but I think the second one is required for AJAX submission and the first one can run if AJAX is disabled. Needs testing.

+++ b/submodules/fbss_mollom/fbss_mollom.module
@@ -79,3 +79,35 @@ function fbss_mollom_facebook_status_form_ahah_alter(&$new_form, $old_form) {
+ * Implements hook_form_submit().

Not an actual hook; I usually use "Handles form submits for [form]."

codesidekick’s picture

Thanks,

With $form['fbss-status']['#value'] = $form_state['values']['fbss-status']; this is to stop people from rewriting their status whilst they fill out the Mollom CAPTCHA. Theoretically you could submit a suspicious link in a status, be asked for a CAPTCHA, replace the status with some profanity, fill in the CAPTCHA correctly, and the profanity would slip through. ['#default_value'] isn't enough because somebody could inject a new value into the form field in other ways.

So currently the workflow is:

  1. User submits a suspicious status.
  2. The status form refreshes, with the status box disabled, and a Mollom field underneath.
  3. The user fills in the Mollom correctly and the status gets added. (or the user submits it incorrectly and ends up at 2.

I'll do some more testing over the coming couple of weeks whilst I'm on this project. Currently struggling to find a fix for FBSMP, that module is a bit more challenging than your usual Drupal AJAX forms.

Thanks,

IceCreamYou’s picture

If a field is disabled, Drupal has built-in validation to check that the field wasn't changed. The right way to do that is with $form['FORM-ELEMENT']['#disabled'] = TRUE; instead of explicitly with the disabled attribute.

However, now that I'm thinking about it, I actually think you should be able to edit your status if it's flagged just like you can with nodes and comments. Let's say you put in a suspicious link for example and once it's flagged you decide to clean up your act and take it out. If the status box is disabled you have to refresh the page, whereas if you can edit it you can fix your mistake.

Having said that, if you disagree, I'll accept a patch that disables the form instead of lets you edit it -- having something working awkwardly is better than having it not work at all. :-)

And yes, FBSMP forms are complicated...

codesidekick’s picture

Cool, wasn't aware of #disabled ... so #disabled in combination with #value should do the trick if we want to prevent users from changing their status.

If we want users to be able to change their status we'll need to regenerate Mollom but only in blacklist mode (and not Captcha mode otherwise a user will get a double captcha if the field is changed). I'm not really a Mollom expert but I can have a play around with it when I have some time. I'll try and stay on top of this issue from now on (been on holidays for a couple of weeks).

If anybody else wants work done on this patch, or wants to test, post a comment in this issue!

IceCreamYou’s picture

If we want users to be able to change their status we'll need to regenerate Mollom but only in blacklist mode (and not Captcha mode otherwise a user will get a double captcha if the field is changed).

What I'd like to do is have the exact same behavior for Statuses as Mollom uses for nodes and comments, and what you describe does seem to be the approach Mollom takes to nodes and comments (although it's kind of hard to tell just from testing).

However, let's address that in a follow-up issue. In the mean time, I'd like to commit the patch from #6 with the few changes suggested (the two docblock changes, using #disabled, and FBSMP compatibility). Any progress on updating the patch?

One acceptable approach to FBSMP compatibility is to force blacklist-only mode (e.g. no CAPTCHA) to avoid all the AJAX trickiness.