Instead of giving it priority, it uses the chosen menu at the exclusion of all others.

To test, try setting the "menutrails menu" to "primary links", and then set "parent item for story" to something in secondary links.

CommentFileSizeAuthor
#2 630296.patch1.11 KBgrendzy
menutrails_priority.patch943 bytesgrendzy
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

sun’s picture

+++ menutrails.module
@@ -86,10 +86,11 @@ function menutrails_nodeapi(&$node, $op, $a3 = NULL, $page = FALSE) {
-  $menu = variable_get('menutrails_menu', FALSE);
-  if (!$menu) {
-    $menu = db_result(db_query("SELECT menu_name FROM {menu_links} WHERE link_path = '%s' AND module = 'menu'", $item['href']));
-  }
+  $priority_menu = variable_get('menutrails_menu', variable_get('menu_primary_links_source', 'primary-links'));
+  $menu = db_result(db_query(
+    "SELECT menu_name FROM {menu_links} WHERE link_path = '%s' AND menu_name = '%s'
+    UNION SELECT menu_name FROM {menu_links} WHERE link_path = '%s'",
+  $item['href'], $priority_menu, $item['href']));

Instead of a complex UNION, why not simply use ORDER BY?

Powered by Dreditor.

grendzy’s picture

FileSize
1.11 KB

good idea; revised with ORDER BY.

Stephen Scholtz’s picture

Tried your patch, worked just fine for me!

nedjo’s picture

Status: Needs review » Closed (duplicate)

Duplicate of #475684: Menutrails assignment fails if menu item is in any menu, which is older and also has a patch.

rkendall’s picture

Status: Closed (duplicate) » Reviewed & tested by the community

I have tested the patch at #2 which works fine, and fixes the problem as described in this issue.

The issue this was marked as a duplicate of seems to be describing different issues (#475684: Menutrails assignment fails if menu item is in any menu), so I have removed the duplicate status.

Thanks grendzy for the patch!

sun’s picture

Status: Reviewed & tested by the community » Needs work
+++ menutrails.module	10 Jun 2010 21:36:20 -0000
@@ -86,10 +86,8 @@ function menutrails_nodeapi(&$node, $op,
-  $menu = variable_get('menutrails_menu', FALSE);
...
+  $priority_menu = variable_get('menutrails_menu', variable_get('menu_primary_links_source', 'primary-links'));

Why did we add an hard-coded assumption about the default menu? I think that belongs into a separate issue/patch. It seems like we can simply default to an empty string?

Powered by Dreditor.

rkendall’s picture

@sun - thanks for looking at it. Your question is worth asking, but I don't think it is really a problem.

As far as I can tell, the only time it would make a difference to the functioning is when a node is linked to multiple menus but not the chosen 'menutrails_menu' (with one of those linked menus being the primary-links). In this case, why would a completely random result be any better than giving priority to the primary links?

The behaviour of the patch in #2 makes sense to me.

Maybe in the future an interface could be provided to give an order of priority to all available menus, however such a feature would only be useful to a handful of people.

william.lai’s picture

Thanks, the patch work for me too.

AndyF’s picture

Status: Needs work » Needs review

It would be great to get this patch committed - this particular bug's caught me out in two successive sites (memory like a sieve!) and it'd be nice to put it to rest!

It seems to me that the choice of whether to default to primary links or not should be linked to what is displayed on the admin screen. Currently the admin screen defaults to primary links, so it makes sense to me to do the same in code (as in patch in #2). Unless I've missed something...? (An alternative would be to add an extra option on the admin screen to represent no preference and modify the code as sun suggests in #6.)

weseze’s picture

Status: Needs review » Reviewed & tested by the community

I've got this patch up and running on 5 installs now. Works perfectly! Please commit this.