The following code adds the logintoboggan.permissions.js javascript file to the page:
function logintoboggan_js_alter(&$javascript) {
// Look for the user permissions js.
if (isset($javascript['modules/user/user.permissions.js'])) {
$id = logintoboggan_validating_id();
// If the pre-auth user isn't the auth user, then swap out core's user
// permissions js with LT's custom implementation. This is necessary to
// prevent the pre-auth role's checkboxes from being automatically disabled
// when the auth user's checkboxes are checked.
if ($id != DRUPAL_AUTHENTICATED_RID) {
$javascript['modules/user/user.permissions.js']['data'] = drupal_get_path('module', 'logintoboggan') . '/logintoboggan.permissions.js';
}
}
}
Problem is, unless the Drupal.settings.LoginToboggan var has been added, the lt...perms...js file will cause a javascript error, which can potentially cause other Javascript not to run.
The attached patch adds the D.S.LT if on the field_ui_field_edit_form (which is the only form I have found so far that requires the addition).
Alternatively, we could add it *every* time (might be a better solution).
But for now, I just added it to the one form.
If I knew more about the hook_js_alter() I would add it there. I would think it could also be added with a drupal_add_js() but from what I can see, it is being added with $form['#attached']['js'] which seems a bit odd.
Any other thoughts, let me know.
Comments
Comment #1
fangel CreditAttribution: fangel commentedI stumbled upon this same problem, however I solved it using a call to drupal_add_js during the hook_js_alter, because then it's certain that the settings-object is going to be set if we replace the permissions.js-file with logintoboggan's
Comment #2
j0rd CreditAttribution: j0rd commentedI believe most people who run into this bug, is because they have field_permissions (Field Permissions) module installed. Going through and tagging issues and adding keywords into issues, so people can find them.
#1407402: Field permissions reset to default (no access) after updating the field
#1407564: CSS conflict with logintoboggan, prevents using Custom Permissions (field_permissions module)
Comment #3
md2 CreditAttribution: md2 commentedI've been looking at the other issues highlighted by j0rd and believe that the fix suggested by fangel may well fix them all.
fangel, your patch failed for me so I've re-rolled it (attached).
Cheers,
Mark
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedFYI: this problem also manifests as turning the settings fieldset in a rules component non-clickable.
I can confirm that the patch in #3 also solves this issue.
Please roll a new version of LoginToboggan incorporating this patch.
Comment #5
ptmkenny CreditAttribution: ptmkenny commented#3 also fixed this issue for me.
Comment #6
Taxoman CreditAttribution: Taxoman commentedhow about pushing #3 to -dev?
Comment #7
jennypanighetti CreditAttribution: jennypanighetti commentedBeen sitting here for months, can we get this rolled in please?
Comment #8
muka CreditAttribution: muka commentedworks for me too, thank you
Comment #9
Kojo Unsui CreditAttribution: Kojo Unsui commentedHi,
having also this TypeError: Drupal.settings.LoginToboggan is undefined with Field Permissions module enabled, when I tried to use custom permissions.
Patch in #3 solved it, thanks !
I made no further test, so I hope it has no incidence on login Toboggan functionality.
Comment #10
StoraH CreditAttribution: StoraH commentedPatch in #3 worked for me as well, thx!
Comment #11
marcus178 CreditAttribution: marcus178 commentedI'm getting the same error with adv agg turned on, but the patch above doesn't fix it.
Comment #12
KerriO CreditAttribution: KerriO commentedWill there be any effort to roll this into a release? I see the issue has been know since March 2012.
Does anyone know if the patch from #3 works on the current release (7.x-1.3)?The bug report thread related to that specific release was closed as a duplicate of this thread, but is the patch still applicable?Editing to confirm that patch appears to still be good for 7.x-1.3.
Comment #13
Anonymous (not verified) CreditAttribution: Anonymous commentedPatch is good. +1
Comment #14
stevenx CreditAttribution: stevenx commentedPatch from #3 works
Comment #15
hansrossel CreditAttribution: hansrossel commentedPatch from #3 works
Comment #16
Ravenight CreditAttribution: Ravenight commentedPatch from #3 works -- It was causing massive issues with the Rules module
Comment #17
david_garcia CreditAttribution: david_garcia commentedThis issue reappears when using the Advanced CSS/JS Aggregation module, even with patch applied:
https://drupal.org/project/advagg
https://drupal.org/node/2202851
Comment #18
david_garcia CreditAttribution: david_garcia commentedDigging into this, the wroking patch (#2) proposes a call to drupal drupal_add_js() inside hooj_js_alter, but hook_js_alter is supposed to be used as a direct manipulation of the $javascript array that is passed by reference.
So, instead of having this:
We should directly alter the javascript configuration array:
Comment #19
david_garcia CreditAttribution: david_garcia commentedFound It!
Patch#3 is "wrong", this is what the documentation states:
The proper way of adding the addition setting is manually altering the &$javascript array.
Comment #20
Alauddin CreditAttribution: Alauddin commented#3 patch file with edit from #18 worked for me.
thanks
Comment #21
david_garcia CreditAttribution: david_garcia commentedComment #22
david_garcia CreditAttribution: david_garcia commentedClosed #1407564 as a duplicate.
Comment #23
david_garcia CreditAttribution: david_garcia commentedI was not confortable with any of the proposed patches, they were actually not attacking the root of the problem, so I took a deeper look into it.
I am proposing a new patch, which is way more intrusive, but aims at solving the root of the problem.
Now, whenever "modules/user/user.permissions.js" is found via hook_js_alter, the module adds a new behaviour (instead of overriding the default one) that alters the checkbox linking performed by user.permissions.js.
It is way more clean than original approach as we are not duplicating user.permissions.js code.
The patch has been tested and rolled against latest dev.
Comment #24
IWasBornToWin CreditAttribution: IWasBornToWin commentedPatch didn't solve issue. (I upgraded to latest dev).
To duplicate, add another role and then go to login toboggon settings and use the new role for immediate temporary logins. Unless role is authenticated, the custom permissions can't been seen within field settings.
Comment #25
david_garcia CreditAttribution: david_garcia commented@IWasBornToWin: Just to make sure:
- You say you upgraded to latest dev, but, did you apply the patch? This has not been commited and thus is not in the repository.
- You say custom permissions cannot be seen within field settings, I guess you are saying that no permissions at all are being shown at field settings, that is what was happening originally and what the patch aims to solve. Can you send a screenshot?
Greetings,
Comment #26
IWasBornToWin CreditAttribution: IWasBornToWin commentedyou're correct....no patch and no permissions showing.
Comment #27
IWasBornToWin CreditAttribution: IWasBornToWin commentedWorks excellent! Thanks
Comment #28
IWasBornToWin CreditAttribution: IWasBornToWin commentedComment #29
Alauddin CreditAttribution: Alauddin commented@david_garcia_garcia
Thanks again for the patch #23
tested working on a new site I am working on.
Comment #30
tawellman CreditAttribution: tawellman commented@david_garcia_garcia
Patch from #23 Works!! Thank You!!
Comment #31
rollingnet CreditAttribution: rollingnet commentedThe patch #23 is working for me, too.
Time to commit it to dev?
Comment #32
alex_sansom CreditAttribution: alex_sansom commentedPatch in comment #23 applies cleanly for me on 7.x-1.3 to solve the same issue as mentioned in #4, with Rules Components "Settings" fieldset.
Comment #33
bisonbleu CreditAttribution: bisonbleu commentedRan into this issue. Applied patch in #23 to 7.x-1.3+7-dev (under Drupal 7.27). Permission UI is back. Thanks @david_garcia !
Comment #34
NWOM CreditAttribution: NWOM commentedThe patch still works if you manually apply it on the newest stable release (7.x-1.4), however the patch will need to be rerolled.
Thanks for the patch!
Comment #35
Majdi CreditAttribution: Majdi commentedThe patch #23 is working for me, too
Comment #36
kevinsiji CreditAttribution: kevinsiji commentedPatch rerolled against stable 7.x-1.4
Comment #37
AnybodyThe patch works great as I can confirm and I would be very very happy to see this in a new stable release because in my case the bug killed rules UI javascript completely.
Thanks a lot for your work!
Is there an active maintainer in this issue?
Comment #38
AnybodySetting up the priority because the bug crashes JS in large parts of the Admin section.
Please commit ASAP. Thanks a lot!
Comment #39
AnybodyThe patch in #36 is okay but can not be applied because the path used is not the module directory but "sites/all/modules" so this should be changed.
Comment #40
AnybodyCorrected patch against 7.14 attached. Of course the final patch will be run against DEV.
I've also attached the patched version of the module until this is fixed.
Comment #41
idebr CreditAttribution: idebr commented@Anybody your patch was created from outside the module directory. I rerolled it from inside the module directory.
Comment #42
DamienMcKennaComment #43
md2 CreditAttribution: md2 commentedTesting patch #23 and it does fix the issue with field permissions etc. It does however introduce a bug on the permissions page. It is hiding the tickbox under the logintoboggan non-athuenticated role column, anywhere were the permission is set on authenticated user. The following screenshot may help show whats happening.
To Reproduce:
If anyone can confirm that would be great! Once fixed we can get this merged to dev.
Thanks,
Mark
Comment #44
BBCI'm also seeing the #43 issue. Checkboxes are there, just hidden with display: none;.
In my case, I applied patch #23 applied against a version: 7.x-1.4.
Comment #45
goldlilys CreditAttribution: goldlilys commentedYep, seeing #43 also for permissions for non-authenticated role where the checkboxes are disappearing. Using 7.x-1.x-dev version.
Fixed the field permissions issue, but this new issue came up. Not sure why those fields have display:none ?
Comment #46
rooby CreditAttribution: rooby commentedI have had success with the patch in #41
Comment #47
sylvaticus CreditAttribution: sylvaticus commentedPatch in #41 also works over LoginToboggan 1.15.
Comment #48
ckngTested patch #41 working.
Comment #49
dooug CreditAttribution: dooug at Promet Source commentedI've tested this and can confirm the patches from #41 and #23 introduce a new bug, as reported in #43. Thanks, @md2 for the clear steps to replicate.
Can someone write some steps to replicate the original error reported and paste in an example of that error as well?
This needs work. We cannot commit something that introduces new bugs, despite fixing others.
Comment #50
scotwith1t#43 is about the older patch in #23 against an older release(1.4) and rerolled against dev (#36). #41 is against the current recommended (1.5) release and works. The patch in #41 also applies against -dev just fine (one offset of 7 lines) and fixes the issue.
Don't see any reason not to commit this patch...we have run into this problem multiple times as this combination of modules comes up quite often for us. Thx.
Comment #51
dooug CreditAttribution: dooug at Promet Source commentedThanks @scotself. However, as I said in #49, I did test this with the latest patch #41 on the latest 7.x-1.x-dev, and it does introduce the bug that removes checkboxes from the permissions page unexpectedly, as reported in #43
That needs to be fixed. Hope you can help refactor this patch to fix the newly introduced bug!
Also, please provide interdiffs to make reviewing easier!
Comment #52
ndewhurst#23 is a great patch, but it does "introduce" the bug reported in #43.
Basically, the user module's Javascript hides all "real checkboxes" in a given row where the authenticated user role has been granted the given permission (in order to visually replace them with the "dummy checkboxes"). david_garcia's code neatly undoes the underlying DOM manipulation, but not the inline style. This is why everything works nicely when toggling the authenticated user checkbox where it was previously unset (i.e. the permission will appear to cascade to all other roles except the pre-auth role), but all the pre-auth checkboxes for permissions previously granted to auth users will be hidden when first rendering the table.
The simple solution here is to extend David's approach (undoing rather than replacing the user module's Javascript), and bring all pre-auth checkboxes back into view. I'm using the same method for setting visibility as in modules/user/user.permissions.js.
Here's a patch against the latest dev that works well in my basic testing. It also applies to 7.x-1.5, and appears to address all concerns reported in this thread. I've also included a whitespace-ignorant interdiff for clarity.
Comment #53
-enzo- CreditAttribution: -enzo- at Anexus commentedPath in #52 by @ndewhurst works perfect for me. thanks
Comment #54
ndewhurstCool @-enzo-. Do you think we can go ahead and RTBC this?
Comment #55
-enzo- CreditAttribution: -enzo- at Anexus commentedoops my fault, changing to RBTC
Comment #56
-enzo- CreditAttribution: -enzo- at Anexus commentedComment #57
rwilson0429 CreditAttribution: rwilson0429 commentedPatch in #52 works well for me too
Comment #58
travelertt+1 RTBC for patch #52
Comment #59
rob.costello CreditAttribution: rob.costello commented#52 worked for me too.
Comment #60
andsigno82 CreditAttribution: andsigno82 as a volunteer commentedConfirmed patch #52 worked for me too
Comment #61
Shane Birley CreditAttribution: Shane Birley commentedI can also confirm that the patch in #52 solves the issue.
When do you think a change like this (which seems to clash with a lot of different js) will be rolled into a 1.6?
EDIT
I forgot to mention that I ran into this problem because I am using conditional fields with Display Suite and not specifically field permissions.
Comment #62
joelstein CreditAttribution: joelstein commented#52 works for me!
Comment #63
AnybodyCould someone please create a new (+stable) release? The module is still broken since month...
Comment #64
delacosta456 CreditAttribution: delacosta456 commentedhi
i don't have the module enable nut i am having this issue.
i had done a complete uninstall, clear cache, run cron, use the variablecheck module, remove the directory form module but stil having the error
Comment #65
Alauddin CreditAttribution: Alauddin commented#52 works.
fyi - applies to dev, not 1.5 version
Comment #66
asiby CreditAttribution: asiby commentedI do not have logintoboggan ... yet I am having the same issue.
Comment #67
ndewhurst@asiby, perhaps another module on your site is doing something similar to what logintoboggan is doing here. You could search for hook_js_alter() implementations that are modifying $javascript['modules/user/user.permissions.js']...
Comment #68
asiby CreditAttribution: asiby commentedGood point @ndewhurst. I search for that and didn't find anything similar.
However, I discovered that this error happens only for one field. It's a field provided by the scripto module. That module may not be properly using the Drupal API. I noticed that when I saw some HTML being crafted using PHP without using theme hooks or the theme() function. sigh.
I will find a way around it.
Thanks for your help.
Comment #69
ThuleNB CreditAttribution: ThuleNB commentedHas the patch been applied to the dev version yet? I am having the same issue in combination with the field permissions module. Unfortunately, I am not much of a coder and have difficulties applying a patch.
Comment #70
C.E.A CreditAttribution: C.E.A commentedAny news for a new release to automatically fix the issue mentioned above, instead of applying a patch ?
Comment #71
BeatnikDude CreditAttribution: BeatnikDude as a volunteer commentedI to would like to see this patch to see this patch applied to dev. Thanks.
Comment #72
burnsjeremy CreditAttribution: burnsjeremy as a volunteer and commentedThis was RBTC 2 years ago, any update on when it will make it into the dev version? Are we waiting on another version of the patch or anything else that needs to be done to push this issue through. I can help out if anything else needs to be done just let me know.
Comment #73
DamienMcKennaI think the module needs a new comaintainer.
Comment #74
burnsjeremy CreditAttribution: burnsjeremy as a volunteer and commentedWell I would be happy to help out if needed! Also, I just tested the patch again (this is the 3rd site I've had to apply it to so far) and it still fixes the issue :)
Comment #75
deggertsen CreditAttribution: deggertsen as a volunteer commentedThanks for patch. Would love to see it applied!
Comment #76
matman476 CreditAttribution: matman476 commented#52 works like a charm!
Comment #77
cmseasy CreditAttribution: cmseasy commentedpatch #52 works, please commit
Comment #78
granticusiv CreditAttribution: granticusiv commentedI've had patch #52 working on a production site for four months without issue. Please commit.
Comment #79
Johan den Hollander CreditAttribution: Johan den Hollander at Finalist commented#52 Fixes my problem. Should be committed.
Comment #80
gngn CreditAttribution: gngn commentedLike all the others: #52 Fixes my problem. Should be committed.
Btw: I succesfully applied #52 against current "7.x-1.5" (not the dev-version).
Comment #81
PhilYComing here because of Field Permissions module issue, patch #52 works for me when applied to 7.x-1.5 release (it seems not included yet on 7.x-1.5+10-dev version).
Thanks to the fixers.
Comment #82
Rafal LukawieckiI'd like to add that I have been using the patch from #52 for over a year in production and it works well. Please commit to the tree.
Comment #83
BeatnikDude CreditAttribution: BeatnikDude as a volunteer commentedI also had a conflict w/ the Field Permissions module.
Patch #52 works applied to 7.x-1.5 release.
Thanks for the patch.
+1 to commit.
Comment #84
timb CreditAttribution: timb commentedThis issue has been a thorn in my side for quite some time now. I have been forced to turn of LT anytime I want to make custom permission changes.
I would like to see this committed.
Comment #85
cmseasy CreditAttribution: cmseasy commentedPatch 52 also solves aa unresolved rules issue: https://www.drupal.org/project/rules/issues/2530440
Please commit
Comment #86
dkatena CreditAttribution: dkatena commentedpatch #52 works for me!
Comment #87
Collins405 CreditAttribution: Collins405 commentedSo the current fix is a patch that is 5 years old.
I'd say that probably the longest community review stage I've ever seen 🤣
Shall we get this committed?
Comment #88
Rafal LukawieckiIt hurts me to see so many good patches left abandoned by module maintainers. This is not specific to logintoboggan but a good few other modules, too. It is really disheartening when someone has contributed their time and effort to make something useful to see it languish.
Drupal 7 is still by far the most popular Drupal out there. No need to let it rot.
Absolutely, this should be looked at by the maintainer and committed, just like a few others. Please.
Comment #89
NWOM CreditAttribution: NWOM commentedI suggest taking over the module as a maintainer via: https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or.... All it takes is e-mailing and adding an issue to the issue queue, and if the maintainer doesn't respond within 2 weeks, you move the issue to a specific project ownership queue (while making a few changes to your issue), and that's it. Within a short time you'll become a maintainer. I recommend also listing all of the patches that should be committed but haven't yet and for how long they've been open.
Comment #90
stevecowie CreditAttribution: stevecowie commentedI'm finally on the case. Got side-tracked by work and then doing the D8 upgrade. I'll focus hard on D7 for the next few weeks and get the issue queue back under control.
Comment #92
stevecowie CreditAttribution: stevecowie as a volunteer commentedPatch now applied to dev version. Thanks to all and apologies for the slow response.
Comment #93
DamienMcKennaThanks Stew!
Comment #95
Rafal LukawieckiThank you, @stevecowie, very much indeed!