Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Our bypass permission is very ambiguous and could also potentially conflict with another module since it does not include the word 'Mollom'. We should rename this to 'bypass mollom protection' so it is in line with our 'bypass content access control' we have in Drupal 7.
| Comment | File | Size | Author |
|---|---|---|---|
| #21 | mollom-HEAD.permission.21.patch | 2.11 KB | sun |
| #18 | mollom-DRUPAL-6--1.bypass-permission.18.patch | 1.71 KB | sun |
| #12 | 663254-mollom-perm-D6.patch | 1.72 KB | dave reid |
| #11 | 663254-mollom-perm-D7.patch | 2.03 KB | dave reid |
| #6 | 663254-mollom-perm-D6.patch | 1.66 KB | dave reid |
Comments
Comment #1
dave reidDoes this sound reasonable before I proceed with a patch?
Comment #2
dries commentedI'm OK with the proposed change.
We should probably worry about an upgrade path though?
Comment #3
dave reidOf course. It's pretty easy to do though. Should we worry about an upgrade path for the HEAD/D7 branch though?
Comment #4
dries commentedCould you rephrase the question? We should have an upgrade path from D6 to D7.
Comment #5
dave reidOh, I was thinking D7->D7 upgrade, but it's going to have to be included in the D6->D7 upgrades anyway, so moot point.
Comment #6
dave reidPatches for 6.x and 7.x attached for review. Open to suggestions on improving the description for the bypass mollom protection permission in the 7.x patch.
6.x
No test failures (comment operation exceptions)
Tested upgrade path
7.x
Currently expected test failures?
Comment #7
dave reidComment #8
sunI wonder whether we shouldn't entirely remove this permission instead?
Due to the new form integration, each registered form comes with a 'bypass access' permission name, which basically should be sufficient?
Comment #9
dave reidProbably not actually. The per-form permissions are very high-level permissions like 'administer nodes', etc. If I have multiple levels of trust on a site, this permission is still very useful. Plus, how do admin/regular users know which permissions to add to a role to bypass forms? They'd have to look in the code to figure it out. One overall bypass permission make a lot of sense. Same reason we still have a 'bypass node access' in D7.
Comment #10
sunok, but this needs a re-roll now
Comment #11
dave reidNot sure why the patch didn't apply on the testbot. Re-rolled for D7.
Comment #12
dave reidAh, odd it was the D6 patch that didn't apply, but the D7 patch still did apply, contrary to the testbot results. Re-rolled for D6 as well.
Comment #13
dave reidAh, test bot is trying to apply D7 patches against D6 Mollom. That's why we're getting that result.
Comment #15
dave reidI'd like to have a few other people try and run the test manually. I can't duplicate the test bot results at all, so it may be something odd with running these tests on the testbot platform.
Comment #16
dave reidArg, thought the failures were with 6.x-1.x. I can confirm test failures with non-patched Mollom 7.x-1.x.
Comment #17
sun@Dave: Yes, Mollom HEAD is known to be broken currently, and will keep that way, until we get to do #683998: Kill cache_mollom, rely on Form API (after the new D6 stable will be released)
Comment #18
sunSorry, my other patch invalidated this again.
Re-rolled against 6.x. Port to HEAD should follow after commit.
Comment #19
dries commentedI'm OK with this patch. It looks like an improvement to me. I guess it is OK to use 'mollom protection' (versus the more generic 'spam protection').
Comment #20
dries commentedCommitted to DRUPAL-6--1. Thanks.
Moving to Drupal 7.
Comment #21
sunRe-roll against HEAD.
Comment #22
dries commentedCommitted to CVS HEAD. Marking this 'fixed'.