Menu active trail query

I can verify this does not work on my installation as well (would more info about our installs be helpful?)

And is there anything else I can do to help with this issue?
I'm motivated BTW, because otherwise I'd need to write some unpleasant code to insert the active trail on a site where we want the menu expanded when user-generated nodes are in the right 'section' (via taxonomy), but don't want to give users access to the menu.

So... how can I help?

Thanks :)
Scott

Agreed -- I'm using 6.x-1.4 but the override menu checkbox doesn't seem to have any effect, even after clearing cache. I expected the behavior fgasking described as well, so if it actually does something else it might be worth a clarification in the documentation, otherwise this does seem like a bug.

I started looking into this and think I see the problem, but am still trying to decide on a solution.

Basically, I agree the menu_set_active_trail_code in the current version of the module is broken, but its not really a bug either - call it an unfinished feature request that still need work....

Here is the relevant code from hook_nodeapi:

for ($i = 0; $i < count($titles); $i++) {
        // Skip empty titles
        if ($title = trim($titles[$i])) {
          // Output plaintext instead of a link if there is a title without a path.
          $path = trim($paths[$i]);
          if (strlen($path) > 0 && $path != '<none>') {
            $trail[] = l($title, trim($paths[$i]));
            $location[$i] = menu_get_item(drupal_get_normal_path($paths[$i]));          }
          else {
            $trail[] = check_plain($title);
          }
        }
      }
      drupal_set_breadcrumb($trail);
      if ($breadcrumb->set_active_menu) {
        $location[] = menu_get_item();
        menu_set_active_trail($location);
      }

This is broken because menu_get_item returns a row from the menu router table, while menu_set_active_trail stores a trail (an array of associative arrays with keys 'title', 'href' for the title and link of the trail). I can fix the code to set a proper active trail like this:

  $location = array();
    $trail = array(l(t('Home'), '<front>'));
    $location[] = array('title' => t('Home'), 'href' => '<front>', 'localized_options' => array());
    for ($i = $offset; $i < count($titles); $i++) {
      if ($title = trim($titles[$i])) { // create breadcrumb
        $trail[] = _custom_breadcrumbs_create_crumb($title, trim($paths[$i]));
        $location[] = array('title' => $title, 'href' => drupal_get_normal_path($paths[$i]));
      }
    }
    if ($breadcrumb->set_active_menu) {
      menu_set_active_trail($location);
    }
    drupal_set_breadcrumb($trail);
  }

And in fact this will set the active trail to the paths stored in the custom breadcrumb.

But this still won't accomplish what you are asking for, at least as I understand it. Correct me if I am wrong, but I think you want to have class='active-trail' for menu link matching the custom breadcrumb path. This way your theme can either open the menu or otherwise emphasize the link in accordance with node type / custom breadcrumb.

But as far as I can tell, menu_get_active_trail (which returns the value set by menu_set_active_trail) is only called by a couple of functions and they don't theme the menu links...

To end a long story...I think the way to get the result you are looking for is to provide a function phptemplate_links that sets an 'active-trail' class according to the custom breadcrumb path - a totally different approach than what is in the current module.

Please let me know if

1. My explanation of what you would like in this feature request is correct
2. Anyone sees a way to accomplish this differently than what I propose

I don't understand either example, but I'm being affected by this lack of feature...

Wrong content-type w/ custom breadcrumb: http://skitch.com/caylan/bdu2t/carrot-prairie-farm-produce
Source: <li class="menu-218 last"><a href="/products" title=""><span>Products</span></a></li>

Custom Breadcrumb:
Title: [term]s Path: [termalias]

Correct view: http://skitch.com/caylan/bdu2w/vegetable-prairie-farm-produce
Source: <li class="menu-218 active-trail last active"><a href="/products" title="" class="active"><span>Products</span></a></li>

Email i@caylan.net for testing.

@MGN: your analysis of the problem definitely describes the problem I am having, which I'm pretty sure is same as that of the original poster. I want the active-trail class on my menu items so that I can theme them properly (keep them highlighted). Do you have an example of the theme based solution you proposed?

I'm having the same issue. The last item in the breadcrumb has a corresponding menu item which should be active. By changing the if to:

      if ($breadcrumb->set_active_menu) {
        menu_set_active_item($path);
      }

it works like I want to. This activates the path of the last breadcrumb in the menu which is exactly what I need.

@Jax: Is this in custom_breadcrumbs module? I haven't dug into the code yet, but just checking with you first -- i'd like to test this out, and can add a patch if i get the same results.

Yes, this is in custom_breadcrumbs.module in the function
function custom_breadcrumbs_nodeapi($node, $op, $teaser, $page)

@Jax: hmm, this didn't work for me. Instead of doing nothing, now with this change, my entire menu now disappears when 'Override menu path' is checked. Maybe i misunderstood though?
Around line 70 in custom_breadcrumbs.module, I replaced:

      drupal_set_breadcrumb($trail);
      if ($breadcrumb->set_active_menu) {
-        $location[] = menu_get_item();
-        menu_set_active_trail($location);
      }

with this:

      drupal_set_breadcrumb($trail);
      if ($breadcrumb->set_active_menu) {
+        menu_set_active_item($path);
      }

@jax. If you can submit a proper patch against the latest 6.x-1.x-dev code, I would be happy to evaluate it. Thanks for helping out!

Let's first see if my use case is the subject of this thread.

I have a menu structured like:

• item-one links to a node
  • item-two is a view that lists all entries of a content-type type-news

When you click item-two, item-one is expanded and item-two is active. The content is now a list of type-news nodes. What I wanted to achieve was when someone clicks one of the links in the view the menu stays active and the breadcrumbs says "Home > item-one > item-two".

With this module the breadcrumb functionality worked but the menu trail was not set to active. With the change above the last item in the breadcrumb trail is set as the active item. In my case this would be "item-two".

Is this also what everyone else tries to achieve?

I do have a wrong, unholy, wretched snippet of theme code that explicitly overrides the default menu item theming function, causing it to apply the active-trail class properly. It works in conjunction with custom_breadcrumbs, and it's the hack we used on the sxsw drupal site to make everything come together for the trails:

function custom_breadcrumbs_theme_registry_alter(&$theme_registry) {
  if (!empty($theme_registry['links'])) {
    $theme_registry['links']['function'] = 'custom_breadcrumbs_override_links';
  }
}

// Darn you, chx.
function custom_breadcrumbs_in_active_trail($link) {
  if (!isset($link) || !isset($link['href'])) {
    return false;
  }

  $activeTrail = menu_get_active_trail();
  if (!isset($activeTrail)) {
    return false;
  }

  foreach ($activeTrail as $trailStep) {
    if ($trailStep['href'] == $link['href']) {
      return true;
    }
  }

  return false;
}

// Sweet merciful God, WHY? This is horrifying.
function custom_breadcrumbs_override_links($links, $attributes = array('class' => 'links')) {
  $output = '';

  if (count($links) > 0) {
    $output = '<ul'. drupal_attributes($attributes) .'>';

    $num_links = count($links);
    $i = 1;

    foreach ($links as $key => $link) {
      $class = $key;

      // Add first, last and active classes to the list of links to help out themers.
      if ($i == 1) {
        $class .= ' first';
      }
      if ($i == $num_links) {
        $class .= ' last';
      }
      if (isset($link['href']) && ($link['href'] == $_GET['q'] || ($link['href'] == '<front>' && drupal_is_front_page()))) {
        $class .= ' active';
      }
      if (custom_breadcrumbs_in_active_trail($link) && ($link['href'] != '<front>')) {
        $class .= ' active-trail';
      }
      $output .= '<li'. drupal_attributes(array('class' => $class)) .'>';

      if (isset($link['href'])) {
        // Pass in $link as $options, they share the same keys.
        $output .= l($link['title'], $link['href'], $link);
      }
      else if (!empty($link['title'])) {
        // Some links are actually not links, but we wrap these in <span> for adding title and class attributes
        if (empty($link['html'])) {
          $link['title'] = check_plain($link['title']);
        }
        $span_attributes = '';
        if (isset($link['attributes'])) {
          $span_attributes = drupal_attributes($link['attributes']);
        }
        $output .= '<span'. $span_attributes .'>'. $link['title'] .'</span>';
      }

      $i++;
      $output .= "</li>\n";
    }

    $output .= '</ul>';
  }

  return $output;
}

For the curious, this basically hijacks the theme system and replaces the normal theme_links() implementation with a copy-and-hacked version that adds active-trail. The downside is that it's hacky, and it has the potential to generate additional DB queries when checking for the active trail. It does work, though. any thoughts on making this some sort of configurable option in Custom Breadcrumbs? Perhaps the 'force active trail' option should be a global switch rather than per-breadcrumb...

@Eaton, thanks for sharing this snippet! I think it really helps to illustrate what the override menu path feature in custom breadcrumbs was supposed to allow for. I think most users expect this kind of "complete solution" when they set the 'override menu path' option for a breadcrumb, so I agree that it would be good to complete it as an optional global configuration.

Since this feature is already halfway into the DRUPAL 6--1 codebase, I would suggest this could be prepared, tested, and released as cb 6.x-1.5. We could then incorporate it in the 6.x-2.x development as well. Does this sound appropriate?

Here is what i would suggest:

1. Fix the currently broken code in custom_breadcrumbs_nodeapi as per #4 - I need feedback on this...

2. Add a global configuration page at admin/settings/custom_breadcrumbs

3. Fix up Eaton's snippet, as necessary, so the feature behaves as expected.

If this sounds right, lets develop a patch against the current 6.x-1.x-dev version and test it out.

I'm still a little confused on the code in #4 - the current code in the dev release that sets the active trail appears to work without modification on several sites I've worked on; it just needed the theme_links overriding to make the proper classes show up. I could be missing more complex interactions, though.

I think a global flag for this behavior is definitely useful; perhaps the per-breadcrumb 'override menu trail' could be removed, as mentioned, and it could all rely on the central flag at admin/settings/custom_breadcrumbs. Then, the custom_breadcrumbs_theme_registry_alter() function could do a quick variable_get() to see whether it should hijack theme_links.

Which is to say, yes, that sounds like a pretty sweet plan.

Here is what I am struggling with in the current custom_breadcrumbs 6.x.1.x-dev code. And I believe this is why Jax and Scottrigby changed it...

$location[] = menu_get_item();
menu_set_active_trail($location);

menu_get_item() returns a row from the menu_router table, which has about 20 fields, but none of these fields are 'href', which is needed in your code snippet.

In the fix described in #4, I've removed the menu_get_item() and replaced it with a proper trail, the kind of trail that menu_set_active_trail() would produce.

$location[] = array('title' => $title, 'href' => drupal_get_normal_path($paths[$i]));

I think this is the kind of array your code snippet needs...but correct me if I am wrong.

Node Breadcrumb may solve some problems for you who want to add breadcrumbs for nodes, and also want to have the active trail set. It doesn't allow you to set custom breadcrumbs in the way custom_breadcrumb does. But Node Breadcrumb may solve some simpler needs.

Hi,

Any idea when a resolution for this will be added into 6.x.1.dev?

Cheers
Zap

subscribing.

I am going to move forward fixing the menu_set_active_trail_code as I describe in #4 above, unless someone disagrees with my assessment of the problem in #16. I'll add Eaton's code snippet in 6.x-1.x-dev for testing and we can take it from there.

subscribing

subscribing

subscribing

I have been working with Eaton's code snippet and now see that it doesn't override menu item theming. Since it replaces theme_links, only the page links themed by this function are affected. But following this approach, hook_theme_registry_alter can also be used to override theme_menu_item and theme_menu_item_link....

The attached patch (against 6.x.1.x-dev) provides the feature originally requested by fgasking. It

• Adds a global configuration page at admin/settings/custom_breadcrumbs with an option to force the active trail (for both menu links and page links). If this option is selected, the theme_registry will be rebuilt to let custom breadcrumbs override the relavent theme functions.
• Removes the per-breadcrumb override menu path option.
• Fixes the currently broken code in custom_breadcrumbs_nodeapi as per #4
• provides three override functions (for theme_links, theme_menu_item and theme_menu_item_link) and a few helper functions to carry out the work. This includes Eaton's code snippet in #13 for the theme_links override, and code (derived primarily from the DHTML_menu project) to expand/truncate the menu tree according to the active trail set through the custom breadcrumb.

It takes quite a bit of code to accomplish all this. If its possible to trim it down I would like to do this. But right now a working solution is better than no solution!

As long as we are posting *unholy* patches, here's something i mocked up before I found this thread.

The specific problem this fixes is that the Override menu option doesn't work for theme('links') which is used to display $primary_links and $secondary_links in page.tpl.php. So I wrote this to rebuild the primary and secondary links in hook_preprocess_page() in a way that would will add the active trail classes. There is no need to hijack the theme so maybe its not as unholy as eaton's patch.

function custom_breadcrumbs_preprocess_page(&$vars) {
  $active_trail = menu_get_active_trail();
  if (!$active_trail) {
    return;
  }
  
  //Save q so we can reset it afterwards.
  $q = $_GET['q'];

  // Use the first or second element in the active trail depending on 
  // if the first element is a link to <front>
  $i = $active_trail[0]['href'] == '<front>' ? 1 : 0;

  // set $_GET['q'] to the 1st link in the active trail to fool the menu system 
  // into printing the primary links with the new active trail.
  if ($active_trail[$i]['href'] != $q) {
    $_GET['q'] = $active_trail[$i]['href'];
    $vars['primary_links'] = menu_primary_links();
    $_GET['q'] = $q;
    $i++;
  
    if ($active_trail[$i]['href'] != $q) {
      // Only process secondary_links if there is a 2nd item in the active trail.
      if (isset($active_trail[$i])) {
        // set $_GET['q'] to the 2nd link in the active trail to fool the menu 
        // system into printing the secondary links with the new active trail.
        $_GET['q'] = $active_trail[$i]['href'];
        $vars['secondary_links'] = menu_secondary_links();
        $_GET['q'] = $q;
      }
    }
  }
}

Since we are trying to keep the 6.x-1.x branch lean, for now I have released 6.x-1.5 without the menu active trail query. 6.x-2.0-beta has a global option to force the menu active trail which seems to be working. Since we have a working implementation in custom breadcrumbs 2.0, I am marking this issue as fixed. If there is significant interest we can consider backporting it to the 6.x-1.x branch.

I have absolutely like comment#12 situation.

I use current dev-6.x version of the Custom breadcrumb module.
Here it is
My menu structure:
primary_menu
--News
----Culture news
----Sciense news

1) I have a node (type=page) called "news" and it is in the primary_menu as item "News".
2) I have a node type "news" and node "new-number-1" of this type
3) I set path to my page "news" as a breadcrumb for all my nodes of the type news
4) I "Forced the active trail" on the custom breadcrumb settings page
5) I have some items under my "News" item in primary menu, such as "culture news", "sciense news"
6) I have a menu_block with 2-nd level of menu in left sidebar, when I go to page "News" it appears.
7) I have a breadcrumb like "main > News" on my news nodes such as "new-number-1"
8) BUT when I go to my "new-number-1" - there is no menu block in left sidebar, it is lost, though item "News" in primari menu has class "active-trail".

There can be a lot of news and the number of them will be increased, so it is not acceptable to have all them in menu. But I need to imitate that I am in the news subdirectory when I go to any node of this type. And don't want to loose navigation block.

Is it possible to not to loose navigation blocks using current version of the module?

Track Back in case someone needs to use dynamic persistent module.
Custom Breadcrumb breaks the active trail usage of this module.
http://drupal.org/node/606968

clean install of dev over head solved this issue.

@netbear and sgriffin. If you have a support request or a bug report, please open a new issue. Thanks.