AccountForm clean-up

Isn't this solved now that we have the form display screen ?

1. +++ b/core/modules/user/src/AccountForm.php
   @@ -112,59 +112,60 @@ public function form(array $form, array &$form_state) {
   +      '#access' => !$register && $user->id() == $account->id(),
   

   The user/account comparison is used in several places, probably a variable would be useful

2. +++ b/core/modules/user/src/AccountForm.php
   @@ -112,59 +112,60 @@ public function form(array $form, array &$form_state) {
   +      '#access' => (!empty($protected_values) && $user->id() == $account->id() && !$register),
   

   A comment here would be helpful

+++ b/core/modules/user/src/AccountForm.php
@@ -112,59 +112,60 @@ public function form(array $form, array &$form_state) {
+    if ($register && (!$config->get('verify_mail') || $admin)) {
       $form['account']['pass'] = array(
         '#type' => 'password_confirm',
         '#size' => 25,
         '#description' => $this->t('Provide a password for the new account in both fields.'),
-        '#required' => TRUE,
+        '#required' => $register && (!$config->get('verify_mail') || $admin),
+        '#access' => $register && (!$config->get('verify_mail') || $admin),

The same comparison is used for the if, access and required, is the if even needed?

+++ b/core/modules/user/src/AccountForm.php
@@ -113,54 +113,56 @@ public function form(array $form, FormStateInterface $form_state) {
+      $request_new = l($this->t('Request new password'), 'user/password', array('attributes' => array('title' => $this->t('Request new password via email.'))));

We no longer have to use l() but can rely on the link generator.

You just need something like in that drupal unit test.

public function setUp() {
  parent::setUp():


  $this->installSchema('system', ['router']);
}

1. new file mode 100644
   index 0000000..96827c1
   
   index 0000000..96827c1
   --- /dev/null
   
   --- /dev/null
   +++ b/core/2324113-color-1.patch
   
   +++ b/core/2324113-color-1.patch
   @@ -0,0 +1,98 @@
   

   nope!

2. +++ b/core/2324113-color-1.patch
   @@ -0,0 +1,98 @@
   + ¶
   ++use Drupal\Component\Utility\Color;
   ...
   + ¶
   ...
   + ¶
   ...
   + ¶
   ...
   + ¶
   ...
   + ¶
   ...
   + ¶
   

   whitespace all over the place.

3. +++ b/core/2324113-color-1.patch
   @@ -0,0 +1,98 @@
   diff --git a/core/lib/Drupal/Component/Utility/NestedArray.php b/core/lib/Drupal/Component/Utility/NestedArray.php
   
   diff --git a/core/lib/Drupal/Component/Utility/NestedArray.php b/core/lib/Drupal/Component/Utility/NestedArray.php
   index c494c24..8e00e06 100644
   
   index c494c24..8e00e06 100644
   --- a/core/lib/Drupal/Component/Utility/NestedArray.php
   
   --- a/core/lib/Drupal/Component/Utility/NestedArray.php
   +++ b/core/lib/Drupal/Component/Utility/NestedArray.php
   
   +++ b/core/lib/Drupal/Component/Utility/NestedArray.php
   +++ b/core/lib/Drupal/Component/Utility/NestedArray.php
   @@ -34,10 +34,10 @@ class NestedArray {
   
   @@ -34,10 +34,10 @@ class NestedArray {
       * $value = NestedArray::getValue($form, $parents);
       * @endcode
       *
   -   * A return value of NULL is ambiguous, and can mean either that the requested
   -   * key does not exist, or that the actual value is NULL. If it is required to
   -   * know whether the nested array key actually exists, pass a third argument
   -   * that is altered by reference:
   +   * The return value will be NULL, regardless of whether the actual value is
   +   * NULL or whether the requested key does not exist. If it is required to know
   +   * whether the nested array key actually exists, pass a third argument that is
   +   * altered by reference:
       * @code
       * $key_exists = NULL;
       * $value = NestedArray::getValue($form, $parents, $key_exists);
   

   This change doesn't seem to be related?

4. +++ b/core/modules/user/src/Tests/UserAccountFormFieldsTest.php
   @@ -26,6 +26,15 @@ class UserAccountFormFieldsTest extends DrupalUnitTestBase {
   +    \Drupal::service('router.builder')->rebuild();
   

   Do we really need this here? Afaik the router would rebuild itself is something requests it.

What about point 4?

+++ b/core/modules/user/src/AccountForm.php
@@ -113,54 +113,56 @@ public function form(array $form, FormStateInterface $form_state) {
+    if ($register && (!$config->get('verify_mail') || $admin)) {

Why not use #access for this too?

Do we have test coverage for the complex logic being touched here?

I authored UserAccountFormFieldsTest, but that only asserts the expected order of input elements (cf. #2191785: Password managers are identifying/storing wrong username field when creating a user account). — btw, not sure why the test suddenly needs to rebuild the router; a clean-up like this should not cause behavior changes.

My attempt at a reroll

Well, that was an impressive number of fails. Here's an improvement.

Well, that helped. But I've either completed messed this up or it needs someone with more knowledge than I currently have.

This should be better.

The exception has to do with making the link discussed in #20, #34 and #35. My patch keeps the original l(), but the same failure happens with getLinkGenerator as used by penyaskito.

$request_new = $this->getLinkGenerator()->generate($this->t('Request new password'), 'user.pass', array(), array('attributes' => array('title' => $this->t('Request new password via email.'))));

What method should be used to create the link, and then what to do about the test?

Thank you to YesCT and mpdonadio.

LinkGenerator is working and removed the the changes to /core/modules/user/src/Tests/UserAccountFormFieldsTest.php

@penyaskito Thanks for sorting that out. Just so you know I don't yet know how to implement what you suggest.

updated issue summary to try and specify why this is a problem.
we need to know so that we know if this should be postponed to 8.1.x per https://www.drupal.org/contribute/core/beta-changes

it looks like this is postponed to 8.1.x to me.

there are bunch of issue still open for 8.0.x, especially bugs... but here is a link to some usability things also
https://www.drupal.org/project/issues/search/drupal?project_issue_follow...

No longer postponded and looking at the current code this is still valid