Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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"
Comment | File | Size | Author |
---|---|---|---|
#73 | user-login-page-title-d7-1251188-73.patch | 1 KB | Anonymous (not verified) |
#63 | user-1251188-63.patch | 3.77 KB | xjm |
#63 | interdiff-63.txt | 2.24 KB | xjm |
#54 | user-fix_page_title-1251188-54-test+fix.patch | 3.76 KB | Cameron Tod |
#54 | interdiff.txt | 717 bytes | Cameron Tod |
Comments
Comment #1
giorgio79 CreditAttribution: giorgio79 commentedLet 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. :)
Comment #2
BenK CreditAttribution: BenK commentedSubscribing
Comment #3
clemens.tolboom[ Powered by #1115636: Issue Macros and Templates - _default ]
This relates to #1337546: Cannot reorder 'User Menu' items for user/register user/login as these are not accessible by admin
Comment #4
-enzo- CreditAttribution: -enzo- commentedI 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.
Comment #5
clemens.tolboom@-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.
Comment #6
MauHG CreditAttribution: MauHG commentedHi 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!
Comment #8
Jelle_SI wouldn't do this using drupal_set_title. I think it would be better to change the user module's hook_menu
Comment #9
sag_13684 CreditAttribution: sag_13684 commentedHi,
Here is the patch which uses hook_menu to correct this .
Comment #10
Jelle_SComment #12
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHi,
Here's the another patch.
Comment #13
clemens.tolboom@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.
Whitespace
Whitespace (3 times)
Comment #14
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHi,
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.
Comment #15
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedComment #17
Jelle_SUndefined variable: $base_url
You need to call global $base_url;
Also, your patch has some trailing spaces which should be removed
Comment #18
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHi,
My bad i copy pasted that code from a thread, removed the spaces and added global $base_url. Here's the new patch.
Comment #20
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHi,
Here is one more try, Anyone else interested in trying this one ?
Comment #22
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedComment #23
Jelle_Sremove trailing spaces from those lines,
besides that everything seems ok. Test passes, but I haven't tested it manually yet
Comment #24
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHere's the patch with those corrections ? How do i commit it to drupal core project on drupal.org if everything works fine ?
Comment #25
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedComment #26
Jelle_SThere 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"
Comment #27
klausiNice work, this patch is a good idea.
a break statement after a return statement is useless.
trailing white spaces.
Comment #28
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedHey,
Thanks kluas for your feedback, i have corrected those, here's the patch.
Comment #29
Sagar Ramgade CreditAttribution: Sagar Ramgade commentedComment #30
DevElCuy CreditAttribution: DevElCuy commentedRemoved wrong tags
Comment #31
fenstrat#28: drupal-page_title_fix_for_user_register_login_password-1251188-14.patch queued for re-testing.
Comment #32
fenstratThis is a great little fix this. It's got good feedback and the patch looks good. RTBC.
Comment #33
catchWould be nice not to use arg() here. What about menu_get_item()?
Comment #34
fenstratGood 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.
Comment #36
ruloweb CreditAttribution: ruloweb commented#34: drupal-1251188-34-user_page_title.patch queued for re-testing.
Comment #38
ruloweb CreditAttribution: ruloweb commentedPatch 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
Comment #39
fenstratThanks @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.
Comment #40
Cameron Tod CreditAttribution: Cameron Tod commentedI 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()?
Comment #41
Cameron Tod CreditAttribution: Cameron Tod commentedWoops wrong patch.
Comment #43
Cameron Tod CreditAttribution: Cameron Tod commentedOK, I see the recursion issue now. catch, do you think it is ok to then switch on arg()?
Comment #44
Cameron Tod CreditAttribution: Cameron Tod commentedSwitched out menu_get_item() for current_path(). This should work even on aliased paths.
Comment #46
Cameron Tod CreditAttribution: Cameron Tod commentedOK, this is the good type of test fail :)
Need to change the test to test a different set of breadcrumbs I guess.
Comment #47
Cameron Tod CreditAttribution: Cameron Tod commentedUgh, 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.
Comment #48
Cameron Tod CreditAttribution: Cameron Tod commentedHere'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.
Comment #49
Cameron Tod CreditAttribution: Cameron Tod commentedComment #51
fenstratNice work @cam8001!
Test failure on #48 is on the /user for registered users test. It appears $link is not being set.
Needs to wrap at 80 characters.
Comment #52
Cameron Tod CreditAttribution: Cameron Tod commentedGah, 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.
Comment #53
Cameron Tod CreditAttribution: Cameron Tod commentedFingers crossed. Comment is fixed up as per #51.
Comment #54
Cameron Tod CreditAttribution: Cameron Tod commentedWell well. Make sure that you have your IDE set up right to show when your comments are over 80 columns!
Comment #55
fenstratLooks good, moving back to RTBC as this addresses @catch's concerns from #33 and adds test to ensure it all works.
Comment #56
catchOK 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.
Comment #57
Cameron Tod CreditAttribution: Cameron Tod commentedAre you sure? It seems to apply ok for me:
Comment #58
catch#54: user-fix_page_title-1251188-54-test+fix.patch queued for re-testing.
Comment #59
Cameron Tod CreditAttribution: Cameron Tod commentedLooks like the test passed, hooray :)
Comment #60
fenstratPatch in #54 still applies cleanly for me.
Comment #61
xjmOkay, 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. :)
Comment #62
xjmOops. x right next to y.
Comment #63
xjmTa-da!
Also, since I didn't say it before and just complained: Nice patch. :)
Comment #64
xjmOh, and for what it's worth, I confirmed that it is properly
'Log in'
elsewhere in core. :)Comment #65
fenstratGood catch @xjm, I can also confirm "Log in" is the correct form. Tested locally, all good. Back to RTBC.
Comment #66
catchCommitted/pushed to 8.x, thanks!
Comment #68
BerdirFYI: 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.
Comment #69
adamdicarlo CreditAttribution: adamdicarlo commentedOne 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!
Comment #70
kristiaanvandeneyndeNot re-opening (just yet), but any chance we can backport this to D7?
Comment #71
ravermeister CreditAttribution: ravermeister commentedI 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
Comment #72
kopeboy CreditAttribution: kopeboy commentedI'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.
Comment #73
Anonymous (not verified) CreditAttribution: Anonymous as a volunteer commentedThis patch can be applied toward Drupal 7 installations.