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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

betz’s picture

the patch

betz’s picture

Status: Active » Needs review
betz’s picture

Title: Change links to item_list instead of markup. » Change user block links to item_list instead of markup.
xjm’s picture

Status: Needs review » Needs work
Issue tags: +Novice

Thanks 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.

betz’s picture

There you go

betz’s picture

Status: Needs work » Needs review
betz’s picture

About the implications for other themes, the markup stays exactly the same.
Except now themes *can* alter the list if they need to.

xjm’s picture

Excellent, thanks!

betz’s picture

betz’s picture

Now what?

Devin Carlson’s picture

Status: Needs review » Reviewed & tested by the community

The 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).

betz’s picture

Yes, 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.

betz’s picture

Sorry was wrong, openid doesn't break as openid just adds a new item list next to it.

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)
Issue tags: +Needs backport to D7

Looks 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.

Devin Carlson’s picture

Status: Patch (to be ported) » Reviewed & tested by the community

The 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.

xjm’s picture

Just 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

webchick’s picture

Version: 7.x-dev » 8.x-dev
Status: Reviewed & tested by the community » Fixed
Issue tags: -Needs backport to D7

Hm. 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.

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