Issue #2191785 by sun: Password managers are identifying/storing wrong...

Issue #2191785 by sun: Password managers are identifying/storing wrong username field when creating a user account.

core/lib/Drupal/Core/Installer/Form/SiteConfigureForm.php

@@ -147,26 +147,23 @@ public function buildForm(array $form, array &$form_state) {
       '#title' => $this->t('Site maintenance account'),
     );
     $form['admin_account']['account']['#tree'] = TRUE;
+    $form['admin_account']['account']['mail'] = array(
+      '#type' => 'email',
+      '#title' => $this->t('E-mail address'),
+      '#required' => TRUE,
+    );
     $form['admin_account']['account']['name'] = array(
       '#type' => 'textfield',
       '#title' => $this->t('Username'),
       '#maxlength' => USERNAME_MAX_LENGTH,
       '#description' => $this->t('Spaces are allowed; punctuation is not allowed except for periods, hyphens, and underscores.'),
       '#required' => TRUE,
-      '#weight' => -10,
       '#attributes' => array('class' => array('username')),
     );
-    $form['admin_account']['account']['mail'] = array(
-      '#type' => 'email',
-      '#title' => $this->t('E-mail address'),
-      '#required' => TRUE,
-      '#weight' => -5,
-    );
     $form['admin_account']['account']['pass'] = array(
       '#type' => 'password_confirm',
       '#required' => TRUE,
       '#size' => 25,
-      '#weight' => 0,
     );
 
     $form['regional_settings'] = array(

core/modules/user/lib/Drupal/user/AccountForm.php

@@ -82,20 +82,6 @@ public function form(array $form, array &$form_state) {
       '#weight' => -10,
     );
 
-    // Only show name field on registration form or user can change own username.
-    $form['account']['name'] = array(
-      '#type' => 'textfield',
-      '#title' => $this->t('Username'),
-      '#maxlength' => USERNAME_MAX_LENGTH,
-      '#description' => $this->t('Spaces are allowed; punctuation is not allowed except for periods, hyphens, apostrophes, and underscores.'),
-      '#required' => TRUE,
-      '#attributes' => array('class' => array('username'), 'autocorrect' => 'off', 'autocomplete' => 'off', 'autocapitalize' => 'off',
-      'spellcheck' => 'false'),
-      '#default_value' => (!$register ? $account->getUsername() : ''),
-      '#access' => ($register || ($user->id() == $account->id() && $user->hasPermission('change own username')) || $admin),
-      '#weight' => -10,
-    );
-
     // The mail field is NOT required if account originally had no mail set
     // and the user performing the edit has 'administer users' permission.
     // This allows users without e-mail address to be edited and deleted.
@@ -105,7 +91,23 @@ public function form(array $form, array &$form_state) {
       '#description' => $this->t('A valid e-mail address. All e-mails from the system will be sent to this address. The e-mail address is not made public and will only be used if you wish to receive a new password or wish to receive certain news or notifications by e-mail.'),
       '#required' => !(!$account->getEmail() && $user->hasPermission('administer users')),
       '#default_value' => (!$register ? $account->getEmail() : ''),
-      '#attributes' => array('autocomplete' => 'off'),
+    );
+
+    // Only show name field on registration form or user can change own username.
+    $form['account']['name'] = array(
+      '#type' => 'textfield',
+      '#title' => $this->t('Username'),
+      '#maxlength' => USERNAME_MAX_LENGTH,
+      '#description' => $this->t('Spaces are allowed; punctuation is not allowed except for periods, hyphens, apostrophes, and underscores.'),
+      '#required' => TRUE,
+      '#attributes' => array(
+        'class' => array('username'),
+        'autocorrect' => 'off',
+        'autocapitalize' => 'off',
+        'spellcheck' => 'false',
+      ),
+      '#default_value' => (!$register ? $account->getUsername() : ''),
+      '#access' => ($register || ($user->id() == $account->id() && $user->hasPermission('change own username')) || $admin),
     );
 
     // Display password field only for existing users or when user is allowed to
@@ -166,6 +168,16 @@ public function form(array $form, array &$form_state) {
       );
     }
 
+    // When not building the user registration form, prevent web browsers from
+    // autofilling/prefilling the email, username, and password fields.
+    if ($this->getOperation() != 'register') {
+      foreach (array('mail', 'name', 'pass') as $key) {
+        if (isset($form['account'][$key])) {
+          $form['account'][$key]['#attributes']['autocomplete'] = 'off';
+        }
+      }
+    }
+
     if ($admin) {
       $status = $account->isActive();
     }

core/modules/user/lib/Drupal/user/Tests/UserAccountFormFieldsTest.php

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\user\Tests\UserAccountFormFieldsTest.
+ */
+
+namespace Drupal\user\Tests;
+
+use Drupal\simpletest\DrupalUnitTestBase;
+
+/**
+ * Tests field order and element attributes in user account forms.
+ */
+class UserAccountFormFieldsTest extends DrupalUnitTestBase {
+
+  /**
+   * Modules to enable.
+   *
+   * @var array
+   */
+  public static $modules = array('system', 'user', 'entity', 'field');
+
+  public static function getInfo() {
+    return array(
+      'name' => 'User account form fields',
+      'description' => 'Verifies that the field order in user account forms is compatible with password managers of web browsers.',
+      'group' => 'User',
+    );
+  }
+
+  /**
+   * Tests the root user account form section in the "Configure site" form.
+   */
+  function testInstallConfigureForm() {
+    require_once DRUPAL_ROOT . '/core/includes/install.core.inc';
+    $install_state = install_state_defaults();
+    $form_state = array();
+    $form_state['build_info']['args'][] = &$install_state;
+    $form = $this->container->get('form_builder')
+      ->buildForm('Drupal\Core\Installer\Form\SiteConfigureForm', $form_state);
+
+    // Verify name and pass field order.
+    $this->assertFieldOrder($form['admin_account']['account']);
+
+    // Verify that web browsers may autocomplete the email value and
+    // autofill/prefill the name and pass values.
+    foreach (array('mail', 'name', 'pass') as $key) {
+      $this->assertFalse(isset($form['account'][$key]['#attributes']['autocomplete']), "'$key' field: 'autocomplete' attribute not found.");
+    }
+  }
+
+  /**
+   * Tests the user registration form.
+   */
+  function testUserRegistrationForm() {
+    // Install default configuration; required for AccountFormController.
+    $this->installConfig(array('user'));
+
+    // Disable email confirmation to unlock the password field.
+    $this->container->get('config.factory')
+      ->get('user.settings')
+      ->set('verify_mail', FALSE)
+      ->save();
+
+    $form = $this->buildAccountForm('register');
+
+    // Verify name and pass field order.
+    $this->assertFieldOrder($form['account']);
+
+    // Verify that web browsers may autocomplete the email value and
+    // autofill/prefill the name and pass values.
+    foreach (array('mail', 'name', 'pass') as $key) {
+      $this->assertFalse(isset($form['account'][$key]['#attributes']['autocomplete']), "'$key' field: 'autocomplete' attribute not found.");
+    }
+  }
+
+  /**
+   * Tests the user edit form.
+   */
+  function testUserEditForm() {
+    // Install default configuration; required for AccountFormController.
+    $this->installConfig(array('user'));
+
+    $form = $this->buildAccountForm('default');
+
+    // Verify name and pass field order.
+    $this->assertFieldOrder($form['account']);
+
+    // Verify that autocomplete is off on all account fields.
+    foreach (array('mail', 'name', 'pass') as $key) {
+      $this->assertIdentical($form['account'][$key]['#attributes']['autocomplete'], 'off', "'$key' field: 'autocomplete' attribute is 'off'.");
+    }
+  }
+
+  /**
+   * Asserts that the 'name' form element is directly before the 'pass' element.
+   *
+   * @param array $elements
+   *   A form array section that contains the user account form elements.
+   */
+  protected function assertFieldOrder(array $elements) {
+    $name_index = 0;
+    $name_weight = 0;
+    $pass_index = 0;
+    $pass_weight = 0;
+    $index = 0;
+    foreach ($elements as $key => $element) {
+      if ($key === 'name') {
+        $name_index = $index;
+        $name_weight = $element['#weight'];
+        $this->assertTrue($element['#sorted'], "'name' field is #sorted.");
+      }
+      elseif ($key === 'pass') {
+        $pass_index = $index;
+        $pass_weight = $element['#weight'];
+        $this->assertTrue($element['#sorted'], "'pass' field is #sorted.");
+      }
+      $index++;
+    }
+    $this->assertEqual($name_index, $pass_index - 1, "'name' field ($name_index) appears before 'pass' field ($pass_index).");
+    $this->assertTrue($name_weight < $pass_weight, "'name' field weight ($name_weight) is smaller than 'pass' field weight ($pass_weight).");
+  }
+
+  /**
+   * Builds the user account form for a given operation.
+   *
+   * @param string $operation
+   *   The entity operation; one of 'register' or 'default'.
+   *
+   * @return array
+   *   The form array.
+   */
+  protected function buildAccountForm($operation) {
+    // @see HtmlEntityFormController::getFormObject()
+    $entity_type = 'user';
+    $fields = array();
+    if ($operation != 'register') {
+      $fields['uid'] = 2;
+    }
+    $entity = $this->container->get('entity.manager')
+      ->getStorage($entity_type)
+      ->create($fields);
+    $form_object = $this->container->get('entity.manager')
+      ->getFormObject($entity_type, $operation)
+      ->setEntity($entity);
+
+    // @see EntityFormBuilder::getForm()
+    return $this->container->get('entity.form_builder')->getForm($entity, $operation);
+  }
+
+}