Enabling the Unified registration page breaks the 'administrators only' account creation setting

i don't see the point of doing this -- it adds code bloat, and no one would sensibly use the unified login when visitors can't create an account. if for some reason you wanted to anyways, you can always form_alter or theme out the offending parts.

I can think of at least two reasons to do this:

1) Even if self-registration is turned off, the site owner may still want the unified option so that "Request new password" is rendered as a normal link instead of a tab.

2) When the unified option is on, the registration policy setting on the "Account settings" page is essentially broken - changing the policy there will not appear to have any effect. Site owners not familiar with LoginToboggan won't know that they also have to turn off the unified option in the "LoginToboggan" settings.

I'd work out a patch, unless you still think it's a won't fix.

I agree with @Les Lim; this should be addressed.

@Les Lim: i still think my point in #1 is valid, it's not asking site admins too much to be familiar with how to configure the modules they're using. ;)

i'll look at a patch, but would only commit it if it solved the problem in a way that didn't increase code complexity too much.

Just come across this and it does need to be fixed.

If the user registration type is set to 'Administrator only' and the unified option is checked, users are able to register to the site.

New users that register this way also get an empty email confirmation.

At the very least, the unified option needs to be unchecked and the field disabled if the 'administrator only' option is checked.

I agree that this module shouldn't show the unified form if only administrators are allowed to create an account. I found that even though accounts could be created, the generated email was blank and left me feeling rather confused.

Here's a patch to fix this, would be good to see this or similar added to future releases.

Trying this again without the hash in the filename :)

+++ b/logintoboggan.admin.inc
@@ -30,6 +30,14 @@ function logintoboggan_main_settings() {
+    $form['login']['logintoboggan_unified_login']['#description'] .= t("<p>This field has been disabled because only

The t() function shouldn't have HTML tags in it.

Also, this is related/duplicate: #1883170: Does not respect "administrator only registration" when "unified login" activated

I think the patch at #1883170-9: Does not respect "administrator only registration" when "unified login" activated contains some unnecessary code e.g. it still adds some tests on the Unifed Login Page theme callback despite already not allowing end users to check the box when user_register is set to USER_REGISTER_ADMINISTRATORS_ONLY.

The t() function shouldn't have HTML tags in it.

Correct me if I'm wrong although I think it's fine to have the <a> inside the t() function. I see it in some examples and you can easily translate it if it's inside.

Anyway, patch attached. Thanks!

@dsdeiz, ah you are right, I was overzealous to say the "t() function shouldn't have html". The recommendation in the Drupal documentation is this:

The general rule of thumb for translatable strings is that we keep inline markup for translation but avoid block tags

quoted from: https://www.drupal.org/node/322774

So, it might be good to pull out the <p></p> tags.

I re-rolled the patch with the <p></p> tags outside of the t() function.

Ah crap, I totally forgot about the <p> tag. Guess it's a no brainer. Marking as RTBC. :D

While testing I realized that this patch is insufficient in this case:
1. unified login is already set & visitors can create accounts
2. "Who can create an account" is changed to "administrators only"

The checkbox is disabled on the LT admin page. But the logintoboggan_unified_login variable remains set, and the unified login form still appears. Simply unchecking that checkbox is not sufficient, the setting's variable needs to be updated.

So, this needs work.

Further work proposal

Fix this so that when the "Present a unified login/registration page" setting is enabled, the "Who can register accounts?" form element should have a warning such as "Changing to 'Administrators only' will disable the unified login/registration page". To make this functional, it will also require adding a submit callback to the account settings page that will disable the unified login form setting variable if the account settings are changed to "Administrators only".

Any thoughts? I've update the issue description to provide a better overview of this issue's progress.

Updated description.

Also, tested that users can register accounts despite "Administrators Only" account creation settings being enabled. I confirmed that the email notification tokens are blank (as mentioned in #6 and the user will see these errors:

Notice: Undefined variable: mailkey in logintoboggan_user_register_submit() (line 524 of /var/www/sites/default-d7.dev/www/sites/all/modules/contrib/logintoboggan/logintoboggan.module).
Notice: Undefined variable: text in _user_mail_text() (line 2825 of /var/www/sites/default-d7.dev/www/modules/user/user.module).
Notice: Undefined variable: text in _user_mail_text() (line 2825 of /var/www/sites/default-d7.dev/www/modules/user/user.module).

Haven't read the whole part but about #1017106-14: Enabling the Unified registration page breaks the 'administrators only' account creation setting, I think this is addressed on #1935070: Notice: Undefined variable: mailkey in logintoboggan_user_register_submit that I tried to work on.