Updated: Comment #19

Problem/Motivation

A programatically added menu item is not removed from the menu after the menu link code has been removed.

Steps to reproduce

  • Create a module with a single route and callback.
  • Add a menu link to the route using an implementation of hook_menu_link_defaults(). Add the menu link to the 'main' menu (for convenience).
  • Enable the module.
  • Check that the menu link appears in the main menu.
  • Remove the code from MODULE_menu_link_defaults().
  • Clear the cache
  • Observe that the menu link is stil in the main menu.

Proposed resolution

Remaining tasks

User interface changes

API changes

Files: 
CommentFileSizeAuthor
#14 drupal_1079628_menu_type_change_13.patch3.75 KBhefox
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal_1079628_menu_type_change_13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#3 menu_alter_callback-1079628-3.patch4.52 KBCaligan
PASSED: [[SimpleTest]]: [MySQL] 35,912 pass(es).
[ View ]
#1 menu_callback_drop-1079628-1.patch1.41 KBCaligan
PASSED: [[SimpleTest]]: [MySQL] 35,763 pass(es).
[ View ]

Comments

Version:7.0» 7.x-dev
Status:Active» Needs review
StatusFileSize
new1.41 KB
PASSED: [[SimpleTest]]: [MySQL] 35,763 pass(es).
[ View ]

Successfully replicated this with the provided steps. Menu items registered in the menu_links table that are later turned into MENU_CALLBACKs (which should not be in menu_links) aren't removed from the table by _menu_navigation_links_rebuild() because their paths are still valid.

This patch sieves for MENU_CALLBACK items and removes them from menu_links along with any stale entries.

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

patch looks good. nitpick:

<?php
+    ->condition(db_or()->condition('router_path', $paths, 'NOT IN')->condition('router_path',$paths_drop, 'IN'))
?>

missing a space after 'router_path' in the second db_or() condition.

here are some hints to help you build a test:

<?php
// file modules/simpletest/tests/menu_test.module
/**
* Implements hook_menu().
*/
function menu_test_menu() {
 
// This item is menu-altered by another test module.
 
$items['menu_alter_test'] = array(
   
'title' => 'Test hook_menu_alter',
   
'page callback' => 'node_save',
   
'type' => MENU_NORMAL_ITEM,
  );
// file: modules/simpletest/tests/menu.test
class MenuRouterAlterTestCase extends DrupalWebTestCase {
  public static function
getInfo() {
    return array(
     
'name' => 'Menu alter',
     
'description' => 'Tests menu alter functionality.',
     
'group' => 'Menu',
    );
  }
  public function
setUp() {
   
parent::setUp('menu_test');
  }
  public function
testMenuAlter() {
   
// Check that our link from menu_test_menu() is registered.
    // Enable our menu alter test module, clear caches.
   
module_enable('menu_alter_test');
   
// Validate that our menu link is gone.
 
}
}
// file: modules/simpletest/tests/menu_alter_test.module
/**
* Implements hook_menu_alter().
*/
function menu_alter_test_menu_alter(&$items) {
 
// Set an item to MENU_CALLBACK from MENU_NORMAL_ITEM.
 
$items['menu_alter_test'] = MENU_CALLBACK;
}
// file: modules/simpletest/tests/menu_alter_test.info
name = "Hook menu alter tests"
description = "Support module for menu alter hook testing."
package = Testing
version
= VERSION
core
= 7.x
hidden
= TRUE
dependencies
[] = menu_test
?>

Status:Needs work» Needs review
Issue tags:-Needs tests
StatusFileSize
new4.52 KB
PASSED: [[SimpleTest]]: [MySQL] 35,912 pass(es).
[ View ]

Patch bundled with a test verifying that menu items changed to MENU_CALLBACK are removed from menu_links table (and so from visible menus).

Title:Changing a Menu Types Doesn't Remove Items From menu_link tableMenu link is not removed after changing the menu router item's type
Version:7.x-dev» 8.x-dev

#1148940: Menu links not removed from menu has been marked as duplicate

Status:Needs review» Reviewed & tested by the community

Hi,

works fine for me. It can be good tested with Views and also fixes: #1029022: Menu item created in a view is not removed from the menu when removed from the view

Status:Reviewed & tested by the community» Needs review

This needs some more reviews, especially from menu system maintainers.

I don't have time myself right now, but will try to come back to this issue shortly.

+++ b/includes/menu.inc
@@ -2689,11 +2689,16 @@ function _menu_link_build($item) {
     if ($item['_visible']) {
...
     }
+    // Identify MENU_CALLBACK items; used to discard records in menu_links.
+    if ($item['type'] === MENU_CALLBACK) {
+      $menu_drop[$path] = $item;
+    }

Why isn't this simply an "else" ?

-2 days to next Drupal core point release.

Same question as sun; switching to other type of menu items other than normal item, like local tasks, will make them not-menu-links, so it'd only work for a subset of links it needs to.

Why keep track of both path and item if only using path? Seems like a waist of memory to keep track of extra information.

the patch (#3) did not removed the menu entry created by a view (dup: #1418618: Views may leave stale menu entries)

Status:Needs review» Needs work

Status:Needs work» Needs review

Here's a slightly different take on it
Patch:

  • Instead of an or condition, changes the paths it checks against to be only visible paths, so the query instead of being "delete all non-customized menu links who's path doesn't exist anymore in menu router" (what core has) is "delete all non-customized menu entries that who's path isn't "visible" in the menu -- ie has a menu entry". This seems to work and makes logically sense to me.
  • Moves the test into menu_test where the other core menu tests exists
  • Changes the static function to be more usable if need to add more test on menu information changing.

Didn't really change the actual test other than the function call that sets the static.

StatusFileSize
new3.75 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch drupal_1079628_menu_type_change_13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Right.. the patch.

The patch menu_callback_drop-1079628-1.patch works for me.

Status:Needs review» Needs work

The last submitted patch, drupal_1079628_menu_type_change_13.patch, failed testing.

Title:Menu link is not removed after changing the menu router item's typeProgramatically added Menu link is not removed after removing the code.
Issue summary:View changes

Since the original report and patches the menu system got a full overhaul, but the problem still remains. Changing issue title and description to match the current situation.