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.
Hey,
The 'create new account' and 'request new password' links are added as markup in the login block form.
Better would be to add this as #theme => item_list, so other modules can add their links if needed.
Example is the OpenID module, which now just adds a second item_list to the form.
Will post a patch in a minute.
Greetings,
Tom
Comment | File | Size | Author |
---|---|---|---|
#5 | userlinks-itemlist-1324972-5.patch | 695 bytes | betz |
#1 | userlinks-itemlist-1324972-1.patch | 675 bytes | betz |
Comments
Comment #1
betz CreditAttribution: betz commentedthe patch
Comment #2
betz CreditAttribution: betz commentedComment #3
betz CreditAttribution: betz commentedComment #4
xjmThanks for the patch. I can see the case for changing this, although I'm not immediately sure what implications it might have for existing themes.
Note that the Drupal 8.x patch will need to be rerolled, because the core directory structure for Drupal 8 has now changed. (For more information, see #22336: Move all core Drupal files under a /core folder to improve usability and upgrades). When the patch has been rerolled, please set the issue back to "Needs Review."
Tagging as novice for the task of rerolling the Drupal 8.x patch.
If you need help rerolling this patch, you can come to core office hours or ask in #drupal-gitsupport on IRC.
Comment #5
betz CreditAttribution: betz commentedThere you go
Comment #6
betz CreditAttribution: betz commentedComment #7
betz CreditAttribution: betz commentedAbout the implications for other themes, the markup stays exactly the same.
Except now themes *can* alter the list if they need to.
Comment #8
xjmExcellent, thanks!
Comment #9
betz CreditAttribution: betz commentedBTW this is a blocker for #1217016: Clean up the CSS for Open ID module
Comment #10
betz CreditAttribution: betz commentedNow what?
Comment #11
Devin Carlson CreditAttribution: Devin Carlson commentedThe patch applies cleanly to Drupal 8 and results in the same markup being generated while providing more flexibility.
I also manually made the same changes in Drupal 7 and did not find any problem, so I'm tagging this as "needs backport to Drupal 7" (it provides an improvement and doesn't require any API changes).
Comment #12
betz CreditAttribution: betz commentedYes, that's right, would work for 7 also, but then some modules will need to be altered also.
Openid for example, where the jquery wouldn't apply anymore.
I will create a patch for openid, for d8 at least.
Then we have to question if it's wise to update for D7 also, as maybe other contrib modules may be broke also.
Comment #13
betz CreditAttribution: betz commentedSorry was wrong, openid doesn't break as openid just adds a new item list next to it.
Comment #14
catchLooks like a good change, and I see the OpenID patch that depends on it (which also look sensible).
Not sure about backport to D7 since potentially this could conflict with a form alter or something, although that seems somewhat unlikely so tentatively moving it back for backport at least for now.
Comment #15
Devin Carlson CreditAttribution: Devin Carlson commentedThe patch in #1 still applies cleanly to D7 (I tested with 7.10).
In my mind, anyone who would have wanted to make a change to the user block links would have had to work around this issue since the release of Drupal 7 and would have done so in a similar fashion to openid (by adding another unordered list beneath the list of user block links) so I can't see any obvious ways in which existing code would break.
This might also mildly improve accessibility (if multiple unordered lists of "user links" are displayed consecutively, a user with a screen reader might wonder what the difference was between each list of "user links", not to mention the extra time the screen reader might take to mention the beginning and ending of each list).
Of course, if committing the patch in #1 might break any existing code, this issue can just be closed.
Comment #16
xjmJust to clarify, though it's not stated explicitly in #14, this has been committed to 8.x:
http://drupalcode.org/project/drupal.git/commit/c9e01c6
Comment #17
webchickHm. I could see this conflicting with hook_page_alter() or similar, so my inclination is not to commit this to 7.x. If you strongly disagree, we can discuss though.