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.menu_node_nodeapi() calls menu_node_delete() when the menu item has been deleted (i.e. $node->menu['delete'] = 1):
// Line 41:
menu_node_delete($node->nid, $mlid);
But the function is actually defined like this:
// Line 361:
function menu_node_delete($node) {
#<snip>
db_query("DELETE FROM {menu_node} WHERE nid = %d", $node->nid);
}
Which means it doesn't actually do anything.
I fixed it on my site as follows:
// Line 41:
menu_node_delete($node, $mlid);
// Line 361:
function menu_node_delete($node, $mlid = null) {
#<snip>
if ($mlid) {
db_query("DELETE FROM {menu_node} WHERE nid = %d AND mlid = %d", $node->nid, $mlid);
} else {
db_query("DELETE FROM {menu_node} WHERE nid = %d", $node->nid);
}
}
I didn't alter the module_invoke_all() calls because I don't know how they should handle deleting a specific menu item rather than the whole node...
Comments
Comment #1
joglin CreditAttribution: joglin commentedSubscribe for D7,
steps to replicate the error:
* Create 2 menu link items to same node
* Check menu_node table: it has 2 rows for the same node
* Delete only one menu link item
* Check the table menu_node must have a row for the node, but has none.
I splitted menu_node_node_delete() in menu_node_record_delete_by_node() and menu_node_record_delete_by_link() then I commented menu_node_node_delete() because it is needless: menu module deletes each menu item when node is deleted then invoke hook_menu_link_delete() for each item
Comment #2
agentrickardWhat version is this patch for?
Comment #3
joglin CreditAttribution: joglin commentedRolled for D7 from git master
Comment #4
agentrickardComment #5
agentrickardPlease explain why the two existing tests for this behavior are incorrect.
I should point out, by the way, that supporting multiple menu links to the same node is not really supported by core because if you edit the node, one of the links will also be deleted, yes?
Comment #7
joglin CreditAttribution: joglin commentedI do not know, the test reports:
* Failed to fetch file [http://drupal.org/files/menu_node-improper-delete record-when-menu-link-delete-D7-1257254-2.patch], instead received response [404]
maybe because the file contains a space.
Yes, multiple menu links to the same node is not really supported but multiple links are not deleted because menu module update or delete mlid. In node_prepare hook the menu module seeks mlid first in the default menu then in allowed menus based on settings in the content type.
For example, I can create multiple menus with duplicate links and enable each menu block in different pages of the site then correct the breadcrumbs with a module like custom breadcrumbs
menu_node binds menu links to node (the table primary field is mlid) then menu_node must respond primarily to menu hook and secondarily to node hook. Because the menu module already reflects the node module hook and in turn generates its hook.
Comment #8
agentrickardThanks.
Comment #9
gabriel.achille CreditAttribution: gabriel.achille commentedgood for me.
Following recent recent fix (issue 1361746) commited in dev branch, i updated this patch to use the new function menu_node_record_delete_by_link() where it should be used in the new code introduced by #1361746 fix.
Also i update the issue title, following joglin patch name and joglin patch purpose because it make more sense.
Comment #10
gabriel.achille CreditAttribution: gabriel.achille commentedsorry: clean up as well when merging with this other patch (#1361746)
Comment #11
gabriel.achille CreditAttribution: gabriel.achille commentedComment #12
gabriel.achille CreditAttribution: gabriel.achille commented