user_login_block (user.module, line 1284) defines $form['links'] as list of links, stored as markup.

When someone needs to alter those links (i.e. http://drupal.org/node/1784544), they need to recreate that links from scratch.
When this happens, if there are multiple modules that alter those links, functionallity from one of them overlaps the other, and only the functionallity (the links) of the last one remains.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

grisendo’s picture

Assigned: grisendo » Unassigned
Status: Active » Needs review
FileSize
448 bytes

I attach the patch I made.

Original links are stored into #original_links, so other modules can pick/store, their our links and re-theme existing ones.

Anonymous’s picture

Version: 7.16 » 8.x-dev
Status: Needs review » Needs work
Issue tags: +Needs backport to D7

Does this issue exist in D8?

grisendo’s picture

Version: 8.x-dev » 7.16
Status: Needs work » Needs review

No, since theming in Drupal 8 is delayed:

Drupal 7:
$form['links'] = array('#markup' => theme('item_list', array('items' => $items)));

Drupal 8:
$form['links'] = array(
'#theme' => 'item_list',
'#items' => $items,
'#weight' => 10,
);

grisendo’s picture

I think it's a better approach to delay theming as Drupal 8 does, instead of keeping another sub-array with original items. This avoids to get, add and call theme_links on every form_alter which changes it. You just need to add your links to the array.

I attach the new patch.

grisendo’s picture

Now what? Patch will be automatically sent to core? Or do I need to do something more?

There will be any collision between two attached patches or it will take the last one in this thread? If there will be collisions, how do I fix it?

Thanks!

Anonymous’s picture

The testbot uses the last file sent or rather actually knows the comment the file relates to. The version though is issue related and the core version at the time the request is sent to the testbot is what is used to test with.

grisendo’s picture

And now, do I need to commit the patch to git? Or testbot automatically does it?

Anonymous’s picture

No the maintainers of the version do that.

h3rj4n’s picture

Version: 7.16 » 7.x-dev
Component: forms system » user system
Status: Needs review » Reviewed & tested by the community

Patch looks good. Works for me!

David_Rothstein’s picture

Status: Reviewed & tested by the community » Needs review

Since this is a data structure change in one of the more heavily-altered forms in Drupal, I think this needs more reviews and discussion about whether it's feasible for Drupal 7 (see https://drupal.org/node/1527558).

A less disruptive option might be to store a copy of the original links in another key on the form array so they could be accessed by anyone altering the form (who can then copy them for the purpose of overwriting the #markup). It's not ideal, but I think it would help.

mgifford’s picture

Issue summary: View changes

How do we generate that discussion? It's been almost a year and since the last comment and 2 years since the patch was implemented.

We need some way to generate that discussion. We've probably got another 4 years of Drupal 7, we need to be able to get a discussion going around these issues. Particularly when there are working patches for D7 and it's already implemented in D8.