Contrib.social

Use getDisplayName() for user names consistently

Open on Drupal.org →
Created on 5 December 2015, almost 9 years ago
Updated 16 February 2023, over 1 year ago

Problem/Motivation

  • Realname module alters the display name, but at user/1234 the username is shown. This is incorrect.
  • Tons of places in core that show the username, but need to show the users display name.
  • Comments do not show DisplayName for registered users.
  • Security problem as usernames should never shown on websites. With real names you cannot login and try passwords, with a username you can.

In Drupal 8.0.0 we deprecated \Drupal\Core\Session\AccountInterface::getUsername(). This is because it was being used for both the display name and getting the username.

The deprecation message is:

   *   Use \Drupal\Core\Session\AccountInterface::getAccountName() or
   *   \Drupal\user\UserInterface::getDisplayName() instead.

The docs for getAccountName() describe the difference:

  /**
   * Returns the unaltered login name of this account.
   *
   * @return string
   *   An unsanitized plain-text string with the name of this account that is
   *   used to log in. Only display this name to admins and to the user who owns
   *   this account, and only in the context of the name used to login. For
   *   any other display purposes, use
   *   \Drupal\Core\Session\AccountInterface::getDisplayName() instead.
   */
  public function getAccountName();

We need to carefully go through the usages and change to getAccountName() or getDisplayName() as required. This is not that simple. For example, another issue was created to covert the following code in UserController to getDisplayName().

          $this->messenger()
            ->addWarning($this->t('Another user (%other_user) is already logged into the site on this computer, but you tried to use a one-time link for user %resetting_user. Please <a href=":logout">log out</a> and try using the link again.',
              [
                '%other_user' => $account->getUsername(),
                '%resetting_user' => $reset_link_user->getUsername(),
                ':logout' => $this->url('user.logout'),
              ]));

However as this is dealing with the logged in user and a password reset link that works using the account name maybe these should be changed to getAccountName().

Proposed resolution

Blocked by #2787871: Properly deprecate getUserName() and use getAccountName() instead →

  • Fix all places where getAccountName() is used, but getDisplayName() should be used.
  • Change core to enable a DisplayName by default. That is why tons of tests are failing because of wrong usage.
  • Where changing to getDisplayName() add test coverage\
  • Ensure getDisplayName() and getAccountName() have good documentation so contrib and custom pick the correct function.

Followup tasks

Allow for configurable truncation length of display name: ✨ Allow custom truncate username settings Needs work

User interface changes

Users display name will be properly shown where appropriate.

API changes

API addition: Entity::getDisplayNameTruncated to get the correct abreviation used in theme_preprocess_username() function.

Data model changes

none

🐛 Bug report
Status

Needs work

Version

10.1 ✨

Component
User module  →

Last updated 1 day ago

Created by

mbkaba22

Live updates comments and jobs are added and updated live.
  • Contributed project blocker

    It denotes an issue that prevents porting of a contributed project to the stable version of Drupal due to missing APIs, regressions, and so on.

  • Triaged core major

    There is consensus among core maintainers that this is a major issue. Only core committers should add this tag.

  • Security improvements

    It makes Drupal less vulnerable to abuse or misuse. Note, this is the preferred tag, though the Security tag has a large body of issues tagged to it. Do NOT publicly disclose security vulnerabilities; contact the security team instead. Anyone (whether security team or not) can apply this tag to security improvements that do not directly present a vulnerability e.g. hardening an API to add filtering to reduce a common mistake in contributed modules.

  • Needs subsystem maintainer review

    It is used to alert the maintainer(s) of a particular core subsystem that an issue significantly impacts their subsystem, and their signoff is needed (see the governance policy draft for more information). Also, if you use this tag, make sure the issue component is set to the correct subsystem. If an issue significantly impacts more than one subsystem, use needs framework manager review instead.

  • Needs issue summary update

    Issue summaries save everyone time if they are kept up-to-date. See Update issue summary task instructions.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 1 year ago →
    🇭🇺Hungary mxr576 Hungary

    The "raw" username is also exposed in \Drupal\node\NodeForm, so there is definitely more work to be done here.

  • Comment over 1 year ago →
    🇨🇭Switzerland berdir Switzerland

    That's true, and probably makes sense to convert.

    > so there is definitely more work to be done here.

    We'll need to figure out how to set the scope here, if we aim to replace everything then this issue might never be good enough. Maybe make it a meta so we can commit the improvements we have? We still need test coverage, though. But we have existing tests for display name, so we need to visit those UI's in that/those test(s).

  • Comment over 1 year ago →
    🇭🇺Hungary mxr576 Hungary

    Maybe make it a meta so we can commit the improvements we have?

    Maybe this should be converted to a meta...? Because I can already see that several issues are references to this. Like this is an existing "subtask" for "The "raw" username is also exposed in \Drupal\node\NodeForm" in #3183509

  • Comment over 1 year ago →
    🇬🇧United Kingdom AaronMcHale Edinburgh, Scotland

    Agree, smaller well scoped issues will be much easier to get done, and so converting this to a meta makes a lot of sense. In my experience if one change starts touching multiple different sub-systems that's a good indication that splitting it up will make it easier to scope and get done.

    For example, with the recent generic Revision UI, we did the "core" implementation in one issue, then have separate issues for each core module. A similar approach here would probably work well.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024