The setting name "Show current term in breadcrumb trail" leads one to assume that it applies to the current term being viewed. The description however talks about showing the lightest term when a node is being viewed. I think we should always show the complete path leading up to the node, but I don't think we should show a link to the current term being viewed in the breadcrumb. Therefore, I think we should remove this setting and use the functionality above.

Also, I think the "Show current node title in breadcrumb trail" should be "Show current page title in breadcrumb trail" (notice the word page instead of node). This means that when a term page is viewed, we would add the term name into the breadcrumb.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jantoine’s picture

Patch attached implementing proposed solution.

jantoine’s picture

Status: Active » Needs review
lmeurs’s picture

I think this module should indeed always show the full path in the breadcrumbs.

Myself I like to see the last breadcrumb to represent the current node/term/etc. as a link with active class for styling purposes (I know this is against Drupal standards). I even prefer to make the last breadcrumb bold, so it's extra clear to the visitor where he is right now in the trail of breadcrumbs and where he came from. I understand that this is supposed to be done in another place (using theme_breadcrumb() or sitetheme_menu_breadcrumb_alter()), site wide.

I applied all of your patches (some patches I needed because of errors, the others I applied out of precaution) and saw your patch from #1311302: Incorrectly validating on taxonomy term pages changed this behavior. When both 'Show current term in breadcrumb trail' and 'Show current node title in breadcrumb trail' are turned OFF, the terms show as expected when browsing the terms, but when viewing a node the lightest term is not shown.

A few small alterations of the code fix this and return the old functionality of letting the user choose to show the current term/current node's title as the last breadcrumb (now consistent as a link with active class). The first alteration is for taxonomy_breadcrumb.inc (I'm not good with patches, so I'm going to do it this way):

-    if ($is_term_page && $parent_term->tid == $tid) {
-      $breadcrumb[] = check_plain(_taxonomy_breadcrumb_tt("taxonomy:term:$parent_term->tid:name", $term_title));
-    }
-    else {
+    if (!$is_term_page || $parent_term->tid != $tid || variable_get('taxonomy_breadcrumb_show_current_term', TRUE)) {
+      $breadcrumb[] = l(_taxonomy_breadcrumb_tt("taxonomy:term:$parent_term->tid:name", $term_title), $term_path);
    }
  }
-
-  // Optionally remove the current TERM from end of breadcrumb trail.
-  if (!variable_get('taxonomy_breadcrumb_show_current_term', TRUE) && !is_null($breadcrumb)) {
-    array_pop($breadcrumb);
-  }
  return $breadcrumb;

In taxonomy_breadcrumb.module I changed in function taxonomy_breadcrumb_node_view():

  if (variable_get('taxonomy_breadcrumb_include_node_title', FALSE)) {
-    $breadcrumb[] = check_plain($node->title);
+    $breadcrumb[] = l($node->title , 'node/' . $node->nid);
  }

And finally, I think it's an improvement to also set the active trail when viewing a node. This way active menu items will get the 'active-trail' class. It has been a while since I changed the active trail for a module, so I will have to dig deep in it... :-)

lmeurs’s picture

In respond to (my own) previous post about setting the active trail when viewing a node, I created a separate feature request issue at #1623314: Set active menu item when viewing a node.

jantoine’s picture

Committed patch in #1 due to an accidental partial commit when committing another patch. Leaving status as 'needs review' until the comments in #3 can be investigated.

philipz’s picture

Well I've applied attached patch based on #3 to the latest version and it seems to be working fine both on the node and term pages.

One important thing is that some theme's may mess with breadcrumb too.
My zen subtheme was set to "Append the content title to the end of the breadcrumb" with gave me double node titles in the breadcrumb.