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.
Comment | File | Size | Author |
---|---|---|---|
#11 | drupal-menu_link_get_preferred-1472354-11.patch | 606 bytes | aschiwi |
#5 | drupal-menu_link_get_preferred-1472354-5.patch | 565 bytes | gabriel.achille |
Comments
Comment #1
mazaza CreditAttribution: mazaza commentedThis 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)
Comment #2
jerhu-1 CreditAttribution: jerhu-1 commentedRather than
$query->orderBy('mlid', 'DESC');
we could use the weight attribute
$query->orderBy('ml.weight');
Comment #3
mazaza CreditAttribution: mazaza commentedI'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
Comment #4
paulmckibbenAn 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.
Comment #5
gabriel.achille CreditAttribution: gabriel.achille commentedplease find a proposal patch attached.
Following previous recommendations I just added:
$query->orderBy('mlid', 'DESC');
insidemenu_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...)
Comment #6
mbrett5062 CreditAttribution: mbrett5062 commentedI 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.
Comment #7
gabriel.achille CreditAttribution: gabriel.achille commented... 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 ?Comment #8
mbrett5062 CreditAttribution: mbrett5062 commentedI 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.
Comment #9
mbrett5062 CreditAttribution: mbrett5062 commentedComment #10
David_Rothstein CreditAttribution: David_Rothstein commentedMarking 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!
Comment #11
aschiwi CreditAttribution: aschiwi at undpaul commentedI 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.
Comment #12
bensti CreditAttribution: bensti commented@aschiwi woow! work just great