Put phptemplate_links in include file, make it optional

Sure this is a real issue. I'm bumping this bug to critical because if you enable the module and your theme already have a phptemplate_links() function, then you will severily break your site (PHP spits out "already defined function" error and you can't disable the module anymore).

(If you're experiencing this problem, the only solution is to edit your theme's template.php file and comment out the phptemplate_links() function, access the module listing, disable the Menu Trails module, and only then uncomment your original phptemplate_links() function.)

and now were at it by having a phptemplate_links() theme call in the module it will screw up any theme inheritance that a themer wants to have in his setup. it goes in and overwrites the parenttheme_links() :(

Can someone check whether we still need phptemplate_links() at all? I've heard rumors that the stock theme functions would already apply the "active-trail" + "active" classes now.

I don't think this is still needed. theme_links now applies an active class to the li element of active menu items in the same way that this function does, which it previously did not.

However, the phptemplate_links() function contains this line of code $link['attributes']['class'] .= ' active'; that the new theme_links does not. This sets an active class on the a element as well as the li element. Is this necessary? Any examples of something where a.active would work and li.active a would not?

If we do still need it though this could be done in a preprocess function.

Glad to see I'm not alone with this issue.

I created a patch that moves phptemplate_links() into a separate include file and adds a settings option to choose not to include it. The file is included by default.

Hope it's useful.

This won't fly.

Instead, we want to massage the theme registry, and only alter theme_links into menutrails_links in case the theme system didn't detect another theme override function.

Ok here's another try -- this time using hook_theme_registry_alter() to register "menutrails_links" if default "theme_links" function is being used.

+++ menutrails.module	7 Nov 2009 02:18:45 -0000
@@ -411,6 +411,36 @@ function _menutrails_recurse($tree, $hre
 /**
+ * Implementation of hook_views_pre_view().
+ *
+ * This is invoked for every view before the it is themed, even if the view is
+ * cached.
+ */
+function menutrails_views_pre_view(&$view) {
+  if ($view->display_handler->display->display_plugin == 'page') {
+    drupal_set_breadcrumb(menutrails_get_breadcrumbs());
+  }
+}
@@ -483,17 +513,4 @@ function phptemplate_links($links, $attr
-/**
- * Implementation of hook_views_pre_view().
- *
- * This is invoked for every view before the it is themed, even if the view is
- * cached.
- */
-function menutrails_views_pre_view(&$view) {
-  if ($view->display_handler->display->display_plugin == 'page') {
-    drupal_set_breadcrumb(menutrails_get_breadcrumbs());
-  }
-}

erm?

+++ menutrails.module	7 Nov 2009 02:18:45 -0000
@@ -411,6 +411,36 @@ function _menutrails_recurse($tree, $hre
+ * 
+ * Massage the theme registry, and only alter theme_links into menutrails_links 
+ * in case the theme system didn't detect another theme override function.

This should be an inline comment.

+++ menutrails.module	7 Nov 2009 02:18:45 -0000
@@ -411,6 +411,36 @@ function _menutrails_recurse($tree, $hre
+    $theme_registry['links'] = array(
+      'function' => 'menutrails_links',
+      'arguments' => array(
+          'links' => NULL,
+          'attributes' => array('class' => 'links'),
+      ),
+    );

Instead of completely overriding the entire info, we just want to replace the 'function' in the array.

+++ menutrails.module	7 Nov 2009 02:18:45 -0000
@@ -483,17 +513,4 @@ function phptemplate_links($links, $attr
+}
\ No newline at end of file

Lastly, this should not happen under any circumstances.

I'm on crack. Are you, too?

Issues addressed as noted - let me know if this works.

+++ menutrails.module	7 Nov 2009 02:48:33 -0000
@@ -411,6 +411,17 @@ function _menutrails_recurse($tree, $hre
+  // If the theme system didn't detect another theme override function,
+  // alter theme_links into menutrails_links.

This can also be fixed before committing, so just FYI: function names in comments should always be suffixed with parenthesis, i.e. function_name(), to make clear that it's talking about a function.

I'm on crack. Are you, too?

I've used this on a handful of client sites now without problems. Assuming the commenting issues can be fixed when committing, it would be great to get it into a release.

Thanks for reporting, reviewing, and testing! Committed to 6.x.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.