Allow dynamic alteration of menu item

http://drupal.org/project/login_destination

there is currently no elegant way to support what you want via drupal core. the problem needs to be addressed there, so reassigning.

in the meantime, you might think about handliing it at the theming level.

This is really a menu system usability bug. We shouldn't have to hack this at the theme level.

The attached patch would enable this for D6 - allows additional callbacks to set the link options (such as the query string).

or for those who prefer to have more power still...

or use drupal_alter if a flag is set.

oops - we already use drupal_alter('menu_link')

code looks good. tested, and works as advertized.

something closer to the original title - the main nut to crack is the login link. Right now it a link to /user can *never* be added or edited via menu module since the access check is via function user_is_anonymous(), and there is no way to return the user to the page they logged in from.

so I think this patch does it.

code looks good. i gave this a pretty thorough workover, testing the editing/enabling/disabling of the menu item, the accessibility of the login item as both an auth and anon user, and the destination functionality. all works flawlessly.

one question: are we creating any attack vectors by bypassing the anon user check like this?

I don't see how this could open any general security issues - the function we are bypassing is only the access check for the login and registration links. It's not generally setting the user as anonymous or with a different uid. In addition, only users with "administer menus" permission are bypassing the check in the context of listing the menu links (actually visiting that path should still give "Access denied").

chx says it needs more doxygen docs.

now with more code comments.

Chad tested it and I read the code and now I like it. So there.

This is not a regression, so it's a feature request. I'm going to postpone this until D7.

@Dries - being unable to add or edit a link to the login page is a bug. I'm not sure how 5.x handles this internally, but it *is* possible to create a link to user/login.

So, I think this is the key 5.x to 6.x difference that caused this regression: In 5.x, the menu module does no access checking or validation for the overview or edit form -

http://api.drupal.org/api/function/menu_overview_tree/5

Apparently you can also set a query string in 5.x, though again I'm not as familiar with it:

http://api.drupal.org/api/function/theme_menu_item_link/5

attached patch is more limited - just focuses on fixing the main D5 regression. Allows an admin to add a login link with menu module.

not going to re-test, since it's just a reduced version of the exact same code.

this seems like a sensible solution -- doesn't add new API functionality, and will be needed when we do fix the dynamic menu link stuff, anyways.

this is now a pretty minimal patch for 6.x, and still applies cleanly.

I can't help but think this looks a bit like a hack. I want to explore this a little bit better before committing this.

@Dries - yes it's a little bit of a hack, I'll agree.

other options I've thought about:

modules could set a global flag in when presenting a menu overview that any/all access callbacks could look at.

There cold be a global flag to bypass access for all menu links (which then means we potentially lose a the feature of not showing all links to a user with "admin menus" permission)

None of these are totally satisfying.

Dries said he would like to explore this.

ok, a different approach suggested by chx: user #1 bypasses all acces checks, so can create/edit these links if desired.

Pro: simple

Con: user 1 will have such a login link dispalyed in their menu. they can also visit pages like user/register.

I proposed this because the root user is presumed to see everything. I agree that this will make uid 1 everyday usage a bit harder but that's a good thing as uid 1 should not be used everyday.

This was fixed in a different issue.

@catch - can you please link to the issue where this is fixed?

Sorry pwolanin, couldn't find it at the time:
http://drupal.org/node/179695

However the original feature request stands, so marking back to feature and CNW for D7.

bump - moshe is crying over the lack of this in 6.x

yes indeed. the lack of dynamic querystring for menu items is devastating to devel module and og (and surely many others). the functionality i really need is in #14 here. for example, devel.module has a cache clear link that really wants a destination=[back to current page] on the querystring. since that querystring is dynamic per page view, we need the alter hook proposed here. sigh. subscribe.

Change title. The still relevant part of this patch is the menu.inc hunk in #14. IT is small, but vital.

due to : http://drupal.org/node/215858

you should be altering $item['localized_options'] rather than 'options'

http://drupal.org/node/218319