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.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

fangel’s picture

Status: Active » Needs review
FileSize
1.31 KB

I 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

j0rd’s picture

Issue tags: +Field Permissions

I 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)

md2’s picture

I'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

Anonymous’s picture

FYI: 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.

ptmkenny’s picture

Status: Needs review » Reviewed & tested by the community

#3 also fixed this issue for me.

Taxoman’s picture

Version: 7.x-1.2 » 7.x-1.x-dev

how about pushing #3 to -dev?

jennypanighetti’s picture

Been sitting here for months, can we get this rolled in please?

muka’s picture

works for me too, thank you

Kojo Unsui’s picture

Hi,

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.

StoraH’s picture

Patch in #3 worked for me as well, thx!

marcus178’s picture

I'm getting the same error with adv agg turned on, but the patch above doesn't fix it.

KerriO’s picture

Will 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.

Anonymous’s picture

Patch is good. +1

stevenx’s picture

Patch from #3 works

hansrossel’s picture

Patch from #3 works

Ravenight’s picture

Patch from #3 works -- It was causing massive issues with the Rules module

david_garcia’s picture

This issue reappears when using the Advanced CSS/JS Aggregation module, even with patch applied:

https://drupal.org/project/advagg

https://drupal.org/node/2202851

david_garcia’s picture

Digging 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:

drupal_add_js(array('LoginToboggan' => array('preAuthID' => $id)), 'setting');

We should directly alter the javascript configuration array:

$javascript['settings']['data'][] = array('LoginToboggan' => array('preAuthID' => $id));
david_garcia’s picture

Found It!

Patch#3 is "wrong", this is what the documentation states:

Note that hook_js_alter(&$javascript) is called during this function call to allow alterations of the JavaScript during its presentation. Calls to drupal_add_js() from hook_js_alter() will not be added to the output presentation. The correct way to add JavaScript during hook_js_alter() is to add another element to the $javascript array, deriving from drupal_js_defaults(). See locale_js_alter() for an example of this.

The proper way of adding the addition setting is manually altering the &$javascript array.

Alauddin’s picture

#3 patch file with edit from #18 worked for me.

thanks

david_garcia’s picture

Status: Reviewed & tested by the community » Needs work
david_garcia’s picture

Closed #1407564 as a duplicate.

david_garcia’s picture

Status: Needs work » Needs review
FileSize
5.29 KB

I 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.

IWasBornToWin’s picture

Patch 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.

david_garcia’s picture

@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,

IWasBornToWin’s picture

you're correct....no patch and no permissions showing.

IWasBornToWin’s picture

Works excellent! Thanks

IWasBornToWin’s picture

Status: Needs review » Reviewed & tested by the community
Alauddin’s picture

@david_garcia_garcia

Thanks again for the patch #23

tested working on a new site I am working on.

tawellman’s picture

@david_garcia_garcia

Patch from #23 Works!! Thank You!!

rollingnet’s picture

The patch #23 is working for me, too.
Time to commit it to dev?

alex_sansom’s picture

Patch 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.

bisonbleu’s picture

Ran 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 !

NWOM’s picture

Version: 7.x-1.x-dev » 7.x-1.4

The 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!

Majdi’s picture

The patch #23 is working for me, too

kevinsiji’s picture

FileSize
5.66 KB

Patch rerolled against stable 7.x-1.4

Anybody’s picture

The 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?

Anybody’s picture

Priority: Normal » Major

Setting up the priority because the bug crashes JS in large parts of the Admin section.

Please commit ASAP. Thanks a lot!

Anybody’s picture

The 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.

Anybody’s picture

Corrected 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.

idebr’s picture

@Anybody your patch was created from outside the module directory. I rerolled it from inside the module directory.

DamienMcKenna’s picture

Version: 7.x-1.4 » 7.x-1.x-dev
md2’s picture

Testing 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.

Screenshot showing bug

To Reproduce:

  1. Run patch against clean 7.x-1.x-dev.
  2. Add a new role.
  3. Goto /admin/config/system/logintoboggan and set non-authenticated role to the new role.
  4. Goto /admin/people/permissions and make some changes to the authenticated user role permissions, and save.
  5. Revisit /admin/people/permissions and observe missing tickboxes under the new role anywhere were authenticated user has ticked permission.

If anyone can confirm that would be great! Once fixed we can get this merged to dev.

Thanks,
Mark

BBC’s picture

I'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.

goldlilys’s picture

Yep, 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 ?

rooby’s picture

I have had success with the patch in #41

sylvaticus’s picture

Patch in #41 also works over LoginToboggan 1.15.

ckng’s picture

Tested patch #41 working.

dooug’s picture

Status: Reviewed & tested by the community » Needs work

I'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.

scotwith1t’s picture

Status: Needs work » Reviewed & tested by the community

#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.

: patch -p1 < logintoboggan-1365764-41.patch
: patching file logintoboggan.module
: Hunk #3 succeeded at 1285 (offset 7 lines).
: patching file logintoboggan.permissions.js

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.

dooug’s picture

Status: Reviewed & tested by the community » Needs work

Thanks @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!

ndewhurst’s picture

#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.

-enzo-’s picture

Path in #52 by @ndewhurst works perfect for me. thanks

ndewhurst’s picture

Cool @-enzo-. Do you think we can go ahead and RTBC this?

-enzo-’s picture

oops my fault, changing to RBTC

-enzo-’s picture

Status: Needs review » Reviewed & tested by the community
rwilson0429’s picture

Patch in #52 works well for me too

travelertt’s picture

+1 RTBC for patch #52

rob.costello’s picture

#52 worked for me too.

andsigno82’s picture

Confirmed patch #52 worked for me too

Shane Birley’s picture

I 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.

joelstein’s picture

#52 works for me!

Anybody’s picture

Could someone please create a new (+stable) release? The module is still broken since month...

delacosta456’s picture

hi

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

Alauddin’s picture

#52 works.

fyi - applies to dev, not 1.5 version

asiby’s picture

I do not have logintoboggan ... yet I am having the same issue.

ndewhurst’s picture

@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']...

asiby’s picture

Good 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.

ThuleNB’s picture

Has 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.

C.E.A’s picture

Any news for a new release to automatically fix the issue mentioned above, instead of applying a patch ?

BeatnikDude’s picture

I to would like to see this patch to see this patch applied to dev. Thanks.

burnsjeremy’s picture

This 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.

DamienMcKenna’s picture

I think the module needs a new comaintainer.

burnsjeremy’s picture

Well 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 :)

deggertsen’s picture

Thanks for patch. Would love to see it applied!

matman476’s picture

#52 works like a charm!

cmseasy’s picture

patch #52 works, please commit

granticusiv’s picture

I've had patch #52 working on a production site for four months without issue. Please commit.

Johan den Hollander’s picture

#52 Fixes my problem. Should be committed.

gngn’s picture

Like 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).

PhilY’s picture

Coming 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.

Rafal Lukawiecki’s picture

I'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.

BeatnikDude’s picture

I 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.

timb’s picture

This 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.

cmseasy’s picture

Patch 52 also solves aa unresolved rules issue: https://www.drupal.org/project/rules/issues/2530440
Please commit

dkatena’s picture

patch #52 works for me!

Collins405’s picture

So 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?

Rafal Lukawiecki’s picture

It 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.

NWOM’s picture

I 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.

stevecowie’s picture

I'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.

  • ndewhurst authored c35279c on 7.x-1.x
    Issue #1365764 by ndewhurst, Anybody, md2, david_garcia, fangel,...
stevecowie’s picture

Assigned: Unassigned » stevecowie
Status: Reviewed & tested by the community » Fixed

Patch now applied to dev version. Thanks to all and apologies for the slow response.

DamienMcKenna’s picture

Thanks Stew!

Status: Fixed » Closed (fixed)

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

Rafal Lukawiecki’s picture

Thank you, @stevecowie, very much indeed!