The menu item at the path user/%user_uid_optional used to be titled 'My account' if it linked to your own profile, but a recent change now renders it as simply USERNAME by the page title callback, user_page_title(). The user edit page also has the title USERNAME (should be "My account -> "Edit").

I propose we change it back to 'My account'; it would seem to make more sense to the average user. Also, since it was changed in an unrelated issue (see #564632: Cruft in $page due to node_build, comment_build, etc.) I think it at least deserves a discussion.

CommentFileSizeAuthor
#14 user-my-account.3.patch2.43 KBcarlos8f
#4 view-tab.png16.06 KBcarlos8f
#4 edit-tab.png20.02 KBcarlos8f
#4 user-my-account2.patch2.67 KBcarlos8f
#2 user-my-account.patch1.09 KBcarlos8f
my-account.png22.34 KBcarlos8f

Comments

carlos8f’s picture

carlos8f commented
Component: base system » user.module
carlos8f’s picture

carlos8f commented
Status: Active » Needs review
StatusFileSize
new user-my-account.patch1.09 KB

Curious how many failures this will cause. This patch also fixes the menu item for user/uid to not point to user.pages.inc anymore, because user_view() was replaced by user_build() in user.module.

Status: Needs review » Needs work

The last submitted patch failed testing.

carlos8f’s picture

carlos8f commented
Title: Fix user page titles » Bring back the 'My account' link title
StatusFileSize
new user-my-account2.patch2.67 KB
new edit-tab.png20.02 KB
new view-tab.png16.06 KB

I think this is a better direction: Here I'm re-introducing the user_view() function in user.pages.inc, for the purpose of putting back the drupal_set_title() there to override the page title (from 'My account' to USERNAME). Screenshots attached. Here the Edit tab is titled My account -> Edit, instead of USERNAME -> USERNAME. Also, had to do one little tweak to openid.test.

carlos8f’s picture

carlos8f commented
Title: Bring back the 'My account' link title » Fix user page titles
Status: Needs work » Needs review
sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Title: Bring back the 'My account' link title » Fix user page titles

This patch needs to take #635378: Wrong local task titles for taxonomy term and user account pages into account

carlos8f’s picture

carlos8f commented

Thanks sun. The only local task in question here is user/%user/edit, in which I removed the title callback anyway. I applied both patches and there's no conflict as far as I can tell.

brianV’s picture

brianV commented 
+++ modules/user/user.module	8 Dec 2009 21:19:35 -0000
@@ -1676,9 +1674,12 @@
+  if ($account->uid == $GLOBALS['user']->uid) {

Instead of accessing the $GLOBALS array directly, we should stick with the regular core convention of using the global keyword:

global $user;
if ($account->uid == $user->uid) {

Also, there is an extra line at the end of the patch.

I'm on crack. Are you, too?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Also, there is an extra line at the end of the patch.

I don't see one. You probably rather found a bug in Dreditor ;)

brianV’s picture

brianV commented

@sun: You are right - looking directly at the patch file, there is no extra line at the end.

Who wrote this dreditor hunk of junk anyways? :p Look for an issue in your queue.

@carlos: Ignore my comment about the extra line.

carlos8f’s picture

carlos8f commented

@brianV: not using $GLOBALS directly is a core convention? :) I count 40+ cases in HEAD that do this. This would require its own issue if we're going to make it a convention.

carlos8f’s picture

carlos8f commented
Status: Needs review » Postponed

Postponing this until there is a consensus on #635378: Wrong local task titles for taxonomy term and user account pages. The patch in #2 overlaps with mine.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

This bug has a Drupal 6 counterpart at #480484: User page title always set to username which is now marked postponed on this one to avoid introducing a regression. Let's work down this postpone chain!

carlos8f’s picture

carlos8f commented
Title: Fix user page titles » Restore 'My account' link
Status: Postponed » Needs review
StatusFileSize
new user-my-account.3.patch2.43 KB

#635378: Wrong local task titles for taxonomy term and user account pages fixed the Edit local task, so I'm removing that from the patch. To summarize, the current patch:

  • Fixes some regression from #564632: Cruft in $page due to node_build, comment_build, etc., which prevents the menu link to your own user page from displaying as 'My account'.
  • Re-introduces user_view() in user.pages.inc, so drupal_set_title() can set the page title to USERNAME, even if it's your own profile. This is for page title consistency on user/uid pages.
  • While it fixes menu link regression, it also introduces a new page title for user/my_uid/local-task, 'My account', to be visible when you're editing your own account, or other tasks related to your account.
joetsuihk’s picture

joetsuihk commented

work as described in #14 point 2,3 (i do not know what pt1 is talking about..)

but in patch, why introduce a new user_view()? just add the line to user_build() should be ok i think.

Also, $GLOBALS? not global $user anymore?

carlos8f’s picture

carlos8f commented

Point 1 in #14 means that what used to be the 'My account' link (in Garland, it's in the upper right) shows up as 'USERNAME' instead in HEAD, because of a regression from the issue I mentioned.

The reason why I'm not doing drupal_set_title() in user_build() is that *_build() functions are intended to just return a renderable array, which is generic and not limited to one specific menu item. drupal_set_title() should only be done in a proper menu callback, which I brought back for this purpose.

Using $GLOBALS wasn't my code, I was merely reverting user_page_title() to what it used to be. HEAD uses $GLOBALS in situations where a function only has to access it once or twice. Seems fine to me.

Status: Needs review » Needs work

The last submitted patch failed testing.

carlos8f’s picture

carlos8f commented
Status: Needs work » Postponed

Note that we're dealing with the 'My account' link in another patch also, #638070: Router loaders causing a lot of database hits for access checks. I'm postponing this one since I've incorporated a better fix in that issue, which doesn't involve re-introducing user_view().

carlos8f’s picture

carlos8f commented

I created #670296: Allow menu callbacks to implement logic specific to menu links, local tasks, etc. which should solve this issue without using workarounds.

carlos8f’s picture

carlos8f commented

Unfortunately that issue got postponed to D8. Instead of doing drupal_set_title() workarounds, I'd like to pursue #408142: Create a 'user links' menu + page template variable to fix the My account link since it's not supposed to be in the secondary menu anyway.

carlos8f’s picture

carlos8f commented

Also, for performance reasons the 'My account' link shouldn't be a menu link. See http://drupal.org/node/638070, benchmarks indicate an unnecessary user_load() for this link is causing 6-8% of authenticated page load time.

David_Rothstein’s picture

David_Rothstein commented
Title: Restore 'My account' link » Regression: 'My account' link can no longer be displayed
Category: task » bug
Status: Postponed » Needs work

Reopening (based on discussion that happened at #410646: "Secondary menu" exists but is no longer the default source for the secondary links).

The change to displaying the username was intentional, as far as I know, and perfectly good for some uses cases. However, the previous D6 behavior (where you could display the user menu as a block e.g. in the sidebar and have the menu title be your username and the link to your profile say "My account") is also a perfectly legitimate way to display user links, and we seem to have lost that in D7?

Ideally, we find a way to enable both.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

#925778: User edit title is broken (so is beta1-beta2 upgrade path) contains a proper fix for the "My account" link, and a detailed summary of what is wrong in HEAD. I suspect that this issue can be marked as duplicate, though aforementioned issue does not really deal with the link title (though that could be merged into the patch).

carlos8f’s picture

carlos8f commented
Status: Needs work » Closed (duplicate)

Marking duplicate on #925778: User edit title is broken (so is beta1-beta2 upgrade path), let's deal with it there.