MENU_LOCAL_TASK disappears when same path is used in a primary link

Confirmed in HEAD.

Confirmed, title correct. I've had a quick look and can repeat but not see why at this time. I hope to get some time soon to investigate further. I'll probably have to discuss with chx on IRC but DrupalCON is going to get in the way of alot of things (unless their "bug sessions" look at this issue ;)

In the menu building process, the $_menu array is populated. Path and item are stored in $_menu['path index'] as [path] => [item_id]. The item with the item_id will contain menu information such as title and type.

In that sense, one path can only be binded to one item. In this case, when the path is a set as a primary link, it cannot be a MENU_LOCAL_TASK at the same time.

I think this conflict might happen for all menu items that share the same path. It might take alot of rewriting to fix this.

Then this is a prime candidate for the "Bug Cafe" at DrupalCON whilst heads are together

Rejoice for I got JonBob on the issue. YAY! I see a forthcoming patch soon :D

I don't have a solution yet, but here's the workflow that causes the problem.

A module defines a menu item which is a local task. The type MENU_LOCAL_TASK is given to this item, which consists only of the flag MENU_IS_LOCAL_TASK.

The menu items are normally stored in the {menu} table by menu_rebuild(). However, this only happens for items that have the MENU_MODIFIABLE_BY_ADMIN flag. This flag is not set for local tasks, so the menu item is not stored in that table.

The user adds a menu item with the same path as the local task. This is stored in the {menu} table in the database.

The _menu_build() function is called to render the menu for the page. Starting at menu.inc line 1091, the saved menu items are read from the database. At this point the local task has already been added to the list of menu items, but has a negative (temporary) mid.

Line 1098 returns TRUE, as the custom menu item and local task have the same path.

Line 1101 returns TRUE, as the local task has a negative mid. This means it is overwritten.

Now this behavior of overwriting the menu item is in place so that module-defined items don't exist twice in the menu when they are customized. Maybe we need to use the "shortcut" behavior from 1113 when the module-defined item wasn't MODIFIABLE_BY_ADMIN?

JonBob was wrong. The problem was in _menu_append_local_tasks(). The local task was not added to the $new_items array because the path was already in the menu.

Patch works for:
/user/#/edit

Patch does not work for:
/user/login
/admin/build/themes/settings

According to kkaefer these are a related, but separate issue since they are stored in the database rather than being generated.

This patch does now fix the other issue as well. In fact, JonBob was not completely wrong. For menu items which are generated in hook_menu(FALSE) ($may_cache == FALSE), the fix is in _menu_append_contextual_items() (my first patch). For cacheable menu items, the problem was in fact in _menu_build(). There, the old menu item was removed although it was still necessary for the local tasks. The menu items are now only removed if the new menu item is of the old menu item’s type. Additionally, we merge the types together so that we can have local tasks and other flags like MENU_VISIBLE_IN_TREE.

Patch works for:
/user/#/edit
/user/login
/admin/build/themes/settings

I'd like to get the opinion of a menu system wizard on this.

I don't much like special cases, but with code as complex as the menu system I guess they're inevitable.

This patch seems to fix the problem and the conditionals seem to limit side effects to just the problematic items.

+1.

Committed to HEAD.