menu_name in hook_menu is ignored on updates

this is breaking an upcoming Views feature: #284893: Allow admin to place menu items into primary links or other menus.. i will review this soon.

I tested that this allows uncustomized items to be moved to another menu and does not allow save for customized. This should backport to 6 after commit to HEAD.

Bump - this is a bug fix. Please commit and backport. chx has agreed with the patch as well.

Subtle (or not so subtle) features or bugs like these are best committed along with a test case. If we break the behavior, we want to know this instantly.

+1 this - I'm experiencing the issue with D6. Failing code looks like this:

function dev_menu() {
  $content_type = 'guide';
  $menu_name = 'guides';
  $result = db_query("SELECT nid, title FROM {node} WHERE type = '%s'", array($content_type));
  while($node = db_fetch_array($result)) {
    $this_node = node_load($node['nid']);
    $items[$menu_name.'/'.$node['nid']] = array(
      'title' => $node['title'],
      'page callback' => 'node_page_view',
      'page arguments' => array($this_node),
      'access callback' => 'node_access',
      'access arguments' => array('view', 1),
      'type' => MENU_NORMAL_ITEM,
      'menu_name' => $menu_name,
    );
  }
  return $items;
}

'menu_name' is ignored. Judging by the title of this issue, I guess it would've worked if I'd placed this code first time, but subsequent updates to 'menu_name' fail, but I didn't test for that. Just simply 'menu_name' is ignored in Drupal 6.4 with the above code.

I guess you could turn the above in to a test case by setting the $menu_name to something that exists and setting $content_type to 'page' (assuming you have some pages). Check the menu builds correctly, change $menu_name to something else, clear caches and see what happens. Old behaviour is "nothing - menu item stays where it is!"

Here is a set of test files that confirm that Peter's patch above is a successful bug fix and never shall we break it :)

These files are the start to a full suite of hook_menu() tests. These files belong in modules/simpletest/files. Remove the .txt extension. They don't belong with the existing menu tests since those tests the menu.module which is a GUI for menu.inc. These files are for non GUI functionality.

As mentioned before, Peters patch should be applied to D6 once this lands in HEAD.

chx and I both cringed at the $GLOBALS thing in there, which could possibly affect the environment running the tests, rather than the test environment. Marking CNW for now, but once the test is fixed up, this should be good to go according to chx, moshe, and earl. :)

Works well and fixes the bug.

Committed to HEAD. Thanks, Peter!

Changing to D6 for the bug fix. Please just commit the patch to menu.inc, not the test files.

Committed to DRUPAL-6 as well. Thanks! :)