I'm trying to theme links in a menu.
Name of the theme function in my case is menu_link__main-menu.
I got it from $elements['#theme'] in drupal_render.
But I can not create such function, because its name contains a hyphen.

Files: 
CommentFileSizeAuthor
#11 drupal-tests-1001146.patch4.86 KBedhaber
PASSED: [[SimpleTest]]: [MySQL] 31,564 pass(es). View
#10 drupal-tests-1-1001146.patch4.86 KBedhaber
PASSED: [[SimpleTest]]: [MySQL] 31,529 pass(es). View
#9 drupal-tests-1001146.patch4.86 KBedhaber
PASSED: [[SimpleTest]]: [MySQL] 31,557 pass(es). View
#7 drupal-tests-1001146.patch4.87 KBedhaber
PASSED: [[SimpleTest]]: [MySQL] 31,579 pass(es). View
#1 drupal-1001146.patch765 bytesdroplet
PASSED: [[SimpleTest]]: [MySQL] 30,370 pass(es). View

Comments

droplet’s picture

Version: 7.0-rc2 » 7.x-dev
Status: Active » Needs review
Issue tags: +Quick fix
FileSize
765 bytes
PASSED: [[SimpleTest]]: [MySQL] 30,370 pass(es). View
effulgentsia’s picture

Status: Needs review » Needs work
Issue tags: -Quick fix +Needs tests

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

markabur’s picture

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’s picture

Title: Cannot theme links in a menu. » Cannot theme links in a menu when menu_name contains a hyphen.
droplet’s picture

@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)

markabur’s picture

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.

edhaber’s picture

Status: Needs work » Needs review
FileSize
4.87 KB
PASSED: [[SimpleTest]]: [MySQL] 31,579 pass(es). View

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.

markabur’s picture

Status: Needs review » Needs work
Issue tags: -Needs tests

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'));
edhaber’s picture

FileSize
4.86 KB
PASSED: [[SimpleTest]]: [MySQL] 31,557 pass(es). View

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.

edhaber’s picture

FileSize
4.86 KB
PASSED: [[SimpleTest]]: [MySQL] 31,529 pass(es). View

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

edhaber’s picture

Status: Needs work » Needs review
FileSize
4.86 KB
PASSED: [[SimpleTest]]: [MySQL] 31,564 pass(es). View

Needs review. Thats what I missed.

Jeff Burnz’s picture

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) {}
droplet’s picture

@#12,
after apply patch ??

dropcube’s picture

Status: Needs review » Reviewed & tested by the community

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.

upupax’s picture

Worked for me with MYTHEME_menu_link__menu_menu_footer

catch’s picture

Version: 7.x-dev » 8.x-dev
Issue tags: +needs backport to D7
jeffschuler’s picture

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

Dries’s picture

Version: 8.x-dev » 7.x-dev

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

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x. Thanks!

plach’s picture

Status: Fixed » Needs work
+++ 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.

sun’s picture

Status: Needs work » Closed (fixed)

Shit happens.

phpconnect’s picture

Status: Closed (fixed) » Active

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

catch’s picture

Status: Active » Closed (fixed)

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

Elijah Lynn’s picture