when configuring mollom to protect eg.' node: Page form', it will put it's abuse-report part into every node-type's delete process,
i guess that's a Bug?

or not? then maybe it should be configurable where it shows.

it would also be nice if there was some configurability for the abuse-report form.
What i miss most is the ability to set the default (i prefer 'don't report', i have only little spam in my user-generated nodetypes, still i often need to delete them.).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Moloc’s picture

This issue seems to be not easy to fix. Here a description, what i see from the code:

If you have two content types (for example article and news), then you have two forms, that you can protect with mollom. The form ids are article_node_form and news_node_form, so mollom is able to identify each content type. The problem occures on delete. Both nodes (article and news) share the same delete-form (node_delete_confirm). Therefore mollom does not know (if at least one of these content types are protected by mollom), if the form is of type article or news, when deleting a node.
(for example see mollom.module - mollom_form_alter().)

Possible solution?
It is possible to create a function, returning the form-id of the edit-form. So it would be possible to check, if this content-type is protected by mollom. As this issue also affect other forms, like webform, it may be possible to extend hook_mollom_form_list() to return the name of this function, generating the edit-form-id with the current $form. Just an idea.

areikiera’s picture

Priority: Normal » Major

I've marked #1232734: Protecting a node type's edit form adds "Report as inappropriate" form to all node types as duplicate. I'm experiencing the problem as well, and agree with @marleythedog on the duplicate post that:

For some content types the content reporting is not necessary. In addition, since the "Report as inappropriate" form defaults to "Unwanted, taunting, off-topic", I am concerned that editors who habitually click "next" will end up reporting false positives to Mollom.

For that reason, I'm changing the priority to major (and maybe it should be critical), since it seems that could compromise Mollom's ability to learn what is actually "Unwanted, taunting, off-topic".

As @Moloc suggests, this doesn't appear to be an easy fix. In the meantime, I would like to suggest a workaround, and would appreciate feedback:

When a user deletes a node now, Mollom's "Report as inappropriate" form defaults to "Unwanted, taunting, off-topic". If we could patch the module to default to "Do not report", users dealing with this now could at least hide the form by content type with CSS. The strange part is that in the module code, it appears that it's already supposed to default to "Do not report" (lines 553-565 in mollom.module):

  $form['mollom']['feedback'] = array(
    '#type' => 'radios',
    '#title' => t('Report as inappropriate'),
    '#options' => array(
      0 => t('Do not report'),
      'spam' => t('Spam, unsolicited advertising'),
      'profanity' => t('Obscene, violent, profane'),
      'low-quality' => t('Low-quality'),
      'unwanted' => t('Unwanted, taunting, off-topic'),
    ),
    '#default_value' => 0,
    '#description' => t('Sending feedback to <a href="@mollom-url">Mollom</a> improves the automated moderation of new submissions.', array('@mollom-url' => 'http://mollom.com')),
  );

Any ideas as to why that doesn't seem to be working properly?

Thanks!

sun’s picture

Issue tags: +API change
NicolasH’s picture

Agree with #2, at the very least the "Do not report" choice should be the default. For some reason I can't form_alter the form...

NicolasH’s picture

Hm, seems like the key as integer is the culprit. It works if the key is a string like the others...

$form['mollom']['feedback'] = array(
    '#type' => 'radios',
    '#title' => t('Report as inappropriate'),
    '#options' => array(
      'no_report' => t('Do not report'),
      'spam' => t('Spam, unsolicited advertising'),
      'profanity' => t('Obscene, violent, profane'),
      'low-quality' => t('Low-quality'),
      'unwanted' => t('Unwanted, taunting, off-topic'),
    ),
    '#default_value' => 'no_report',
    '#description' => t('Sending feedback to <a href="@mollom-url">Mollom</a> improves the automated moderation of new submissions.', array('@mollom-url' => 'http://mollom.com')),
  );
sun’s picture

@NicolasH: That's a separate issue - which has been fixed in 2.x already.

The original/actual issue here remains. Will try to look into that later.

NicolasH’s picture

@sun: Nice, thanks.

sun’s picture

Priority: Major » Normal

Hm. I'm not able to reproduce this bug in 7.x-2.x.

I'll try to write an automated test based on the scenario / steps to reproduce provided, but I'm fairly confident that it is going to pass, too.

sun’s picture

Title: abuse-report-form gets into every node-type, even if protection is configured for one node-type only » Feedback options appear for every entity bundle, even when not protected
Version: 7.x-1.1 » 7.x-2.x-dev
Status: Active » Needs review
FileSize
11.22 KB

Ok, apparently I was wrong and I managed to reproduce this bug.

The main problem is that the current architecture of the module does not cater for the bundle concept of entity types at all.

So, in order to fix this, we need to make the module aware of that concept. Doing so was not exactly easy — without rewriting the entire thing ;)

Attached patch introduces two new database columns for properly tracking the entity type and bundle information of protected forms. This information will not only be useful for fixing this particular bug. Given where Drupal core is heading, I actually played many times with the idea of making the Mollom module only support entities in the future - or at least, to make that the primary integration mechanism (inherently turning non-entity integrations into second-class citizens that may not get all features).

But for now, this is the best solution I was able to come up with. If you'd like to test it, I'd recommend to use http://drupal.org/project/demo for easily backing up your database before running the module update. Please note that the module update can only cater for the entity types/bundles being defined by Drupal core. Any form protections for other entity types/bundles need to be updated manually. Without updating the others, this bug remains to exist for them, but otherwise nothing bad will happen.

I hope that @Dries will be able to have a quick look at this.

sun’s picture

Any form protections for other entity types/bundles need to be updated manually.

Sorry - with that I meant: Simply going to the administrative configuration page for a particular form protection and re-saving the form without any changes will populate the correct entity type + bundle information.

sun’s picture

And just for kicks, here's the test-only patch, which is supposed to fail ;)

Status: Needs review » Needs work

The last submitted patch, mollom.entity-bundle-feedback.9.test-only.patch, failed testing.

sun’s picture

Status: Needs work » Needs review

Good. :)

sun’s picture

Status: Needs review » Fixed
Issue tags: -API change
FileSize
10.79 KB

Thanks for reporting, reviewing, and testing! Committed to 7.x-2.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Since Drupal 6 does not have a generic Entity API concept, I do not think this can be backported to 6.x-2.x — at the very least, that would require a considerable amount of custom code for each particular entity type that is protected, and I just don't think it's worth the time and resources to go there.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

  • Commit 0afaafd on 7.x-2.x, 8.x-2.x, fbajs, actions by sun:
    - #1352012 by sun: Fixed Feedback options appear for every entity bundle...

  • Commit 0afaafd on 7.x-2.x, 8.x-2.x, fbajs, actions by sun:
    - #1352012 by sun: Fixed Feedback options appear for every entity bundle...