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.
When using menu_link_maintain to edit a link ($op = 'update'), the menu cache is not cleared unless the menu item you're editing is the the 'navigation' menu. The reason is that menu_cache_clear isn't passed the correct menu_name as a parameter so uses 'navigation' by default.
A quick solution would be to patch menu_link_maintain to call menu_cache_clear_all instead.
Current code (menu.inc line 2040)
function menu_link_maintain($module, $op, $link_path, $link_title) {
switch ($op) {
...............
case 'update':
db_query("UPDATE {menu_links} SET link_title = '%s' WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
menu_cache_clear(); // clears cache for navigation menu when no parameter is passed in
break;
...............
}
}
A solution (maybe not a good one)
function menu_link_maintain($module, $op, $link_path, $link_title) {
switch ($op) {
...............
case 'update':
db_query("UPDATE {menu_links} SET link_title = '%s' WHERE link_path = '%s' AND customized = 0 AND module = '%s'", $link_title, $link_path, $module);
menu_cache_clear_all(); // clears cache for all menus
break;
...............
}
}
Comment | File | Size | Author |
---|---|---|---|
#8 | maintain-cache-278458-6.patch | 907 bytes | pwolanin |
#4 | maintain-cache-278458-4.patch | 920 bytes | pwolanin |
Comments
Comment #1
pwolanin CreditAttribution: pwolanin commentedYeah, I think I noticed this at one point recently but didn't open an issue.
This code is problematic since we don't know which menus we are saving to. The solution you suggest would work, but we would invalidate less of the cache maybe if we did something like:
Comment #2
pwolanin CreditAttribution: pwolanin commentedso, chx agrees in IRC that this is the sensible approach
Comment #3
j0hn-smith CreditAttribution: j0hn-smith commentedA much better solution.
Comment #4
pwolanin CreditAttribution: pwolanin commentedjust rolled the above into a patch. not really tested yet.
Comment #5
catchPatch looks completely fine to me. I ran all tests, no regressions, and manually edited a menu item. Didn't yet look inside the cache_menu table to ensure it's actually deleting the correct records though.
Comment #6
catchSpoke to pwolanin in irc, doesn't seem to be a way to reproduce this in core. Since it's pretty minor, going to call it RTBC.
Comment #7
catchNope, it doesn't work. Regression in aggregator module when changing a category name - menu items should change with it, but they don't with the patch applied.
Comment #8
pwolanin CreditAttribution: pwolanin commentedok, that was a stupid bug - extra argument to db_query() was causing the SELECT query to fail.
Tested and seems to work as expected.
Comment #9
catchThis one works fine :)
Comment #10
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Moving to Drupal 6.
Comment #11
Gábor HojtsyCommitted to 6.x. Thanks!
Comment #12
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.