Cannot theme links in a menu when menu_name contains a hyphen.

Thanks for finding and fixing this. Can you add a test, so that it stays fixed?

This should have been fixed as part of #890884: Targeted overrides for theme_menu_link() and theme_menu_tree() fail for menus with a hyphen. Following that example, here's how to reproduce the problem here:

Add this to Garland's template.php and add the user-menu block to a region:

function garland_menu_link__user_menu(&$vars) {
  dpr('user menu'); // this does not print!
  return theme_menu_link($vars);
}
function garland_menu_link__management(&$vars) {
  dpr('management menu'); // this does print!
  return theme_menu_link($vars);
}

I can confirm that the patch in #1 does cause 'user menu' to be printed twice (once for each link in the menu) -- so, the patch is RTBC as far as that goes.

I'd be willing to work on the automated test but could use a pointer about where it should go and whether or not we've got similar tests someplace already.

@markabur,

I tried to understand what you said..at beginning I'm agreed with you... but when I dig into code...
It seems like "print twice" is EXPECTED.

menu_link__user_menu (#theme) is used for print each links, so it should be printed TWICE (or same as how many links are there).
(that we can alter each link)

if you use menu_tree__user_menu (#theme_wrappers) it will be ONCE.

function garland_menu_link__user_menu(&$vars) {
  dpm($vars);
  dpr('user menu');
  return theme_menu_link($vars);
}

for above example, you will get 2 array & printed twice BUT each array is difference. Each call is for different things.

(hope I'm not wrong)

Yes, we are saying the same thing. Before the patch, "user menu" doesn't print at all due to the bug, and afterwards it prints twice as expected (once for each link). So, I still say the patch looks good except for needing tests.

I created a new test object for this menu_tree_output function and created tests for this case, the #theme_wrapper and then a few more tests for the function.

Nice! Reviewed the patch and noticed a couple things.

The title on these two shouldn't be 'Item 5':

+    '6' => array('link' => array( 'menu_name' => 'main-menu', 'mlid' => 6, 'hidden'=>0, 'has_children' => 0, 'title' => 'Item 5', 'in_active_trail' => 0, 'access'=>0, 'href' => 'f', 'localized_options' => array('attributes' => array('title' =>'')) ), 'below' => array( ) ),
+    '7' => array('link' => array( 'menu_name' => 'main-menu', 'mlid' => 7, 'hidden'=>0, 'has_children' => 0, 'title' => 'Item 5', 'in_active_trail' => 0, 'access'=>1, 'href' => 'g', 'localized_options' => array('attributes' => array('title' =>'')) ), 'below' => array( ) )

The last test should be testing item 7, not item 6:

+    $this->assertFalse( isset($output['6']), t('False access should be missing'));
+    // Item 7 is after a couple hidden items. Just to make sure that 5 and 6 are skipped and 7 still included
+    $this->assertFalse( isset($output['6']), t('Item after hidden items is present'));

Thank you for checking that. My copy and pasting got a little away from me there but at least I commented what I was trying to test.
Here is an updated patch with those changes.

Do I need to rename the patch for it not to be ignored by the test system?

Needs review. Thats what I missed.

Perhaps related but I can't seem to get any override running (menu name hyphen or not), I was helping a guy in the forums with this (which is how I came to this issue), even using something like this does not appear to work:

function genesis_SUBTHEME_menu_link__management(&$variables) {}

@#12,
after apply patch ??

After applying the patch it works. I am implementing

function MYTHEME_menu_link__main_menu(array $variables) {
...
}

and it's working.

This is a simple patch that should have been fixed as part of #890884: Targeted overrides for theme_menu_link() and theme_menu_tree() fail for menus with a hyphen, but the menu_link part was not included.

Worked for me with MYTHEME_menu_link__menu_menu_footer

Patch in #11 worked in D7 for THEMENAME_menu_link__primary_links().

Committed to 8.x. Moving to 7.x. Thanks!

Committed to 7.x. Thanks!

+++ modules/simpletest/tests/menu.test	9 Feb 2011 12:39:38 -0000
@@ -846,6 +846,64 @@ class MenuTreeDataTestCase extends Drupa
+  }
+  ¶
+  function setUp() {

It seems some trailing whitespaces have just been committed...

Powered by Dreditor.

Shit happens.

I am still getting this issue.
i have added the method
_menu_link__menu_menu in templates.php but it's still not working.

Please open a new bug report for your issue, it's unlikely to be directly related to this one.