Hello.

I've created next menu structure for my primary links:

Contents
-Page1
--Page1-1
-Page2
-Page3

I've disabled "Contents" item, because wanted my primarylinks start from next stage (Page1,Page2...)

When I done this, primery links show me that I expected (Page1,Page2...)
But, secondary links did not work, when you are in Page1 section (but it have to).
I've dugg menu.inc file and found interesting thing: _menu_get_active_trail() and _menu_get_active_trail_in_submenu() are not checking the items on enabled/disabled state. So, I've added this checking and all becomes fine. Also, I tested this patch on my production sites. No one seems crashed :) So, please, review this patch, I hope it would be usefull.

Comments

neochief’s picture

StatusFileSize
new1.03 KB

With some additional testing, found bug in my patch in several situations. Here is the update.

drumm’s picture

Status: Needs review » Needs work

This patch needs to follow our coding standards at http://drupal.org/coding-standards. Specifically, if blocks always need to use {} and the conditional should not have linebreaks.

neochief’s picture

StatusFileSize
new1.03 KB

I beg your pardon, I didn't found any words about this in coding standarts, but I'll be know it in the future. I've updated the patch, please check it.

neochief’s picture

Status: Needs work » Needs review
drumm’s picture

Status: Needs review » Closed (works as designed)
($menu['items'][$mid]['type'] & MENU_VISIBLE_IN_TREE & MENU_MODIFIABLE_BY_ADMIN)

will always be false since MENU_VISIBLE_IN_TREE & MENU_MODIFIABLE_BY_ADMIN is zero. So, the condition can be simplified to

!($item['type'] & MENU_MODIFIABLE_BY_ADMIN)

Or, the menu has some other type bit than MENU_MODIFIABLE_BY_ADMIN. I think this might be true for all items. So I'm either reading this wrong, or this patch doesn't do anything.

I am also not sure that changing the menu behavior is appropriate at this point in the 5.x release cycle.

neochief’s picture

Sorry, I not remember the problem exactly, but anyway, it's not big deal already, as it was reported more than year ago :) But anyway, thanks for your reply, Neil.