getUsername() should return the username getDisplayName() for the formatted user name

Not sure. getUsername() *is* the replacement for user_format_name().

Most use cases in core use it like that. If we change it, then we need to provide an alternative. Or, provide a new method to get the raw name, where appropriate.

getUsername() is the same as label() (user specific vs. entity generic, like getTitle() and label() on nodes), which *always* returned the formatted username. This is the one to use in most cases, so I think it should be the default.

@scor and I talked to @moshe weitzman and he agreed that getUsername should return the user's unique log in name and that another method should return the display name. Most of core and contrib should call getDisplayName but getting access to the value needed for log in seems legit.

This patch adds a getDisplayName method to the AccountInterface and updates the method to UserSession class and User entity class.

Fixed failed tests, added test coverage.

forgot to update comment user name

+++ b/core/lib/Drupal/Core/Session/AccountInterface.php
@@ -112,7 +112,18 @@ public function getPreferredLangcode($default = NULL);
+   *   An unsanitized string with the unique username. The code receiving
+   *   this result must ensure that \Drupal\Component\Utility\String::checkPlain()
+   *   is called on it before it is
+   *   printed to the page.

Weird wrapping, the last part shouldn't be on a separate line.

Patch looks good, but I think we need to review the current usage of getUsername() and change to getDisplayName() wherever we want to display the formatted username.

Some that I think need to be updated:

- comment_preview():

    if (!empty($account) && $account->isAuthenticated()) {
      $comment->setOwner($account);
      $comment->setAuthorName(check_plain($account->getUsername()));
    }
    elseif (empty($author_name)) {
      $comment->setAuthorName(\Drupal::config('user.settings')->get('anonymous'));
    }

Can be simplified a lot with getDisplayName(). That said, comment author name is actually a bit tricky, see also preSave(). Not sure what to do there, as it is used as the form value in some cases... But not using the display name would mean that the alter wouldn't work?

- ContactController::contactPersonalPage()
- template_preprocess_forums()
- Rss::render()
- Not sure why rdf_preprocess_user() in the referenced issue did not want to use the display name?
- shortcut_set_switch_submit()
- A few in user.module I think and user.pages.inc I think
- RegisterForm
- UserAutocomplete::getMatches()
- UserSearch

I addressed following:
- ContactController::contactPersonalPage()
- template_preprocess_forums()
- Rss::render() (only node)
- Not sure why rdf_preprocess_user() in the referenced issue did not want to use the display name?
- shortcut_set_switch_submit()
- A few in user.module I think and user.pages.inc I think
- UserAutocomplete::getMatches()
- UserSearch

lets see what test bot says.

Comments are more tricky. I think for the author field we should already store the displayName value. However there are two places where the author value is used to load the user - comment_preview() and CommentFormController::submit(). I suggest to rework these two places to load the user entity differently and then the preSave() can directly set as the author the displayName.

Something else that I noticed is the user:name token. We probably need a token there for the raw username and one for the formatted, if you look at things like the registration confirmation mails, they use user:name twice, when addressing the user, it should use the formatted version and when it's used for the login information, it should be the actual user name.

psr-4 re-roll.

Looks like a random fail:

Drupal\system\Tests\Cache\ApcuBackendUnitTest
Created time is correct. Other GenericCacheBackendUnitTestBase.php 172 Drupal\system\Tests\Cache\GenericCacheBackendUnitTestBase->testSetGet()

13: 2112679-get_username-13.patch queued for re-testing.

I'll work on updating token support and the comment module related issues.

Updated the user token support and references to the [user:name] token that I could find.

[user:name] should probably get $user->getUsername() since D7 doesn't use user_format_name() when it does the replacements for e-mails, filepaths.

Also, [comment:author:name] and [node:author:name] need to be updated. This is probably the trickiness @blueminds was referring to in #10 as the comment auth values are stored in the comment table, not pulled from an associated user. Further, some places (according to #10) might use the stored name in the comment table to retrieve the user object. If we started storing the altered username those things would break.

Actually, if you look at d7 user_tokens(), it *does* use format_username(), as does 8.x right now.

Dave Reid might be able to help here, he maintains realname.module, one of the main reasons for that hook to even exist and he maintains the token module and I think I've seen him mentioning this problem before I think.

Ok. New patch adds the [user:username] token. [user:name] is mapped to getDisplayName. I also found some other instances of user_format_name().

Still need to figure the node:author:name token and the comment->author (including the token).

just to have the testbot trigger

+++ b/core/modules/user/user.tokens.inc
@@ -25,7 +25,11 @@ function user_token_info() {
+    'name' => t("Display Name"),

As far as I remember, to distinguish between processed and non-processed value the token module uses "raw" in the token name/description. Shouldn't this pattern be used here as well to clearly communicate if the username value is processed or comes directly from the database?

@blueminds good point. I'll check token module and re-roll the patch to keep it consistent.

Token module itself doesn't define a "raw" token. Realname does, but it also has a different name for the "display name". I'm going to leave as is for now in case anyone else has an opinion on the token naming.

Diving into the Comment author related stuff mentioned in #24.

The comment stuff was weird, as @blueminds mentioned the form submit and preview were using the author name to load the user and set the comment owner.

Instead I followed the advice of the "todo" and set the uid for the comment in the form constructor. Then the parent form submit builds the comment entity and sets the owner properly.

Patch attached.

Unassigning and setting to needs review.

Thanks, looks good!

A few comments:

1. +++ b/core/lib/Drupal/Core/Session/UserSession.php
   @@ -217,6 +217,14 @@ function getPreferredAdminLangcode($default = NULL) {
      public function getUsername() {
   +    $name = $this->name ?: '';
   +    return $name;
   +  }
   

   This can be on one line, no need to create a new variable here. Same below in User::getUsername()

2. +++ b/core/modules/user/user.tokens.inc
   @@ -25,7 +25,11 @@ function user_token_info() {
      $user['name'] = array(
   -    'name' => t("Name"),
   +    'name' => t("Display Name"),
   +    'description' => t("The display name of the user account."),
   +  );
   +  $user['username'] = array(
   +    'name' => t("Login Name"),
        'description' => t("The login name of the user account."),
      );
   

   I still think this is not self explanatory enough. If you look into template_preprocess_username() there we have a variable 'name_raw'. The token however uses just 'name' and introduces 'username'. Moreover we have the API method getDisplayName(). So we have four namings and just two things. I suggest either the token to use name_raw for the DB value or let's introduce 'display_name'. I prefer the later as we already have getDisplayName(), so making it uniform makes sense (also in the templates). To justify: if you look at 'name' and 'username' none of them really says which one comes formatted.

+++ b/core/modules/user/user.module
@@ -638,7 +638,7 @@ function template_preprocess_username(&$variables) {
+  $name = $variables['name_raw'] = $account->getDisplayName();

Also this seems to be not right, as the raw value is expected to be as *is* from DB. getDisplayName() will get already processed value.

@blueminds I know what you mean. However, see @scor's comment in #20. He makes a good point that people moving from D7 to D8 will need to update all their [user:name] tokens. Perhaps we leave [user:name] as "deprecated" and map it and a new token [user:displayname] to getDisplayName().

RE: #32 it's definitely not intuitive but that's the functionality in D7: https://api.drupal.org/api/drupal/includes%21theme.inc/function/template...

Per the comments "name_raw" in template_preprocess_username() is still the formatted username--but unshortened and unsanitized. Do we want to change the meaning of "name_raw" in that context?

I'll roll a patch with fixes for 1. in #31.

Here's the patch re-rolled to now reduce the size of the getUsername methods per first part of #31.

Yup, I see. Other than that, I think it can be RTBC. Me coded part of it, so someone else please :)

The views user name field doesn't work correctly yet I think, create a list of users as table with fields, the display formatted user name (which is checked by default) has no effect.

#734878: [user:name] tokens should be using format_username() and use [user:name-raw] in email templates is related, whatever we do should probably follow that...

34: user_getusername-2112679-34.patch queued for re-testing.

Reroll, will test.

Tested: The "Use formatted username" in views doesn't change anything.

How do you get a formatted name that is different form the raw name?

I created a user_format_name_alter() and that changes the name (Is that what a formatted name is?), but the views setting doesn't affect the altered name; It always uses the altered name. Tried username_alter() but that didn't change the name at all.

I guess you still had the link to user option checked too? This will then use theme_username regardless, which means you will always get the display name if you check to use a link. I will open a separate issue for that.

Actually, might as well roll it in here. It's dependent on this anyway.

@lokapujya, these changes should work as expected. If you could test again, that would be great. This is a quick patch - I don't have much time. Should work though :)

#44 fixes the problem mentioned in #41, however it seems like renderLink() needs an authenticated user. I'm not sure how to fix it, but here is some code that is passing tests for me.

That's a revert of what I changed in #44. It works but it is not right IMO - It's ugly and hacky. That's an artefact of D7 views.

We now have the entity attached to a row. Hence the call to getEntity() instead. So we need to get it working using that.

Thinking about it, what exactly is not working for you with the patch in #44?

getEntity() will return an entity which is what we want - just without the hackery. The tests just need adapting to reflect doing this properly IMO.

what exactly is not working for you with the patch in #44?

Nothing, I didn't mean so suggest that the code should be reverted; Just showing the exact part of the change that broke tests as they existed.

The remaining test failure is passing on my local environment.
EDIT: Well it was, until I did a git clean, and reapplied.

Changed test to use the views result from the new user. Removed an unused variable.

Edit: Left in a debug that is not highlighted in the interdiff.

OK. Looks pretty good to me. I didn't actually test the patch, just converted the test. It was using the same Jacky approach that the code was before :) do you want to remove that debug? We are prod looking pretty good then I think.

Looks like the actual patch is correct (with debug() removed) but just the wrong interdiff attached?

Also, if possible, could you use the name of the issue-comment in the interdiff too? d.o will hate you for calling another file interdiff.txt :)

interdiff do-over. :)

Yep :) looks good.

Re-roll.

Another re-roll. Setting to a major bug because if you save the user edit form with a module that alters usernames, you end up with a different username and crazy bugs like that...

Ups, had some junk in that patch file.

Agrh. Just started this issue https://www.drupal.org/node/2311219 and then saw this topic.
Maybe we should only add getDisplayName() and change logic of getUserName() in this issue, and do not replace user_format_name occurrences, or just close that issue ?
Anyway with the latest path 'label_callback' from core/modules/user/src/Entity/User model is till using user_format_name() function.
Need to deal with it.

Let's see how this gets on. Amended the deprecated message to make it correct too.

I think we should not tackle that in this issue, as the callback will need to pass the callable from the annotation which we currently cannot do.

I lost the test module along the way. Also added an explicit sort on that test view, the order of users was not reliable, not sure how that is related to this issue, though.

Ah ha! I assumed they were still in there :)

1. +++ b/core/lib/Drupal/Core/Session/AccountInterface.php
   @@ -114,7 +114,17 @@ public function getPreferredLangcode($default = NULL);
   +   *   An unsanitized string with the unique username. The code receiving this
   +   *   result must ensure that \Drupal\Component\Utility\String::checkPlain()
   +   *   is called on it before it is printed to the page.
   
   @@ -123,13 +133,13 @@ public function getPreferredAdminLangcode($default = NULL);
   -   *   An unsanitized string with the username to display. The code receiving
   +   * @return string
   +   *   An unsanitized string with the user name to display. The code receiving
       *   this result must ensure that \Drupal\Component\Utility\String::checkPlain()
       *   is called on it before it is
       *   printed to the page.
   

   It would be great if we could mention that the best way would be to use this just in templates.

2. +++ b/core/modules/action/src/Plugin/Action/EmailAction.php
   @@ -135,7 +135,7 @@ public function buildConfigurationForm(array $form, array &$form_state) {
   +      '#description' => t('The message that should be sent. You may include placeholders like [node:title], [user:name], [user:username] and [comment:body] to represent data that will be different each time message is sent. Not all placeholders will be available in all contexts.'),
   

   I wonder whether we could avoid confusing if there would be user:displayname and user:username instead.

3. +++ b/core/modules/comment/comment.module
   @@ -759,22 +759,6 @@ function comment_preview(CommentInterface $comment, array &$form_state) {
        // Attach the user and time information.
   

   I guess we can drop the user bit of the documentation.

4. +++ b/core/modules/comment/src/CommentForm.php
   @@ -212,6 +212,12 @@ public function form(array $form, array &$form_state) {
    
   +    // Set the uid on new comments to the current user.
   +    $form['uid'] = array(
   +      '#type' => 'value',
   +      '#value' => $comment->id() ? $comment->getOwnerId() : $this->currentUser->id(),
   +    );
   +
   
   @@ -308,13 +314,7 @@ public function submit(array $form, array &$form_state) {
   -    // If the comment was posted by a registered user, assign the author's ID.
   -    // @todo Too fragile. Should be prepared and stored in comment_form()
   -    // already.
        $author_name = $comment->getAuthorName();
   -    if (!$comment->is_anonymous && !empty($author_name) && ($account = user_load_by_name($author_name))) {
   -      $comment->setOwner($account);
   -    }
   

   nice!

5. +++ b/core/modules/user/src/Plugin/views/field/Name.php
   @@ -79,26 +69,29 @@ public function buildOptionsForm(&$form, &$form_state) {
      protected function renderLink($data, ResultRow $values) {
   -    $account = entity_create('user');
   -    $account->uid = $this->getValue($values, 'uid');
   -    $account->name = $this->getValue($values);
   -    if (!empty($this->options['link_to_user']) || !empty($this->options['overwrite_anonymous'])) {
   -      if (!empty($this->options['overwrite_anonymous']) && !$account->id()) {
   -        // This is an anonymous user, and we're overriting the text.
   -        return String::checkPlain($this->options['anonymous_text']);
   -      }
   -      elseif (!empty($this->options['link_to_user'])) {
   -        $account->name = $this->getValue($values);
   +    $account = $this->getEntity($values);
   +
   +    if (!empty($this->options['overwrite_anonymous']) && !$account->id()) {
   +      // This is an anonymous user, and we're overwriting the text.
   +      return String::checkPlain($this->options['anonymous_text']);
   +    }
   +
   +    if (!empty($this->options['link_to_user'])) {
   +      if (!empty($this->options['format_username'])) {
            $username = array(
              '#theme' => 'username',
              '#account' => $account,
            );
            return drupal_render($username);
          }
   +      else {
   +        return parent::renderLink($data, $values);
   +      }
        }
   

   looks perfect! Much simpler logic actually at the end of the day.

6. +++ b/core/modules/user/src/Tests/UserTokenReplaceTest.php
   @@ -66,7 +67,8 @@ function testUserTokenReplacement() {
   +    $tests['[current-user:name]'] = String::checkPlain($global_account->getDisplayName());
   

   Same as with just user:

7. +++ b/core/modules/user/src/Tests/Views/HandlerFieldUserNameTest.php
   @@ -25,29 +25,27 @@ class HandlerFieldUserNameTest extends UserTestBase {
      public static $testViews = array('test_views_handler_field_user_name');
   

   quick note: this view does not have a sort handler, at least we should add it on a followup due to given potential random failures.

8. +++ b/core/modules/user/tests/modules/user_test_views/test_views/views.view.test_views_handler_field_user_name.yml
   @@ -42,6 +42,13 @@ display:
            type: fields
   +      sorts:
   +        uid:
   +          id: uid
   +          table: users
   +          field: uid
   +          plugin_id: standard
   +          provider: views
   

   Nice, someone read my mind!

Here we go. Made those changes and discussed on irc - Let's go back to user:name like we had before and user:display-name for the display name?

+++ b/core/modules/user/config/install/user.mail.yml
@@ -1,28 +1,28 @@
-  body: "[user:name],\n\nYour account on [site:name] has been canceled.\n\n--  [site:name] team"
-  subject: 'Account details for [user:name] at [site:name] (canceled)'
+  body: "[user:display-name],\n\nYour account on [site:name] has been canceled.\n\n--  [site:name] team"
+  subject: 'Account details for [user:display-name] at [site:name] (canceled)'

Tokens are fine with me.

Is this something that the migration of those settings should care about? Possibly in a follow-up?

Yes, that's a good point indeed. And yes, I think that should be a follow up. This patch is big enough for this "simple" change :)

It would be great to fill a change notice or rather update an existing one, given how otherwise the mass of change notices would grow.

I am very confused. One source of the confusion is the issue summary suggesting ->label() but the patch adding a new ->getDisplayName() . That, I guess is a simple change to the issue summary.

Then getUsername is return $this->name ?: ''; when just return $this->name should be fine. Isn't that so? The default is '' after all.

But the biggest confusion is the issue itself. So getUsername doesn't return the raw username? In what circumstances? That's not in the IS either. I mean, yeah, anonymous and that alter probably doesn't belong there but my problem is that getDisplayName is not the result of user_format_name() as one would expect but ... what? The name of method is decidedly odd and the distinction between getUsername and getDisplayName is not clear to me -- one wonders whether that's a case "listen to the code, it's trying to tell you something" or "chx is dumb and the doxygen is not helping". Is really the distinction only in anonymous + alter? If so then why are we adding a new method instead of moving those into user_format_name() ? I just don't get what and why happens here.

Tentatively setting to CNR.

I only understand half of what you said @chx, but here's how it is:

Current situation:
- getUsername() == label() == user_format_name(), which is the possibly altered username. Most common use case is realname module.

This is a problem because getUsername() is used as if it *would* be the raw username, for example it is the default value for the username field when editing a user.

So this issue adds a getDisplayName(), which is the same as label() and user_format_name() (which is deprecated, that is why we are adding a new method). getUsername() now returns the actual username, unchanged.

Remind me then why we are not using label() ?

But if we go with getDisplayName, the summaries are not helpful: "Returns the unique username of this account." vs "Returns the display name of this account." I think what you wanted to say is "Returns the username as it is stored" vs "Returns the user name as it should be displayed" but then again the second is not right because it's not how it should be displayed because it needs to be checkPlain'd first :/ see this is a case of "listen to the code, it's trying to tell you something" after all -- why don't we checkPlain the return value of getDisplayName, hrm?

what method exactly? All entities have label() and label on users is user_format_name()/getDisplayName().

label() is however currently not check plain'ed, so we can't check_plain() getDisplayName(). Yes, maybe it would make sense, but that's a completely different discussion and would result in a number of other problems, because sometimes, you need the label unescaped and it is very common to use the label in @something placeholders, so those would have to be updated to use !. So I disagree that we should do this here.

Update the issue summary and the docblocks, sure, if anyone has ideas on how to improve that...

Amend the doxygen of EntityInterface::label() to clearly state

1. It's not escaped
2. It is not necessarily any property of the entity. AFAICS ContentEntityBase::label supports label callback on entities -- I think it's only for users that it is defined, user_format_name apparently.

once that's done, you can use label() freely for the formatted username. Next, why getUserName() ? Why not getName() since it returns $user->name->value ? What else would it be if not the name of the user? Node has getTitle and not getNodeTitle.

Here's the getUsername > getName change. That makes sense. Let's talk about chx's other point some more, as we ideally do not want to be keeping user_format_name. I would like to see us being able to support callbacks on the entity instance. So core could ship with a call to getDisplayName(), other people could then override easily if they wanted to by specifying their own label callback. thoughts?

Actually, sorry, I am going mad.

getUsername() is the current name already. Let's not change that in this issue chx.

Can be a followup, for sure. Also, we can simply override User::label once we documented label as described in #77. We can even remove the "label callback" capability because that's a really weird thing in an OOP world -- just override label(), done.

What' up with the patch in #78?

Re-rolled the patch from #68 (patch in 78 changed getUsername to getName which we're postponing for follow-up).

I honestly don't really remember why we went with getDisplayName instead of just overriding label. If we do decide to use label() instead, can we get rid of all that get/set/hasLabelCallback business from entities in general?

" If we do decide to use label() instead, can we get rid of all that get/set/hasLabelCallback business from entities in general?"

Are we doing that here or in a followup? Would be nice to move this along.

This issue is affected by this: #2434697: Remove UserAutocompleteController

The UserAutocompleteController is now gone. We are instead to use entity_autocomplete.

Patch re-rolled. Attaching the re-rolled patch below.

Missed out the test files. Attaching another patch with test files attached as well.

Re-rolled patch. Uploading the updated patch.

Removed duplicate functions & uploading the fixed patch.

Quick reroll attached fixing patch failure after change in 8.0.x-dev.

Continued patch changes.

Reroll and bunch of fixes.

Return lost changes and fix test failures

Oops, wrong one.

Has to be rerolled after this one

Rerolled

Not sure about the rest of it, but core/modules/user/src/Entities/User.php says this in the entity definition annotation:

 *   label_callback = "user_format_name",

Note that the entity annotation docs don't tell you what to do for label_callback: https://www.drupal.org/node/2207559

This issue is a blocker for #2311219: Fix hook_user_format_name_alter() documentation and stop referring to user_format_name() which means we can't finish deprecating user_* functions until this is decided.

This is a re-roll as getting rid of a change to hook_rebuild as list_themes() is no longer available since #2151469: Clean-up usage of deprecated list_themes() and _system_rebuild_theme_data() in favor of theme_handler service

oh, here's the patch :)

All other lines in this patch seem related to this issue.

Thanks for sticking with it, @nlisgo.

Updated the issue summary, pretty sure this will need a change record.

Hopefully some of the folks from early in the comment thread can weigh in as well.

Didn't verify that all places have been updated but the patch looks good to me.

I've been using an old version of the patch here for almost a year now, some places in Drupal are really broken in HEAD when you use hook_user_name_alter(), for example the user edit form that displays the altered username instead of the raw one as default value.

Yes, this needs needs a change record, or maybe some change record updates (if that, then that should be outlined in the issue summary) and a beta evaluation, explaining why the disruption (and is quite a disruption, I've had conflicts on my old patch on almost every update I did) is worth it and needed. Those things need to be done before it can be RTBC'd, so setting back to needs work for that.

This is a bit of a blocker for #2311219: Fix hook_user_format_name_alter() documentation and stop referring to user_format_name(), which is removing a @deprecated function.

It's possible that we could deprecate user_format_name() in some other way, and if that's the case then maybe we could work on that instead of this disruptive way.

Needs work here: #2450793: Properly deprecate support for entity type label callbacks

Almost a blocker.

Added beta evaluation. Instead of new CR, can we update: #2017231?

Edit: never mind.

Re-roll of #122.

Removing 'needs reroll' tag.

I started working on this issue while I was at work and was unable to continue it. Looking into it now.

+++ b/core/modules/file/src/Tests/FileFieldPathTest.php
@@ -46,7 +46,7 @@ function testUploadPath() {
-    $this->updateFileField($field_name, $type_name, array('file_directory' => '[current-user:uid]/[current-user:name]'));
+    $this->updateFileField($field_name, $type_name, array('file_directory' => '[current-user:uid]/[current-user:display-name]'));

that's looks wrong because defaults for user uploads are not affected in patch otherwise you need to provide upgrade path and change migrations, so please remove this hunk

Here's a re-roll that reverts the hunk in FileFieldPathTest.php as suggested in #141

Re-rolling again, previous patch didnt apply

Changing status

I missed some git conficts; had to clean up some tests that had changed in HEAD since original patch

Fixed the last failing test.

Seems like the test is not looking at the right result. It should look at the new user, not anonymous.

Note: In a previous patch we used the hardcoded 3rd result. Somewhere along the line, the 3 changed to 0. My patch picks the result from the userID which is hopefully less fragile.

Reroll, fixed some tests.

I changed a couple of instances where we already had a changed hunk but still used the old style array syntax to use the new array syntax, I also changed one comment that was wider than 80 columns.

Another reroll.

I did a quick review and I have two remarks:

1. +++ b/core/modules/user/src/Tests/UserTokenReplaceTest.php
   @@ -65,12 +66,15 @@ function testUserTokenReplacement() {
   +    $tests['[current-user:name]'] = Html::escape($global_account->getDisplayName());
   

   This line does not look good to me.

+++ b/core/modules/user/user.tokens.inc
@@ -30,7 +30,11 @@ function user_token_info() {
+  $user['username'] = array(

I thought we were using name and display-name and not name and username.

@nlisgo: Thanks for the typo fix.

diff --git a/core/core.api.php b/core/core.api.php
index 85fc654..5f9d22a 100755

I guess its an unrelated change.

Items suggested in #161 are covered.

Seems like a random failure. Tests passed in my local.

Added beta evauation as per #129. Are we going to edit 2017231 for change record?

Updated the change record #2017231: Added AccountInterface as common interface for the global $user object and user entities.

The patch looks good to me.

1. +++ b/core/lib/Drupal/Core/Session/AccountInterface.php
   @@ -124,6 +124,17 @@ public function getPreferredAdminLangcode($fallback_to_default = TRUE);
   +   *   \Drupal\Component\Utility\String::checkPlain() is called on it before it
   

   This needs updating to Html::escape() and is not quite right with Twig autoescape.

2. +++ b/core/lib/Drupal/Core/Session/UserSession.php
   @@ -160,6 +160,13 @@ function getPreferredAdminLangcode($fallback_to_default = TRUE) {
   +    return $this->name ?: '';
   

   When is this the case?

+++ b/core/modules/action/src/Plugin/Action/EmailAction.php
@@ -185,7 +185,7 @@ public function buildConfigurationForm(array $form, FormStateInterface $form_sta
+      '#description' => t('The message that should be sent. You may include placeholders like [node:title], [user:display-name], [user:name] and [comment:body] to represent data that will be different each time message is sent. Not all placeholders will be available in all contexts.'),

Having both name and display name here is confusing.

2. Yeah, looks like that doesn't make sense, we already initialize the property.

Confusing how? That's *exactly* where we need those two different tokens, see the updated default mails. You want to use the display name when addressing the user, but when telling him how to log in, you want to use the actual username.

I understand that this is quite an API change but it's also a serious bug when altering the username, since it will use the display name in the user edit form, just to name one example. I've been using a patch from this issue for more than I year but never found time to push it through. We can try to find a way to make the change smaller if needed, but in a way, the change is good since it forces you to think about which one you want to use. A possible alternative would be to keep user:name as it is and introduce a new token for the actual user name and stop using getUsername() where you want to actual username (yes, sounds weird).

Addressing points 1 and 2 in #170.

Ignore the bad patch in #172.

@berdir #171

I think it would be less confusing if we had getLoginName() or getAccountName() vs. getDisplayName(). We could possbly deprecate name() and add the extra method?

#178++

Patch using getAccountName().