Empty "Region and language" fieldset on Registration page

[olafgrabienski]

Description of the bug

On localized Backdrop sites where visitors are allowed to register user accounts, there is an empty "Region and language" fieldset on the page "Create new account".

Steps To Reproduce

To reproduce the behavior:

1. On a vanilla Backdrop site, enable the Locale and the Language module.
2. Add a Language, e.g. Spanish, on admin/config/regional/language.
3. Allow visitors to register an user account on admin/config/people/settings.
4. Log out, and visit the page "Create new account" (user/register).

Actual behavior

There is an empty "Region and language" fieldset above the "Create new account" button. (See screenshot below.)

Expected behavior

No "Region and language" fieldset.

Additional info

Backdrop CMS version: 1.24.2 (Tugboat demo)

Screenshot:

[avpaderno]

Looking at the code on user_account_form(), I gather this can happen when #access is set to TRUE, but none of the hooks altering the registration form then adds form elements to that fieldset.

$form['region_language'] = array(
  '#type' => 'fieldset',
  '#title' => t('Region and language'),
  '#weight' => 6,
  '#access' => (!$register && config_get('system.date', 'user_configurable_timezones')) || (module_exists('locale') && language_multilingual()),
  '#group' => 'additional_settings',
);
// See system_user_timezone() and locale_language_selector_form() for the form
// items displayed here.

$register is set from the following line.

$register = ($form['#user']->uid > 0 ? FALSE : TRUE);

That happens because the value of #access verifies the value returned by config_get('system.date', 'user_configurable_timezones') is not a falsy value, but system_form_user_register_form_alter() verify config_get('system.date', 'user_configurable_timezones') does not return a falsy value and $config->get('user_default_timezone') returns BACKDROP_USER_TIMEZONE_SELECT (2).

  $config = config('system.date');
  if ($config->get('user_configurable_timezones') && $config->get('user_default_timezone') == BACKDROP_USER_TIMEZONE_SELECT) {
    system_user_timezone($form, $form_state);
  }

As for locale_form_alter(), the form element it adds to the registration form is only visible to the users with the administer users permissions.

  // Only alter user forms if there is more than one language.
  if (language_multilingual()) {
    // Display language selector when either creating a user on the admin
    // interface or editing a user account.
    if ($form_id == 'user_register_form' || $form_id == 'user_profile_form') {
      $selector = locale_language_selector_form($form['#user']);
      $selector['locale']['#type'] = 'container';
      $form['region_language'] += $selector;
      if ($form_id == 'user_register_form') {
        $form['region_language']['locale']['#access'] = user_access('administer users');
      }
    }
  }

The fix seems quite easy: Put in user_account_form() the same check used by system_form_user_register_form_alter().

$form['region_language'] = array(
  '#type' => 'fieldset',
  '#title' => t('Region and language'),
  '#weight' => 6,
  '#access' => (!$register && config_get('system.date', 'user_configurable_timezones')) && config_get('system.date', 'user_default_timezone') == BACKDROP_USER_TIMEZONE_SELECT || (module_exists('locale') && language_multilingual()),
  '#group' => 'additional_settings',
);
// See system_user_timezone() and locale_language_selector_form() for the form
// items displayed here.

[avpaderno]

(Let's wait tests run.)

[avpaderno]

Actually, for the language form elements to be shown, it should be sufficient that config_get('system.date', 'user_configurable_timezones') returns TRUE. The other configuration value should not make any difference.

[avpaderno]

I tested the PR preview following the steps described in the IS. This is what I see in the registration form (accessed as anonymous user).

(The timezone shown in the screenshot is the one I selected, not the default one shown by the form.)

[avpaderno]

As a side note, if I do not install the Locale and the Language modules, the registration form doesn't allow users to select a timezone.

The same happens when the Locale and the Language modules are installed, but no language is added.

I am not sure this is expected. A site could not need to be localized, but users should still be able to select their timezone, if the Users may set their own time zone setting has been selected.

[olafgrabienski]

Many thanks for the PR, @kiamlaluno ! Looks good on the sandbox site, works for me.

[olafgrabienski]

Oh, wait: After disabling the option "Users may set their own time zone" on admin/config/regional/settings, I see the empty "Region and language" fieldset again on the Registration page. In this case, it shouldn't be rendered at all, right?

[argiepiano]

Working with a multilanguage site:

Without the patch:

• If I select "Users may set their own time zone at registration." in admin/config/regional/settings and then clear the cache, the problem doesn't exist - I see the time zone selection select appears normally. 👍🏽
• However, if I select "Empty time zone" or "Default time zone" in that page, then I see the empty fieldset. 👎🏽

---

After applying the patch:

• If I select "Users may set their own time zone at registration." and clear the cache, the time zone select appearch 👍🏽
• If I select "Default time zone" of "Empty time zone" and clear the cache, the time zone still appears 👎🏽

So the PR doesn't solve the original problem, and unfortunately, it creates a new problem.

BTW: don't forget to clear caches every time you make a change. Anonymous users see cached pages otherwise, so your testing may give you false positives or negatives.

[avpaderno]

The issue is that what user_account_form() checks to show the region_language fieldset is not what system_form_user_register_form_alter() and locale_form_alter() check to add form elements to that fieldset.

The effect is that the fieldset is added, but no form element is added to that fieldset.

[avpaderno]

To make a summary:

• user_account_form() sets #access to (!$register && config_get('system.date', 'user_configurable_timezones')) || (module_exists('locale') && language_multilingual()) where $register is set with $form['#user']->uid > 0 ? FALSE : TRUE
• system_form_user_register_form_alter() adds the form elements with the following code

  $config = config('system.date');
  if ($config->get('user_configurable_timezones') && $config->get('user_default_timezone') == BACKDROP_USER_TIMEZONE_SELECT) {
  system_user_timezone($form, $form_state);
  }

• locale_form_alter() adds the form elements using the following code

  if (language_multilingual()) {
  // Display language selector when either creating a user on the admin
  // interface or editing a user account.
  if ($form_id == 'user_register_form' || $form_id == 'user_profile_form') {
    $selector = locale_language_selector_form($form['#user']);
    $selector['locale']['#type'] = 'container';
    $form['region_language'] += $selector;
    if ($form_id == 'user_register_form') {
      $form['region_language']['locale']['#access'] = user_access('administer users');
    }
  }
  }

[avpaderno]

Is the Region and language fieldset supposed to appear (with the form element to select a timezone) when a person registers an account?
I am asking because I think I misunderstood that part.

[olafgrabienski]

Is the Region and language fieldset supposed to appear (with the form element to select a timezone) when a person registers an account?

Good question. I don't recall seeing the element, but most of my sites aren't even open for visitor registration, so I'm not sure.

[avpaderno]

I will check what happens when I register a new account on d.org, which uses Drupal 7. If then it does not appear on d.org, I will check what code is used from Drupal 7.

(I forgot to add not on the previous sentence. 😅)

[avpaderno]

(And since I can see the user settings on d.org, I can also verify if that happens because how the site has been set.)

[olafgrabienski]

Interesting, when I disable the Locale module, the issue is gone.

• Enabled only Language module:
  • user/*/edit: the Region and Language vertical tab contains "Time zone"
  • user/register: the Region and language fieldset is not visible
• Enabled Language and Locale:
  • user/*/edit: the Region and Language vertical tab contains "Time zone" and "Language"
  • user/register: the Region and language fieldset: is visible but empty

[avpaderno]

On d.org, that fieldset is not shown.

I checked the settings they have on /admin/config/people/accounts, but none of the settings there is about allowing people who register an account to set their timezone.

[olafgrabienski]

Okay, let's see: Maybe the issue was introduced when Backdrop added vertical tabs to the user account form (related: https://github.com/backdrop/backdrop-issues/issues/5747) with version 1.23.0. I've just downgraded one of my test sites to Backdrop 1.22.2, and there I can't reproduce the issue.

[avpaderno]

On Drupal 7 the Region and language fieldset is not shown because user_account_form() does not add it. I also checked user_profile_form() and user_registration_form() (the functions that call user_account_form()), but those functions do not add the Region and language fieldset either.

[avpaderno]

The issue is definitively in (!$register && config_get('system.date', 'user_configurable_timezones')) || (module_exists('locale') && language_multilingual()). It returns TRUE, makes the fieldset visible to people who register an account, but then the other modules do not add any field. That is why is empty.

In fact, when $register is TRUE, config_get('system.date', 'user_configurable_timezones') returns TRUE, the Local module is installed and the site is multi-lingual, the value of (!$register && config_get('system.date', 'user_configurable_timezones')) || (module_exists('locale') && language_multilingual()) is TRUE ((!TRUE && TRUE) || (TRUE && TRUE)).

I would change the code to match the Drupal 7 code, but I would like to understand why Backdrop uses that code.

(I forgot some parentheses in the code.)

[avpaderno]

Finally, this is the code for Drupal 7 system_form_register_form() and locale_form_alter().

function system_form_user_register_form_alter(&$form, &$form_state) {
  if (variable_get('configurable_timezones', 1)) {
    if (variable_get('user_default_timezone', DRUPAL_USER_TIMEZONE_DEFAULT) == DRUPAL_USER_TIMEZONE_SELECT) {
      system_user_timezone($form, $form_state);
    }
    else {
      $form['account']['timezone'] = array(
        '#type' => 'hidden',
        '#value' => variable_get('user_default_timezone', DRUPAL_USER_TIMEZONE_DEFAULT) ? '' : variable_get('date_default_timezone', ''),
      );
    }
    return $form;
  }
}

function locale_form_alter(&$form, &$form_state, $form_id) {

  // Only alter user forms if there is more than one language.
  if (drupal_multilingual()) {

    // Display language selector when either creating a user on the admin
    // interface or editing a user account.
    if ($form_id == 'user_register_form' || $form_id == 'user_profile_form' && $form['#user_category'] == 'account') {
      locale_language_selector_form($form, $form_state, $form['#user']);
    }
  }
}

system_user_timezone() adds the Locale settings fieldset ($form['timezone']), while locale_language_selector_form() add the Language settings fieldset ($form['locale']).

[avpaderno]

For the #access value in user_account_form(), I used (!$register && config_get('system.date', 'user_configurable_timezones')) || (!$register && module_exists('locale') && language_multilingual()). I was going to simplify as suggested from https://www.calculators.tech/boolean-algebra-calculator, but I preferred to wait for that.

(The site seems to suggest (!$register) && (config_get('system.date', 'user_configurable_timezones') || module_exists('locale') && language_multilingual()), but I am not sure it would respect the PHP operator precedence.)

[avpaderno]

The test that fails is LocaleUserCreationTest::testLocalUserCreation(), but it tests the $langcode . '/admin/people/create route, not the one used by the registration form.

// Check if the language selector is available on admin/people/create and
// set to the currently active language.
$this->backdropGet($langcode . '/admin/people/create');
$this->assertFieldChecked("edit-language-$langcode", 'Global language set in the language selector.');

[avpaderno]

At least, the empty fieldset is not shown. I also disabled Cache pages for anonymous users to avoid getting a cached page.

[argiepiano]

This is a great start, @kiamlaluno, but we can't ignore those test failures. It happened in all 3 PHP versions. Would you have a chance to figure out what in your PR is creating those failures?

[avpaderno]

I checked /admin/people/create on a different preview site. Effectively, the Region and language fieldset is shown.

On the preview site for my PR, it does not appear.

That is a bit strange, since !$register should be FALSE for anonymous users and TRUE for logged-in users.

[avpaderno]

No, $register is set to $form['#user']->uid > 0 ? FALSE : TRUE. It does not check the logged-in user ID. If an administrator user is creating an account, $form['#user']->uid is still 0, since user_register_form() set $form['#user'] with the following line.

$form['#user'] = entity_create('user', array());

UserStorageController::create() does not set the user ID. (That is expected; the user ID will be set when the user object is saved.)

It seems that the code should check the currently logged-in user. If its user ID is zero, then is a visitor who is registering an account; if the user ID is not zero, it must be an administrator user who is creating an account (or somebody is editing an existing account).

[avpaderno]

Now tests are failing because connection time-outs.

Warning: Failed to download action 'https://api.github.com/repos/shogo82148/actions-setup-mysql/tarball/797491cf45b78ab2d4329bfbbadca16e25ad6dc6'. Error: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing.

Error: HttpError: request to https://api.github.com/repos/backdrop/backdrop/pulls/4412/commits failed, reason: connect ETIMEDOUT 192.30.255.116:443

[argiepiano]

If its user ID is zero, then is a visitor who is registering an account; if the user ID is not zero, it must be an administrator user who is creating an account.

The standard behavior for visiting user/register when you are logged in, is to redirect you to user/X/edit. So, no, administrators cannot create accounts by going to user/register.

The only way for administrators to create accounts is to visit admin/people/create, and that's a different form.

[avpaderno]

Administrator users do not visit user/register to create an account. They just get the same form when they visit admin/people/create.

  $items['admin/people/create'] = array(
    'title' => 'Add user account',
    'page callback' => 'backdrop_get_form',
    'page arguments' => array('user_register_form'),
    'access arguments' => array('administer users'),
    'type' => MENU_LOCAL_ACTION,
  );

user_register_form() then calls user_account_form().

$form['#user'] = entity_create('user', array());

$form['#attached']['library'][] = array('system', 'jquery.cookie');
$form['#attributes']['class'][] = 'user-info-from-cookie';

// Start with the default user account fields.
user_account_form($form, $form_state);

user_account_form() is also used when people edit their own account. Checking the logged-in user ID allows to understand when a visitor is registering an account (which is when that fieldset should not be shown).

[avpaderno]

Between 9 hours, I will close and re-open the PR to let the tests start again. (It is past midnight for me.)

[avpaderno]

All tests pass.

[olafgrabienski]

@kiamlaluno Thanks for the updated PR! I've tested the sandbox site with the 'Steps to reproduce', and the fix works for me: The (empty) "Region and language" fieldset on the "Create new account" page isn't there anymore.

For curiosity: Do you think the issue was introduced when Backdrop 1.23.0 added vertical tabs to the user account form in #3872 and #5747? Or became it just visible then, or is it not related at all?

[avpaderno]

@olafgrabienski I would say they were "fooled" by the variable name as I was. I thought that $register would be TRUE when a visitor registers an account, but it is TRUE when an account is created and FALSE when an account is edited.

That variable name has been inherited from Drupal 7; the only difference in Drupal 7 is that user_account_form() does not add that fieldset.

[argiepiano]

Thanks @kiamlaluno

I may be misunderstanding what you are trying to do or what is the expected behavior, but this is what I'm seeing now:

1. I applied the latest patch
2. I set up my site as multilingual, and installed another language
3. I went to admin/config/regional/settings and enabled "Users may set their own time zone"
4. I clicked "Users may set their own time zone at registration."
5. I flushed all caches

Now, I'm logging out and going to user/register

I don't see the fieldset that allows me to set up my timezone at registration

This behavior, to me, sounds very confusing. I expressly selected "Users may set their own time zone at registration" yet I'm not able to set my timezone.

Can you let me know what I'm missing?

[olafgrabienski]

Oh, sorry, I forgot to test the "Users may set their own time zone at registration" setting.

[avpaderno]

I apologize: It is me who forgot what already said about that setting.

I need to add a condition more in the if() statement I changed.

[avpaderno]

The preview site shows the Region and language fieldset with the timezone selector to visitors who want to register an account. I set the preview site to avoid caching pages for anonymous users.

User #ㅤ1 see that fieldset too.

When Users may set their own time zone is not selected, visitors who register an account are not allowed to set their time zone.

ㅤUser #ㅤ1 still see the fieldset when creating an account.

[avpaderno]

Tests passes.

[olafgrabienski]

Thanks for another PR update! Works for me in the sandbox, including the different behavior depending on the "Users may set their own time zone at registration" setting.

[argiepiano]

Hi @kiamlaluno

Thanks so much again... but sorry to keep going back to this issue, but there are still several inconsistencies.

I think this may be due in part to lack of clarity in the current configuration UI. Specifically, in admin/config/regional/settings the three options: "Default time zone", "Empty time zone" and "Users may set their own time zone at registration" can be confusing.

I'm basing this functionality on the behavior in Drupal 7, which I believe does it correctly, and its behavior clarifies the meaning of those options.

I'm looking ONLY at the new user registration form, and ONLY when "Users may set their own time zone." is selected. Everything else works fine

In DRUPAL 7 (having selected "Users may set their own time zone."):

• Selecting "Default time zone" HIDES the selection of time zone in the New User Registration form. When you register as a new user, the default site timezone is automatically assigned to you (the registration form does not give the option of changing that). Users, however, can change their time zone after they've registered by logging in and editing their own account.

• Selecting "Empty time zone" also HIDES the selection of time zone in the New User Registration form. When you register as a new user, the site automatically saves a "blank" time zone for you. However, you can go back and edit that once you are able to log in the site

• Selecting "Users may set their own time zone at registration" is the ONLY option that should show the time zone select in the New User Registration form.

So, with your patch, unfortunately, the Time Zone selection is shown for ALL three options listed above. This is incorrect.

So, to me, this needs work. And perhaps we can clarify the UI to indicate that these 3 options ONLY apply to the New User Registration form, for example:

Time zone in new user registration form: [] Hide time zone selection widget and assign default time zone. [] Hide time zone selection widget and assign empty time zone. [] Show time zone selection widget. Users may set their own time zone at registration.

[avpaderno]

I am not able to change the code and avoid tests fail. IMO, the code should be changed a little more than the necessary to fix this issue. As it is, the User module sets the access for the Region and language fieldset to which the System module and the Locale module add their own form elements. It would be simpler if the System module and the Locale module would set #access to TRUE when they add their form elements and the User module initially set #access to FALSE.

[avpaderno]

To give more details:

• user_account_form(), which is called when an existing user account is edited (even from the account owner) or a user account is created (from users with the administer users permission or from anonymous users, when people are allowed to register an account) adds the Region and language fieldset, without knowing whether the System or the Locale module is going to add form elements to that fieldset.
• system_form_user_profile_form_alter() adds form elements under a condition (if (config_get('system.date', 'user_configurable_timezones')) { /** **/ }).
• system_form_user_register_form_alter() adds form element under a different condition (if ($config->get('user_configurable_timezones') && $config->get('user_default_timezone') == BACKDROP_USER_TIMEZONE_SELECT) { /** **/ }).
• locale_form_alter() adds its own form elements under a third condition (if (language_multilingual()) { /** **/ }), but then its form element is only accessible to users with the administer users permission, when the form is used to register an account.