Moving the "Administer" menu item breaks pages

Yes, I've noticed this too. I think the way it is coded is a bit faulty in that it makes assumptions about the depth of the menu links which do not hold if they are moved. So, we need more robust code.

Admin! Bad dog! Sit!

no - I think the logic is not good.

don't we need something like:

  $admin = db_fetch_array(db_query("SELECT mlid, menu_name FROM {menu_links} WHERE module = 'system' AND link_path = 'admin'"));
  $result = db_query("
    SELECT *
    FROM {menu_links} ml
    INNER JOIN {menu_router} m ON ml.router_path = m.path
    WHERE ml.link_path like 'admin/%%' AND ml.link_path != 'admin/help' AND ml.plid = %d AND ml.menu_name = '%s' AND hidden = 0
    ", $admin);

admin may not be in the 'navigation' menu, but there should only be one hook_menu defined instance of this path (right? http://api.drupal.org/api/function/_menu_navigation_links_rebuild/6).

we are only getting items at a single depth right, so there is no need for the ORDER BY. I think there is a sort later.

I do not think there is a sort of the blocks. Let's see this version.

Um. Maybe this.

Hmmm, now I remember - you want this to be blank if moved out of the navigation menu?

This is wrong no matter what:

ORDER BY p1 ASC, p2 ASC, p3 ASC"

and I still think it should be removed. The array of blocks should get indexed with 50000+weight and title and sorted like links, I think.

I still don't like that solution. I think it's enough that the blocks don't show if 'admin' and the blocks are in different menus. Then you can move the blocks out of 'navigation' (or vis-versa) if you want to construct your own custom control panel. Also, changing the weight via menu module ought to affect the bloc ordering or item ordering.

how about this?

plid checking in itself is adequate i think, no need for menu_name checking (as plid is nonzero). also admin/% path checking is not needed, whatever is under administer item is fine.

@chx - there is an index on (menu_name, plid) but not just on plid so I think the query would be faster leaving menu_name in the WHERE.

@webernet - actually the BC is right - for this reason: The menu system builds the BC based on the 'active' menu (defaults to the Navigation menu). So if you move an item out of the active menu, it will not have an active trail/BC other than "Home". Maybe we should cross-check what happens in D5, but I think it's similar.

Ah, apparently the titles are wrong too. However, that's all separate from this issue. Here's the issue I'd been planning to use to tackle these sorts of problems: http://drupal.org/node/155624

ok, I think this addresses problems with the previous patch in terms of queries and descriptions.

It would be great to document, why the 50000+ sorting weight is required. What does it need to be after?

@Gabor - in both places I added have this code comment:

 // Prepare for sorting as in function _menu_tree_check_access().

since there is a detailed explanation in that function - do we also need a detailed explanation here?

Erm, a short doc would be great there.

ok, added additional code comments.

Ample comments. Functionality same as the one webernet tested.

Thanks for the update, committed!