Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#11 | drupal-tests-1001146.patch | 4.86 KB | edhaber |
#10 | drupal-tests-1-1001146.patch | 4.86 KB | edhaber |
#9 | drupal-tests-1001146.patch | 4.86 KB | edhaber |
#7 | drupal-tests-1001146.patch | 4.87 KB | edhaber |
#1 | drupal-1001146.patch | 765 bytes | droplet |
Comments
Comment #1
droplet CreditAttribution: droplet commentedComment #2
effulgentsia CreditAttribution: effulgentsia commentedThanks for finding and fixing this. Can you add a test, so that it stays fixed?
Comment #3
markabur CreditAttribution: markabur commentedThis 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:
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.
Comment #4
markabur CreditAttribution: markabur commentedComment #5
droplet CreditAttribution: droplet commented@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.
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)
Comment #6
markabur CreditAttribution: markabur commentedYes, 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.
Comment #7
edhaber CreditAttribution: edhaber commentedI 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.
Comment #8
markabur CreditAttribution: markabur commentedNice! Reviewed the patch and noticed a couple things.
The title on these two shouldn't be 'Item 5':
The last test should be testing item 7, not item 6:
Comment #9
edhaber CreditAttribution: edhaber commentedThank 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.
Comment #10
edhaber CreditAttribution: edhaber commentedDo I need to rename the patch for it not to be ignored by the test system?
Comment #11
edhaber CreditAttribution: edhaber commentedNeeds review. Thats what I missed.
Comment #12
Jeff Burnz CreditAttribution: Jeff Burnz commentedPerhaps 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:
Comment #13
droplet CreditAttribution: droplet commented@#12,
after apply patch ??
Comment #14
dropcube CreditAttribution: dropcube commentedAfter applying the patch it works. I am implementing
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.
Comment #15
upupax CreditAttribution: upupax commentedWorked for me with MYTHEME_menu_link__menu_menu_footer
Comment #16
catchComment #17
jeffschulerPatch in #11 worked in D7 for THEMENAME_menu_link__primary_links().
Comment #18
Dries CreditAttribution: Dries commentedCommitted to 8.x. Moving to 7.x. Thanks!
Comment #19
webchickCommitted to 7.x. Thanks!
Comment #20
plachIt seems some trailing whitespaces have just been committed...
Powered by Dreditor.
Comment #21
sunShit happens.
Comment #22
phpconnect CreditAttribution: phpconnect commentedI am still getting this issue.
i have added the method
_menu_link__menu_menu in templates.php but it's still not working.
Comment #23
catchPlease open a new bug report for your issue, it's unlikely to be directly related to this one.
Comment #24
Elijah Lynn