Hello,

There are plenty of issues around the forum:
http://drupal.org/node/229714
http://drupal.org/node/904604
http://drupal.org/node/100450

The user/register, user/password, user/login all show the same title "User Account"

CommentFileSizeAuthor
#73 user-login-page-title-d7-1251188-73.patch1 KBAnonymous (not verified)
#63 user-1251188-63.patch3.77 KBxjm
#63 interdiff-63.txt2.24 KBxjm
#54 user-fix_page_title-1251188-54-test+fix.patch3.76 KBCameron Tod
#54 interdiff.txt717 bytesCameron Tod
#53 user-fix_page_title-1251188-53-test+fix.patch3.81 KBCameron Tod
#53 interdiff.txt1.15 KBCameron Tod
#48 user-fix_page_title-1251188-48-test.patch2.69 KBCameron Tod
#48 user-fix_page_title-1251188-48-test+fix.patch3.71 KBCameron Tod
#48 interdiff.txt967 bytesCameron Tod
#44 user-fix_page_title-1251188-44.patch1.02 KBCameron Tod
#44 interdiff.txt478 bytesCameron Tod
#41 user-fix_page_title-1251188-41.patch1.13 KBCameron Tod
#40 699604-forms-api-reference-link-d6-backport-43.patch617 bytesCameron Tod
#38 drupal-user_page_title_2-1251188-38.patch1001 bytesruloweb
#34 drupal-1251188-34-user_page_title.patch1.05 KBfenstrat
#28 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch736 bytesSagar Ramgade
#24 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch796 bytesSagar Ramgade
#22 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch915 bytesSagar Ramgade
#20 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch907 bytesSagar Ramgade
#18 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch1.61 KBSagar Ramgade
#14 drupal-page_title_fix_for_user_register_login_password-1251188-14.patch1.42 KBSagar Ramgade
#12 page_title_for_user_login_password_register-1251188-11.patch1.63 KBSagar Ramgade
#9 page_title_for_user_login_password_register-1251188-9.patch908 bytessag_13684
#6 page_title_for_user_register_pass_login_pages-1251188-4.patch1.33 KBMauHG
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

giorgio79’s picture

Let me add that the current setup is also a usability issue as it does not help if a user searches for "sitename + lost password" or "sitename register" via a search engine, so would be great to fix it. :)

BenK’s picture

Subscribing

clemens.tolboom’s picture

-enzo-’s picture

I want to fix this issue

My proposal for new titles are:

/user -> User Log in
/user/login -> User Login
/user/register -> User New Account
/user/password -> User new password

If some has better titles , let me know.

clemens.tolboom’s picture

@-enzo- do you mean fix with code then follow below. And check the forum discussions for other hints :)

[Powered by #1115636: Issue Macros and Templates]

Please update the issue summary by applying the template from http://drupal.org/node/1155816.

Please update the issue summary by editing the issue. This helps people to understand this issue quicker.

MauHG’s picture

Status: Active » Needs review
Issue tags: +dlatino, +drupalcr, +#drupalcr
FileSize
1.33 KB

Hi people!
As part of the first Drupal 8 Contribute Costa Rica, we made this patch, this put the title propossed in the comment #4 by -enzo-.
Work.
people involved:
Katia Zamora
MauHG
-dude-

Pura vida!

Status: Needs review » Needs work

The last submitted patch, page_title_for_user_register_pass_login_pages-1251188-4.patch, failed testing.

Jelle_S’s picture

I wouldn't do this using drupal_set_title. I think it would be better to change the user module's hook_menu

sag_13684’s picture

Hi,

Here is the patch which uses hook_menu to correct this .

Jelle_S’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, page_title_for_user_login_password_register-1251188-9.patch, failed testing.

Sagar Ramgade’s picture

Hi,

Here's the another patch.

clemens.tolboom’s picture

@sag_13684: Shouldn't you use http://drupal.org/user/399718? You can better disable/remove this sag_13684 user account I guess ;-)

After fixing the whitespace please set the status to needs review.

+++ b/core/modules/user/user.module
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+function user_menu_title() {  ¶

Whitespace

+++ b/core/modules/user/user.module
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+      ¶

Whitespace (3 times)

Sagar Ramgade’s picture

Hi,

Yes i will disable that account i thought of playing around with git to learn so used that account, here is the correctd patch file. Hope this works.

Sagar Ramgade’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Jelle_S’s picture

+++ b/core/modules/user/user.pages.incundefined
@@ -508,6 +508,10 @@ function user_page() {
+    $breadcrumb[] = l(drupal_get_title(), $base_url . request_uri());    ¶

Undefined variable: $base_url

You need to call global $base_url;

Also, your patch has some trailing spaces which should be removed

Sagar Ramgade’s picture

Status: Needs work » Needs review
FileSize
1.61 KB

Hi,

My bad i copy pasted that code from a thread, removed the spaces and added global $base_url. Here's the new patch.

Status: Needs review » Needs work
Sagar Ramgade’s picture

Status: Needs work » Needs review
FileSize
907 bytes

Hi,

Here is one more try, Anyone else interested in trying this one ?

Status: Needs review » Needs work
Sagar Ramgade’s picture

Status: Needs work » Needs review
FileSize
915 bytes
Jelle_S’s picture

Status: Needs review » Needs work
+++ b/core/modules/user/user.moduleundefined
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+function user_menu_title() {  ¶
+  if (!user_is_logged_in()) {
+++ b/core/modules/user/user.moduleundefined
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+        break;
+      ¶
+      case 'login' :
+++ b/core/modules/user/user.moduleundefined
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+        break;
+      ¶
+      case 'password' :
+++ b/core/modules/user/user.moduleundefined
@@ -1953,8 +1953,28 @@ function user_uid_optional_to_arg($arg) {
+        break;
+      ¶
+      default :

remove trailing spaces from those lines,
besides that everything seems ok. Test passes, but I haven't tested it manually yet

Sagar Ramgade’s picture

Here's the patch with those corrections ? How do i commit it to drupal core project on drupal.org if everything works fine ?

Sagar Ramgade’s picture

Status: Needs work » Needs review
Jelle_S’s picture

There still are some trailing spaces after the break; statement of login and password.

You can't commit it to drupal. Someone with commit access to drupal must chime in on this issue and when they approve the patch they will commit it to drupal, mentioning your name as contributor. Before that this patch needs to be tested by somewone and then (s)he must set the issue status to "reviewed & tested by the community"

klausi’s picture

Status: Needs review » Needs work

Nice work, this patch is a good idea.

+++ b/core/modules/user/user.module
@@ -1954,7 +1954,24 @@ function user_uid_optional_to_arg($arg) {
+        return t('Create new account');
+        break;

a break statement after a return statement is useless.

+++ b/core/modules/user/user.module
@@ -1954,7 +1954,24 @@ function user_uid_optional_to_arg($arg) {
+        break;      ¶
+      case 'password' :
+        return t('Request new password');
+        break;      ¶

trailing white spaces.

Sagar Ramgade’s picture

Hey,

Thanks kluas for your feedback, i have corrected those, here's the patch.

Sagar Ramgade’s picture

Status: Needs work » Needs review
DevElCuy’s picture

Issue tags: -#drupalcr

Removed wrong tags

fenstrat’s picture

fenstrat’s picture

Status: Needs review » Reviewed & tested by the community

This is a great little fix this. It's got good feedback and the patch looks good. RTBC.

catch’s picture

Status: Reviewed & tested by the community » Needs review

Would be nice not to use arg() here. What about menu_get_item()?

fenstrat’s picture

Good call, here it is with menu_get_item().

I'm getting odd PHP out of memory errors on /user (but not /user/login), seems due to calling menu_get_item()? Will see how the testbot goes.

Status: Needs review » Needs work
Issue tags: -dlatino, -drupalcr

The last submitted patch, drupal-1251188-34-user_page_title.patch, failed testing.

ruloweb’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work
Issue tags: +dlatino, +drupalcr

The last submitted patch, drupal-1251188-34-user_page_title.patch, failed testing.

ruloweb’s picture

Status: Needs work » Needs review
FileSize
1001 bytes

Patch on #34 throws PHP out of memory error when trying to access /user.

The problem seems to be that you can't use menu_get_item in user_menu_title because it gets recursively called (user_menu_title is the "title callback" for "/user").

Patch #28 works fine, so we combined both patches into this new one.

--
BADUG Argentina

fenstrat’s picture

Thanks @ruloweb however the patch in #38 doesn't set the title to "Login" on /user, and (more importantly) it uses arg() which @catch suggested avoiding in #33.

However, as you say, user_menu_title seems to be called recursively, so I'm sure of the best way forward here.

Cameron Tod’s picture

I just came across this bug today at work, so I'd love to see if get committed.

How about caching the result of menu_get_item() with drupal_static()?

Cameron Tod’s picture

Woops wrong patch.

Status: Needs review » Needs work

The last submitted patch, user-fix_page_title-1251188-41.patch, failed testing.

Cameron Tod’s picture

OK, I see the recursion issue now. catch, do you think it is ok to then switch on arg()?

Cameron Tod’s picture

Status: Needs work » Needs review
FileSize
478 bytes
1.02 KB

Switched out menu_get_item() for current_path(). This should work even on aliased paths.

Status: Needs review » Needs work

The last submitted patch, user-fix_page_title-1251188-44.patch, failed testing.

Cameron Tod’s picture

OK, this is the good type of test fail :)

Page title 'Login | Drupal' is equal to 'User account | Drupal'.	Other	BreadcrumbTest.php	382	Drupal\system\Tests\Menu\BreadcrumbTest->testBreadCrumbs()

Need to change the test to test a different set of breadcrumbs I guess.

Cameron Tod’s picture

Ugh, this is pretty tightly coupled up with #1564388: "My account" link is never in the active trail

I will have a look later on and see if I can change the tests to work well for both issues.

Cameron Tod’s picture

Status: Needs work » Needs review
FileSize
967 bytes
3.71 KB
2.69 KB

Here's a new test that checks that the new links are being applied correctly, and a fix to the tests introduced in #1564388: "My account" link is never in the active trail that test the user/* page breadcrumbs.

The "My Account" execution path only sets the title in Menus, never on an actual page view because of the redirection for logged in users in user_page(), so we just test the default menu.

First patch should fail, second patch should pass and will include the content of the patch in #44.

Cameron Tod’s picture

Title: Fix page title for user/register, user/password, user/login pages, currently all the same » Set unique titles for user/register, user/password, and user/login menu items

Status: Needs review » Needs work

The last submitted patch, user-fix_page_title-1251188-48-test+fix.patch, failed testing.

fenstrat’s picture

Nice work @cam8001!

Test failure on #48 is on the /user for registered users test. It appears $link is not being set.

+++ b/core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.php
@@ -94,4 +94,36 @@ class UserAccountLinksTests extends WebTestBase {
+    // Login the admin user to check the page title for registered users is "My Account" in menus.

Needs to wrap at 80 characters.

Cameron Tod’s picture

Gah, it passed locally after much swearing and hair pulling over finding a suitable xpath. I'll fiddle around a bit more and get something going.

Cameron Tod’s picture

Status: Needs work » Needs review
FileSize
1.15 KB
3.81 KB

Fingers crossed. Comment is fixed up as per #51.

Cameron Tod’s picture

Well well. Make sure that you have your IDE set up right to show when your comments are over 80 columns!

fenstrat’s picture

Status: Needs review » Reviewed & tested by the community

Looks good, moving back to RTBC as this addresses @catch's concerns from #33 and adds test to ensure it all works.

catch’s picture

Status: Reviewed & tested by the community » Needs work

OK current_path() is better than arg(), I wonder if the menu_get_item() recursion issue might get fixed by the new router.

Unfortunately this no longer applies though.


error: patch failed: core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php:379
error: core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php: patch does not apply
error: patch failed: core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.php:94
error: core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.php: patch does not apply
error: patch failed: core/modules/user/user.module:1485
error: core/modules/user/user.module: patch does not apply

Cameron Tod’s picture

Are you sure? It seems to apply ok for me:

cam8001 @ ~/Sites/drupal (user-fix_page_title-1251188) 
██ 11:19:27 $ git fetch

██ cam8001 @ ~/Sites/drupal (user-fix_page_title-1251188) 
██ 11:19:31 $ git rebase origin/8.x
Current branch user-fix_page_title-1251188 is up to date.

██ cam8001 @ ~/Sites/drupal (user-fix_page_title-1251188) 
██ 11:19:36 $ git log --pretty=oneline --abbrev-commit -n1
8643c01 Issue #1338966 by loganfsmyth, geerlingguy: Fixed Introduce _rdf_mapping_load_multiple() to reduce queries.

██ cam8001 @ ~/Sites/drupal (user-fix_page_title-1251188) 
██ 11:19:39 $ git apply -v --reject user-fix_page_title-1251188-54-test+fix.patch
Checking patch core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php...
Checking patch core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.php...
Checking patch core/modules/user/user.module...
Applied patch core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.php cleanly.
Applied patch core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.php cleanly.
Applied patch core/modules/user/user.module cleanly.
catch’s picture

Status: Needs work » Needs review
Cameron Tod’s picture

Looks like the test passed, hooray :)

fenstrat’s picture

Status: Needs review » Reviewed & tested by the community

Patch in #54 still applies cleanly for me.

xjm’s picture

Assigned: Unassigned » yched
Status: Reviewed & tested by the community » Needs work
Issue tags: +String freeze, +UI Change
+++ b/core/modules/system/lib/Drupal/system/Tests/Menu/BreadcrumbTest.phpundefined
@@ -379,7 +379,7 @@ class BreadcrumbTest extends MenuTestBase {
-    $this->assertBreadcrumb('user', $trail, t('User account'));
+    $this->assertBreadcrumb('user', $trail, t('Login'));

+++ b/core/modules/user/lib/Drupal/user/Tests/UserAccountLinksTests.phpundefined
@@ -94,4 +94,37 @@ class UserAccountLinksTests extends WebTestBase {
+    $this->assertTitle('Login' . $title_suffix, "Page title of /user is 'Login'");
...
+    $this->assertTitle('Login' . $title_suffix, "Page title of /user/login is 'Login'");

+++ b/core/modules/user/user.moduleundefined
@@ -1485,11 +1485,26 @@ function user_uid_optional_to_arg($arg) {
+      case 'user/login' :
+        return t('Login');

Okay, I hate to be a pain in the ass, but since this is a user-facing string... "Login" is a noun. "Log in" is a verb. Since we're giving the user an invitation to log in (similar to Create new account), it should be two words.

I'll reroll to change this. :)

xjm’s picture

Assigned: yched » xjm

Oops. x right next to y.

xjm’s picture

Assigned: xjm » Unassigned
Status: Needs work » Needs review
FileSize
2.24 KB
3.77 KB

Ta-da!

Also, since I didn't say it before and just complained: Nice patch. :)

xjm’s picture

Oh, and for what it's worth, I confirmed that it is properly 'Log in' elsewhere in core. :)

fenstrat’s picture

Status: Needs review » Reviewed & tested by the community

Good catch @xjm, I can also confirm "Log in" is the correct form. Tested locally, all good. Back to RTBC.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Berdir’s picture

FYI: The tests caused a random test failure because of the use of randomString() in an URL, which caused funky issues when the first character was a . (then .htaccess denied access).

Fixed in #1833928: Random test failure in Drupal\user\Tests\UserAccountLinksTests.

adamdicarlo’s picture

Okay, I hate to be a pain in the ass, but since this is a user-facing string... "Login" is a noun. "Log in" is a verb. Since we're giving the user an invitation to log in (similar to Create new account), it should be two words.

I'll reroll to change this. :)

One thing I've always loved about Drupal is that it is one of the few projects that gets "Log in" vs. "Login" correct. So thank you for being a "pain in the ass," xjm -- you rock!

kristiaanvandeneynde’s picture

Issue summary: View changes

Not re-opening (just yet), but any chance we can backport this to D7?

ravermeister’s picture

I would like to see that fixed in drupal7 too.

currently I did this solution:
https://drupal.org/node/100450#comment-6508204

and used the this code part:
https://drupal.org/node/100450#comment-6854258

is it a good solution or is there a better "workaround"

anyway would be great to have that generally fixed in D7.

thanks in advance
jonny

kopeboy’s picture

I'm up as well for a solution on Drupal 7.

My h1 at those pages is not even "User Account" but "Home".
Don't know if it's the AT theme setting that.. but I have no way to override those titles.

Anonymous’s picture

This patch can be applied toward Drupal 7 installations.