I have a website where the client wishes to point some extra menu items to an existing page. For example:

Services
-- Service 1
-- Service 2

Methods (node/34)
-- Internal
-- External

About US
-- Mission
-- Staff
-- Our Methods (extra menu item point at node/34)

In this case the "Our Methods" menu item is pointing to the "Methods" item. I know there could arguably be some UX-problems with this kind of interlinking in menus, but sometimes it makes sense.

The problem though, is that if you do this, clicking on Methods will set the active trail to "Our Methods". It will also in the case of Menu Blocks show the "wrong" branch of the tree, if you have made a menu block, starting from second level. Surely the right behaviour would be to set the active trail too the node that actually holds the menu-item - Methods, not Our Methods - it's just pointing.

Why does it do this? I expect the problem is caused by the function _menu_link_get_preferred($path = NULL, $menu_name = ''), which is a modification of menu_link_get_preferred from includes/menu.inc. In both functions when selecting candidates for the active menu item, the function retrieves all menu items pointing to node/34, traversing them and picking the last. This is accidentially the last created menu-item (highest mlid).

But since the highest mlid is the Our Methods menu item, this is picked as the most active item, and thereby setting the context - incorrectly in my opinion! The normal use-case would be to create the node with menu item first, and later make the pointing links. But there are afaik no way I can see in menu_links, which mlid originated from the node-form.

But, looking at the function menu_node_prepare($node) in the menu.module, I can see

$mlid = db_query_range("SELECT mlid FROM {menu_links} WHERE link_path = :path AND menu_name = :menu_name AND module = 'menu' ORDER BY mlid ASC", 0, 1, array(

So here Drupal concludes that, if there are more than one item point to the same URL take the lowest mlid. So in lack of anything else - Drupal assumes that the first created mlid belongs to the node.

This brings me to my conclusion: I tried to put
$query->orderBy('mlid', 'DESC');

in the query selecting the candidates in _menu_link_get_preferred. This actually solved the problem. Also the same should be done in prefered link function in menu.inc, to set the right active trail.

But I'm not a Drupal coder, so maybe I'm off the wrong track here. If this is considered a feature, or for some other reason is the right behaviour, I apologise for taking your time.

BTW, Menu Block is a fantastic module.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mazaza’s picture

Project: Menu Block » Drupal core
Version: 7.x-2.3 » 7.12
Component: Code » menu system

This is not longer part of menu block module - the mentioned function was included in the file custom_menu_active_trail.inc, which is not part of menu block anymore. Because http://drupal.org/node/942782 has been fixed.

BUT, the "bug" still exists - but it should be addressed in Drupal core (menu.inc)

jerhu-1’s picture

Rather than

$query->orderBy('mlid', 'DESC');

we could use the weight attribute

$query->orderBy('ml.weight');

mazaza’s picture

I'm not sure!

The weight doesn't reflect, which menu item is "connected" to the node! The author might have placed the node form menu item deep down the menu hierarchy, and the extra link in the top level.

Like this:

Services
-- Service 1
-- Service 2

Our Methods (extra menu item point at node/34)

About US
-- Mission
-- Staff
-- Methods (node/34)
-- Internal
-- External

paulmckibben’s picture

Title: Setting active menu item from highest mlid when multiple items point to same node - instead of lowest mlid » Node menu link is inconsistent with active trail when multiple items point to the same node
Version: 7.12 » 7.15

An additional problem is that this "highest mlid" setting for the active trail is inconsistent with the "lowest mlid" setting for the node's menu link setting.

I am using pathauto with the pattern "[node:menu-link:parents:join-path]/[node:title]" to create an automatic URL alias for the node. However, the node's menu link uses the lowest mlid, while the active trail uses the highest mlid. This inconsistency results in the URL alias reflecting a completely different path from the expanded menu items.

gabriel.achille’s picture

Assigned: Unassigned » gabriel.achille
Status: Active » Needs review
FileSize
565 bytes

please find a proposal patch attached.

Following previous recommendations I just added: $query->orderBy('mlid', 'DESC'); inside menu_link_get_preferred function. Basically it make the active trail use the lowest mlid of a node.
Note that this is the same patch a the one proposed for drupal 8.x in http://drupal.org/node/1649062 (not sure how patch should be ported...)

mbrett5062’s picture

I am reviewing your patch. New install of 7.15.
Create menu items as described and yes, when clicking original menu entry I end up in sub menu on later additional link.

I.E.

A
--A1
B
--B1
--A1a (Link to A1)

Clicking A1 takes me to A1a in parent B, showing page A1

However this behavior does not change when I have installed your patch.
I have tried with your patch as is, and have also tried changing to ascending sort. No change.

gabriel.achille’s picture

... I'm confused. I did the same test on a fresh drupal 7.15 install and it works. First without the patch and then with. I've got the behavior you describe without patch and the opposite with the patch.
Actually you have to clear the menu cache. Do you think we need to add something like cache_clear_all(NULL, 'cache_menu'); inside a hook_update_N ?

mbrett5062’s picture

I did clear all caches. As for adding something, I am not sure/do not have enough knowledge.

I will try again, to be sure I did nothing wrong. Let you know when re-checked.

Ok re-checked this and for some reason I can not fathom, now it works as expected.

Please ignore my previous comment.

This is fixed just needed to flush all caches after applying patch.
I was sure I had done so before, maybe just a brain fart.

This is RTBC as far as I am concerned.

mbrett5062’s picture

Status: Needs review » Reviewed & tested by the community
David_Rothstein’s picture

Status: Reviewed & tested by the community » Closed (duplicate)

Marking this as a duplicate of #1649062: Core is inconsistent when handling two menu items with the same path. It says one thing in the UI and does another in reality., since although this issue was older, that one had the patch posted first, and it needs to get committed to Drupal 8 before it can go into Drupal 7.

I'll tag the other issue for backport, though. Please feel free to follow up there. Thanks!

aschiwi’s picture

I found a patch from this issue in a project we recently took over. Since it did the job and the other ticket has no solution, I re-did the patch for the latest Drupal version 7.59.

bensti’s picture

@aschiwi woow! work just great