When you have multiple items in a menu that point to the same path the menu system will only set the trail to the menu item that is returned first when searching for parents in menu_tree_page_data().
If you create a node which gets the path node/1 and you create a first menu item "One" which points to this node and then you create a second menu item "Two" that also points to this node as a child of menu item "One" that second menu item won't have its in_active_trail property set to true. Moreover if you create yet another menu item as a child of menu item "Two" the item two will remain collapsed thus not divulging the child menu items.

I've attached two patches, the first one use the deepest menu item to create the trail and the second one uses all the menu items to create the trail.

Comments

jax’s picture

Status: Active » Needs review

Patches were attached for review.

ronino’s picture

Thank you so much for this patch. I struggled a long time with this bug, moving around and re-creating menu items all the time without getting it fixed... The interesting thing is that I had this problem only with one particular trail whereas it worked fine for other items.

anrikun’s picture

StatusFileSize
new1.23 KB

I've run into the same problem on a multilingual site, when having 2 menu items (one in English, the other in French) pointing to the same View path.
Active trail will then only work with the first language, not with the other.
The attached patch solves this problem.

anrikun’s picture

StatusFileSize
new1.71 KB

Sorry the patch above is incomplete.
Here's the full patch to apply to Drupal 6.14's includes/menu.inc:

jax’s picture

@anrikun: Please create a new thread for your issue which isn't the same problem this issue was created for. Your patch doesn't even incorporate the changes I suggest.

For people reading this thread, please ignore comments #3 and #4.

anrikun’s picture

Sorry Jax,
I wanted to integrate your changes but I did not understand how as there are 2 different patches.
It would be good to mix the 3 patches into one.

juhaniemi’s picture

Subscribing. We're having these issues on a multilingual site. At least the menu2.patch seems to solve our problem.

harrrrrrr’s picture

@anrikun: your patch worked on 6.14, but doesn't work anymore with 6.15...

anrikun’s picture

Yes it's normal as a patch is always tied to one version.
So this patch needs to be updated for 6.15.

harrrrrrr’s picture

it does seems to work, my bad

mrtorrent’s picture

subscribing

stephandale’s picture

Thanks Jax. Your first patch, which uses all menu items, works well with i18n. In fact I'd go so far as to say it's essential.

I've been working on a site with multiple languages and the stock menu_tree_page_data prevents you linking to a language neutral page from many language specific menus. (When I say language neutral I mean those nodes with their language set to "all languages").

In my case I'm using a single menu that contains a mix of language specific and language neutral items, organised in a hierarchy. When you switch language, you want the menu tree to show items for the selected language as well as any language neutral items.

Viewing in English:
- English section (node/1)
-- English page (node/3)
-- Neutral page (node/5)

Viewing in Finnish:
- Finnish section (node/2, a translation of node/1)
-- Finnish page (node/4, a translation of node/3)
-- Neutral page (node/5)

As stock, menu_tree_page_data only allows the neutral page to have the correct menu in one language. So you could view the neutral page in English, but when you switch to Finnish all menus are lost.

When patched, so that all menu items are used, the i18n module removes those items that are in the wrong language and everything seems to work perfectly.

I've attached a copy of your patch (menu1.patch) updated for 6.19. No code has changed - only the line numbers.

Status: Needs review » Needs work

The last submitted patch, menu.inc_.patch, failed testing.

Sarenc’s picture

My problem is related to this and so I thought I would post my solution.

When adding a second menu item manually to a node, in addition to the menu item created by the node menu settings, the second menu item sometimes receives the active-trail. The behaviour I expect is that the menu item created on the node edit form should always receive the active trail leaving menu items added through the menu system to act as simple redirections.

My solution was to change the query in menu_tree_page_data
From:
$parents = db_fetch_array(db_query("SELECT p1, p2, p3, p4, p5, p6, p7, p8 FROM {menu_links} WHERE menu_name = '%s' AND link_path IN (". $placeholders .")", $args));
To:
$parents = db_fetch_array(db_query("SELECT p1, p2, p3, p4, p5, p6, p7, p8 FROM {menu_links} WHERE menu_name = '%s' AND link_path IN (". $placeholders .") ORDER by customized ASC", $args));

This way the non-customized menu item (the one added on the node edit menu settings) always achieves the active trail before any menu items manually added.

Sarenc’s picture

It turns out the customized column isn't representative of what I thought. Is there any way to distinguish between a menu item created by the node menu settings form or through the menu admin page?

anrikun’s picture

IFAIK no.

chaugi’s picture

Status: Needs work » Needs review

#12: menu.inc_.patch queued for re-testing.

chaugi’s picture

#12 patch is working like a charm on Drupal 6.19, should be included in the core. Strange that patch does not pass automated testing.

Status: Needs review » Needs work

The last submitted patch, menu.inc_.patch, failed testing.

anrikun’s picture

Status: Needs work » Needs review

This is because the automated test expects the patch to be for Drupal 7, and it is not the case here.

sp3boy’s picture

Status: Needs review » Reviewed & tested by the community

I applied this patch in D6.19 and it corrected the "random" assignment of active trail to only one of my menu items. With it applied, all menu items pointing to the front page were identified as active when I was browsing the front page.

gábor hojtsy’s picture

Version: 6.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

1. This looks to possibly have all kinds of other interesting side effects beyond just active trail changes, right? Menu items will get more parents.

2. There does not seem to be consideration here that this same issue is probably present in Drupal 7. Bugs are fixed first there if applicable.

jurriaanroelofs’s picture

This problem was bugging met when testing menus on themes that I'm converting to D7.

To me this seems like a major issue because it visually breaks Drupal sites that have multiple links to the same place. It is default to have more than 1 link to the homepage in Drupal 7, so the active styling on the home link in any menu that style li.active-trail will be broken or missing for all users. That is, all users who use a theme with sliding doors CSS* on primary menu items, or other advanced scripting or styling on menus.

Out of the 10 themes that I'm currently upgrading, 8 are broken**. Furthermore I've tested 5 of the most popular themes in contrib that have a 7.X version and 3 of them rely on the active-trail on the li tag (Kanji, Omega, Danland). I see the Omega theme has "fixed" this by using Javascript to add .active-trail classes on the li tags, I'm sure this is not what we want our Drupal 7 themes to do.

*This requires styling on the anchor tag as well as on the li tag and/or an extra tag inside the link
**active styling does not appear or does not appear completely on affected links

jurriaanroelofs’s picture

Priority: Normal » Major

see comment above

Hawaiipersson’s picture

Status: Patch (to be ported) » Needs review

menu1.patch queued for re-testing.

timhilliard’s picture

Although this is multilingual I think it is the same issue as #942782. Can someone confirm this?

jax’s picture

The basic issue is no way related to multilingual functionality.
This issue isn't the same as the one you reference, it might be somewhat related but it's not the same IMO.

dmitriy.trt’s picture

subscribing

carn1x’s picture

subscribing

coderintherye’s picture

Status: Needs review » Needs work

There isn't a patch here for 7, let alone a valid patch, and I'm not sure at this point if we have to create a patch first for 8 then backport it.

I do still consider this a core bug, though I wonder if there is a duplicate of this issue out there.

dealancer’s picture

Hey guys.

Patch http://drupal.org/node/618700#comment-3697756 could probably solve your issues with menu on multilingual site, it is for D6 yet.

sun’s picture

Version: 7.x-dev » 6.x-dev

I'm pretty sure that this bug does not exist in D7, due to the heavy re-factoring into menu_build_tree().

That said, the patch doesn't look right to me. Someone should compare the logic of the D7 code and check for viable solutions for D6.

coderintherye’s picture

This bug very much occurs in D7, and is easily reproducable, although I'd agree that a solution for 6 is not going to be the same as for 7 due to the refactoring. To reproduce on 7, simply install, create two menus, put the same link in both menus. See that only one as set as the active trail. Where this typically bites me is when a link is in the footer, and nested in the main menu, depending on conditions, it returns the footer item first. Or could try in the same menu with nested menu items in two different places in the menu. I haven't tried on 8, but probably would be best to segregate this out to a separate bug for 8, put in the steps to reproduce, and patch as appropriate.

For those working on this though, I'm going to have to disagree with looking at 7 for a solution, unless sun is not able to reproduce the behavior I have seen above.

Cyclodex’s picture

StatusFileSize
new74.43 KB

I went to the same Problem on the current Drupal 7.4.
I am configuring a multilingual site for english/german and came across the same problem.

I have attached a picture which should explain and show the behavior...

The english nodes are expanded, when changing to german, it is broken.
The crazy thing is, that it recognizes that there are "sub" items, and shows a arrow, but no items.

What I discovered to is, if you set your parent menu items (here: about us / über uns), the items will be shown, but of course all the time !

I just wanted to share that with you and hopefully we will find a solution for that...

othermachines’s picture

Using Domain Access with multiple domains and spent the last 3 hours trying to figure this problem out. The patch in #12 seems to have solved things for me in 6.22. I'll report if any side effects come up.

othermachines’s picture

Issue tags: +active menu path
StatusFileSize
new1.69 KB

Damn! Side effect from patch in #12:

I have a child of one branch linking to the parent of another branch. When either one is selected, the menu system thinks both branches are active. Boo hoo.

I've been mulling over this today and think I may have come up with a more intuitive solution (unfortunately perfection is impossible, here, given the way the core menu system works).

It works as follows:

In the case where there is more than one match on a link path, give precedence to lowest depth (limit 1), then find other matches in the same branch. This approach prevents two branches from being active simultaneously and addresses a common use case where both the parent and first child of a menu branch point to the same location.

I've given first priority to lowest depth (highest menu position) because, in the case where two items in a menu point to the same page, I think the intention is more likely that an item placed lower in the hierarchy should direct to the higher item (thus making it "active"), not vice versa.

This isn't going to work as expected 100% of the time, but I think it increases the odds over the current implementation. I'm not sure if it addresses the issues described in #12. I'm using the patch successfully (for the moment) on a multilingual and multidomain site.

@nowarninglabel - Re: "when a link is in the footer, and nested in the main menu, depending on conditions, it returns the footer item first" - I'm a little confused by this... are you talking about two separate menus?

Here's the patch...

coderintherye’s picture

@othermachines Yes, two separate menus, say you have an "About Us" which you put in your primary links, then you create another menu for your footer, and you also place the "About Us" link there, same path. Problem occurs.

However, I haven't tested since 7.2., so need to do some testing on a fresh 7.8 install to see where things are at.

othermachines’s picture

@nowarninglabel That makes sense. Not sure about 7.x, but in 6.x there doesn't appear to be any way to determine which menu item or menu has been selected because the look-up solely relies on the path. Seems to me we can make the system make smarter guesses as to which to load but beyond that I don't think there's a whole lot that can be done without altering the db.

That said, in my experience (from an information architecture point of view) it's generally wise to avoid duplicating menu items-- and I almost always recommend against it-- but of course there will always be clients who are adamant. Fat footers are somewhat of an exception and they have gained in popularity (in which case you end up with a situation like you just described).

I think the first menu item created (lowest mlid) will be returned as active. In which case you could just make sure to build out your menus in a certain order, or swap the mlids if you're comfortable with manipulating the db. Kind of a pain, though.

A menu item setting "Set active priority (Always mark this item as active when path is loaded.)" might solve part of the problem-- at least maybe in 7.x? I haven't yet spent as much time with 7.x as I would have liked.

Cheers-

mostou’s picture

I have just quickly tested in Drupal 7.10 and i18n 7.x-1.2 - I couldn't reproduce the error. Can other confirm that this is not an issue in Drupal 7.10?

svdhout’s picture

Drupal 7.10, i18n 7.x-1.2
I am using one menu containing different items for different paths.
e.g. an english item 'Articles' linked to 'articles', and a dutch item 'Artikels' linked to 'articles' within 'main-menu'.

The problem seems to be at

function menu_link_get_preferred($path = NULL) {
...
foreach ($query->execute() as $candidate) {
  $candidate['weight'] = $candidate['link_weight'];
  $candidates[$candidate['link_path']][$candidate['menu_name']] = $candidate;
}
...

The menu_name and the link_path are the same for both items, thus the last candidate overwrites the first candidate in the candidates array.

mostou’s picture

This is still an issue with Drupal 7.12, i18n 7.x-1.3 and Menu Block 7.x-2.3. The reason why I couldn't reproduce the error earlier was because of a workaround to another core issue in Menu Block that is now removed.

mostou’s picture

This is no longer an issue with Drupal 7.12 + i18n 7.x-1.4 (+ Menu Block 7.x-2.3). I assume it was fixed with: #1351678: Follow menu_link_get_preferred active trail handling for custom menus and #942782: Custom menus never receive an active trail

This issue was in fact handled in #942782: Custom menus never receive an active trail for Drupal 7. So this issue is only for Drupal 6 - the pathces posted is also for Drupal 6. If you still experience any issues in Drupal 7 please post in the two other issues.

ttiurani’s picture

Unfortunately I still have an issue with the "active-trail" class not showing up on <li> for menu items with Drupal 7.14. The problem I have is that "active-trail" is missing for only a menu link to the front page (path "<front>"), all other links have "active-trail" just fine.

The situation is also slightly different for the horizontal primary link tabs that can be switched on from theme settings Toggle display - Main menu, than they are for the vertical navigation in the Main menu block. In the horizontal tabs the front page <li> has "active" but not "active-trail". So "active" could be used to do get around this issue for horizontal menus. But unfortunately I want the vertical menu and in that there is no "active" nor "active-trail".

I also tried different themes and created a new testmenu with a link to the front page with exactly the same problem.

I suspect this then might have something to do with "<front>" not being handled properly?

p.s. If this is the wrong place to notify about this, where should I comment? (I don't quite understand what was meant by "two other issues".)

Edit: I ended up creating a new issue:
#1578832: [already commited but followup for CR?] <front> menu links are missing active trail classes

mostou’s picture

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.