Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
user.module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
28 Oct 2011 at 14:02 UTC
Updated:
29 Jul 2014 at 20:05 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
betz commentedthe patch
Comment #2
betz commentedComment #3
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 commentedThere you go
Comment #6
betz commentedComment #7
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 commentedBTW this is a blocker for #1217016: Clean up the CSS for Open ID module
Comment #10
betz commentedNow what?
Comment #11
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 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 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 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.