Menu module for D6

happy 3rd year of Drupal!

Setting to critical (since it is)

Starting to review the patch. Why are you removing this code?

-
-    // Update the has_children status of the parent
-    $children = (bool)db_result(db_query("SELECT COUNT(*) FROM {menu_links} WHERE plid = %d AND hidden = 0", $item['plid']));
-    db_query("UPDATE {menu_links} SET has_children = %d WHERE mlid = %d", $children, $item['plid']);

I added submitted this as a menu.inc patch (http://drupal.org/node/145058#comment-250460) because without this code the has_children flag remains inappropriately set when items are deleted (I saw this problem while converting the book module to the menu API).

1. I was getting unintialized variable notice problem was on line 599 in menu.module variable has to be initialized before appending new values. I updated the patch.

2. When navigating to Administer->Site building->Menus I get 3 default menus but when clicking on List tab on this page I get top level links from Navigation menu (My Account, Create Content, Administer,...). I havent found why this is happening yet.

On a clean CVS, applying menu_module_patch_9, I'm getting:

patching file includes/menu.inc
patching file modules/menu/menu.install
patching file modules/menu/menu.module
Hunk #3 FAILED at 142.
Hunk #4 FAILED at 273.
Hunk #5 FAILED at 490.

Uploading the .rej file.

File didn't get uploaded.

subscribing to this (too many menu issues :P)

patches with no error.

I can add menu's but there is no option to delete the created menus (maybe i am missing something)
I added items to the created menu:normal menus with links to internal and external links.
I edited one of the external linked menu and replaced the path with '1' which in turn brought about this fugly error:

warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, '' was given in /home/flk/public_html/test.techarena.co.uk/includes/menu.inc on line 377.

No way to get rid of the created sub-menu since it no longer shows up in UI.
I am trying a quick and dirty backtrace will see if i can follow it

I think this code you're adding to menu.inc in

+    $t_args = array('%title' => $item['link_title']);
+    drupal_set_message(t('The menu item %title has been deleted.', $t_args));
+    watchdog('menu', 'Deleted menu item %title.', $t_args, WATCHDOG_NOTICE);

would be better off in menu.module rather than in menu.inc. For example, if the menu.inc framework is used by book, I think this message would be confusing to see any time a node is deleted.

Found a bug in the add menu code- it should check if the menu_name already exits in {menu_links}, not just {menu_custom}.

Attached patch changes these two things.

Also, the tabs are a bit confusing- patch changes the default tab titles to "List menus" and "List items" . Otherwise it wasn't clear to me that I was on a different set of tabs.

There seems to be no interface for deleting a menu - is that coming later?

Disabling a node link is a custom menu generates this error:

warning: Missing argument 3 for menu_flip_item() in /Users/Shared/www/drupal6/modules/menu/menu.module on line 222.

Seems &$form_state was added as a first parameter by mistake. Fixed in this patch.

Attached patch fixes block titles and deltas.

Committed to CVS HEAD. Thanks.

menu_edit_item_validate() is bit behind times, since wrong variable names was being used internal paths were not getting validated.
refer to #11 for example.

well first of all the $items was getting overwritten in the 2nd loop making some of the references such as $item['original_item']['link_path'] etc in undefined... also spoke to chx who informed me there was he 'unaware of any problems with a path repeating in a menu'.
Therefore removed the elseif statement check.

removed the whole else/elseif check for items pointing to the same path.
since pointing different items to the same path will not cause conflict i do not think this is really necessary..users can have as many items pointing to the same path as they want (yes it is inefficient but really that's not our problem)

Patch prevents the creation of invalid internal paths.

Perhaps the #description for $form['link_path'] should say "'The path this menu item links to. This can be a valid internal Drupal path..." but that doesn't mean this isn't RTBC.

@chx: I'm loosing track- is there a separate issue with the 5.x to 6.x update code?

sorry - didn't mean to change the status; found the issue with the update code: http://drupal.org/node/147657

Committed to CVS HEAD. Thanks.