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 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:

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.

The admin pages now work if the 'Administer' menu tree is moved out of 'Navigation' (for example to 'Primary links'). Unfortunately the page titles (and breadcrumbs) are wrong for all of /admin* ("Home").

Also tested moving the 'Administer' menu tree under 'Create content' - everything works fine.

@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.

Moving the administer menu tree around under navigation works fine.
Moving the administer menu tree out of navigation works, but the page titles are all 'Home'.

Assuming the page title issue will be addressed in another patch, this one is working properly and is ready to go.

Rerolled to remove whitespace changes which were already committed and added periods to two comments.

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:

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!