Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Er, I know D6 is still very beta, so I might be just pushing it, but the absence of a nice admin menu makes it so tedious to develop for D6 currently, that it would be really neat to have the module updated for the new menu system :-)
Comment | File | Size | Author |
---|---|---|---|
#88 | cleanup-D6-132524-88.patch | 4.07 KB | pwolanin |
#86 | admin_menu.mlids_.patch | 1.35 KB | sun |
#85 | admin_menu-DRUPAL-6--1.node-add.patch | 2.79 KB | sun |
#84 | cleanup-D6-132524-84.patch | 6.73 KB | pwolanin |
#83 | cleanup-D6-132524-82.patch | 5.88 KB | pwolanin |
Comments
Comment #1
sunI had a quick look at the new menu system. First of all - bad news: The global variable
$_menu
does not exist anymore. It seems that there's no function yet designed to retrieve (and alter) the complete, expanded menu tree.This is shocking… Because until now the main advantage of admin_menu was that it doesn't alter the menu in any way. We might need to completely rewrite the module to extract administrative menu items to a new menu at installation time.
I'll greatly appreciate any input and/or patches from anyone having deeper insights into the new menu system.
Comment #2
sunAfter talking to chx on IRC the new menu system will have a hook_menu_alter() that will allow the operations needed for admin_menu. However, the hook still is under heavy development, so we'll have to wait until there is a solid ground on which we can base this update.
Comment #3
yched CreditAttribution: yched commentedOK, thanks for following this :-)
Comment #4
sunSince Drupal 6 beta is about to be released, Admin Menu should be available ASAP.
Currently, I'm very busy with several projects, so if anyone has successfully or unsuccessfully attempted to port Admin Menu for 6.x I would be glad to test, review, fix or commit any patch. :)
Comment #5
sunFirst try. Includes plenty of debug code.
Critical issue: menu_links does not contain any local tasks. In this patch, I've tried to retrieve them manually through a crude database query. Even if this could work out somehow, we'd still have an unresolved issue regarding adding and altering menu items.
A completely different approach could be:
Comment #6
alliax CreditAttribution: alliax commentedsubscribing and so sad to have started a multi install in d6 without checking on this module.. so sad..
Comment #7
WorldFallz CreditAttribution: WorldFallz commentedI just loaded up a brand spanking new multi-site dev environment based on D6. I'm no coder, but I 'm a big fan of this module and would love to contribute testing. Should I start with the patch posted here, which is probabably out-of-date already, or will a 6.x-dev be available sometime soon? FYI I"m windows based and have access to FF2, IE6, and IE7 for testing purposes.
Comment #8
sun@WorldFallz: Patch in #5 should still apply, but the outlined problems remain.
Comment #9
keesje CreditAttribution: keesje commentedsubscribing
Comment #10
koorneef CreditAttribution: koorneef commentedsubscribing
Comment #11
sunNew patch with improvements by smk-ka. Displays some local tasks (not all).
Comment #12
alliax CreditAttribution: alliax commentedsorry to ask here, but i'm interested by the patch and this time there's just too many things to modify:
how can I apply a patch file to a module file? which software do you use to automatically remove/add the lines in the patch?
Cheers,
James
Comment #13
sun@alliax: See http://drupal.org/patch/apply
Comment #14
ekendra CreditAttribution: ekendra commentedsubscribing. cheers!
Comment #15
manerhabe CreditAttribution: manerhabe commentedIs it supposed to spit the menu out at the bottom in a unordered list format (does so with multiple core themes)? Just wondering, because my patch might of applied incorrectly. I didn't get any errors, but I had to get some old versions of files (admin_menu.info, .module, .inc) from CVS because the patch was out of date since it was created for an old dev version no longer available on the releases page.
Comment #16
derjochenmeyer CreditAttribution: derjochenmeyer commentedsubscribing.
Comment #17
derjochenmeyer CreditAttribution: derjochenmeyer commentedi would be happy to test :) patching fails with 5.x-dev and 2.4 :(
Comment #18
eigentor CreditAttribution: eigentor commentedI also tried tthe patch on 2.4: 3 out of seven hunks failed :( Daniel, gib dir einen Ruck, wir wollen doch auch in Drupal 6 aminmenuisieren - sorry for the german interlude
Comment #19
chx CreditAttribution: chx commentedI would think you can do
Comment #20
John Bickar CreditAttribution: John Bickar commentedYay! Admin Menu rules!
Subscribing. Patch in 11 failed for me on 5.x-2.x-dev
Comment #21
sunRe-rolled patch against latest HEAD.
@chx: Thanks for your input! I've added your proposed code to admin_menu_tree_all_data(), commented-out for now, because:
returns all top level items of the 'navigation' menu instead of all items below the 'admin[ister]' menu item. However, we could work around this later.
The real question is: I do not understand, why the menu system only returns the top level items below 'admin' (i.e. "Content management", "Site building", aso.), but not all menu items below 'admin'.
If only 'mlid' is selected, it doesn't return any items below 'admin' at all.
Comment #22
John Bickar CreditAttribution: John Bickar commentedPatch fails for me on admin_menu.info
But I got it working by manually applying the changes (woo hoo!). Some notes:
Thanks for working on this! I'm just working on porting my site over to D6 and this will make it much easier!
Comment #23
steve.colson CreditAttribution: steve.colson commentedSubscribing. Any issues with the latest patch?
Comment #24
wwwoliondorcom CreditAttribution: wwwoliondorcom commentedhello,
As many others here I really miss the admin menu on drupal 6, so should we ALL send few dollars with Paypal to expect getting it faster?
As this is the only thread about admin menu that I read I don't know so much about the future development of this menu for Drupal 6, so, sorry if I wrote something that had been discussed before.
Thanks anyway for this great module!
Comment #25
sunwwwoliondorcom, yes, that is probably the best idea to push this port. I've created a ChipIn for Drupal Administration Menu Port for D6, so everyone interested in this port can throw a few bucks in. I did not add a limit, however, due to the already identified issues regarding the new menu system this port will require a lot of hard work, maybe even a complete rewrite of the module.
Comment #26
dennys CreditAttribution: dennys commentedI use this patch for 2.4 but got the error message:
And I also use this patch for 2008-Mar-20 version but still got error.
Comment #27
sunAs written above, patch in #21 is against latest code in HEAD (in CVS).
Comment #28
dennys CreditAttribution: dennys commentedSorry, I just download the CVS but still get error.
Comment #29
sunRe-rolled patch against latest HEAD. Applies on a fresh CVS checkout only.
Comment #30
mrtoner CreditAttribution: mrtoner commentedApplied patch in#29 to the latest CVS versions (cherry-picked from the repository since I don't know what I'm doing in there). No errors. The only issue I'm seeing at this point is that the favicon, logged users, and logout items are not displaying. Otherwise, the admin menus are working fine.
Comment #31
dennys CreditAttribution: dennys commentedI applied patch #29 too, I think it's ok, thanks. How about release a snapshot for D6 and more people can help to test.
Comment #32
sunSorry guys, it seems you got it wrong - please read comments #1 and #25 again. This port is far from complete, and if above patch partially works for you, that is caused by coincidence. In case you intend to use this on a production site, I do not recommend this, because there will be no support if something goes wrong.
Comment #33
mrtoner CreditAttribution: mrtoner commentedDaniel, my comment wasn't meant to indicate that the patched module works perfectly; it was meant to offset the prior 7 comments indicating problems with the patch. You'll be happy to know that beyond the issues I mentioned, some of the menus do not have all the submenus I expect. :-)
Still, it's worth using on my 6.x sites because I'm still more productive.
Comment #34
eigentor CreditAttribution: eigentor commentedMaybe we should chip in some money http://unleashedmind.chipin.com/drupal-administration-menu-port-for-d6 to give Daniel a strong incentive to investigate the depth of the new menu system ;) As seen from his comments, patches simply won't do. I fully understand that a maybe complete rewrite exceeds what he can do in his spare time.
Comment #35
robertgarrigos CreditAttribution: robertgarrigos commentedDefinitely! I've added my few hundred cents to ti ;-)
Comment #36
PanchoJust a reroll of patch #29 against HEAD (besides some fuzz, one hunk failed).
The port is still somewhat feature incomplete, but the most important things seem to work well enough for testing and maybe non-production usage.
Comment #37
jamm CreditAttribution: jamm commentedsubscribing
Comment #38
ardee-1 CreditAttribution: ardee-1 commentedSubscribing.
Comment #39
rapsli CreditAttribution: rapsli commentedpatch works for me... only basic functions, but it makes working so much nicer. Maybe we can release a first alpha version for D6 as I believe there will be lots of testers and a lot of users will appreciate it (even though some of the nice functionality is not in yet)
Comment #40
WorldFallz CreditAttribution: WorldFallz commented+1 to the patch in #36. Without the bells and whistles of the added local tasks, but still far better than any other admin option out there.
Comment #41
IceCreamYou CreditAttribution: IceCreamYou commentedtracking
Comment #42
alliax CreditAttribution: alliax commentedyes, if someone could post somewhere on the net the latest dev module file with the patch applied on it, it will be so fine. Anything like admin menu on D6 will be better than nothing or than this other module, I forgot the name but it was not good.
Comment #43
sunLast patch by Pancho (#36) was incomplete.
Re-rolled patch in #29 against latest HEAD. Applies on a fresh CVS checkout only.
I'll clean-up the existing code now and create a DRUPAL-6--1 branch, along with a development snapshot for 6.x. However, comment #32 still applies.
Comment #44
sunhttp://drupal.org/node/251566 should be available within the next 12 hours.
Comment #45
eigentor CreditAttribution: eigentor commenteddrupal.org/node/251566: access is denied to that node.
Comment #46
Pancho> Last patch by Pancho (#36) was incomplete.
Not a big thing, but I can't see why my patch should have been incomplete. It's been just slightly smaller, because Eclipse doesn't support the -p flag in diff.
Comment #47
sun@Pancho: Sorry, I was wrong. My diff tool was confused because the order of files in your patch was different.
Comment #48
alliax CreditAttribution: alliax commentedThank you very much for putting something for D6 even if not as useful as for D5, it's a timesaver anyway! Thanks.
Comment #49
joris.verschueren CreditAttribution: joris.verschueren commentedgreat that a D6 version is available! even if it hasn't got all functionalities, I feel much more at home on my website again!
Joris
Comment #50
sunsmk-ka and I spent some hours on this port this evening. Attached patch re-introduces base-functionality of Drupal Administration Menu we already had in Drupal 4.7.x. This means all local tasks in the administrative menu are back again. Additional improvements like direct access to create content items and all other add-ons are still missing.
Beware that your database user needs to have sufficient permissions to create temporary tables, and, this patch requires the MyISAM table engine currently.
This will hopefully change in an upcoming code-sprint. Currently, we are investigating the idea to store a mix of menu_router, menu_links, and hook_admin_menu() items in a new admin_menu table to eliminate the need for a temporary table.
Additionally, caching has been re-enabled. This will save you approx. 100ms on each page request.
While I really would like to get some feedback on this patch, I recommend to not use it unless you know how to revert your module to the last official release.
Comment #51
sunSince this port still needs much work and the previous ChipIn somehow ended too early:
http://unleashedmind.chipin.com/drupal-administration-menu-port-for-d6-2
Thanks to all who have contributed until now!
Comment #52
snufkin CreditAttribution: snufkin commentedsubscribe
Comment #53
sunThis patch is just for reference. Uses a permanent table holding a mixture of items from menu_router and menu_links. Still no hook_admin_menu items.
We will investigate a completely different way now.
DO NOT USE, unless you know how to manually remove a module.
Comment #54
dergachev CreditAttribution: dergachev commentedsubscribe
Comment #55
Gábor HojtsyI have actually been playing with this latest patch, and it seems to work quite right now (although admittedly not too nice a code, it resembles the menu API internals in complication :). Anyway, I was working on making this patch easier to use by applying schema API goodness (install and uninstall) as well as fixing the Cache API todo which was included.
Not sure what the completely different way would be, but knowing more about it could help us collaborate on bringing a full feature set for Drupal 6.
Comment #56
pwolanin CreditAttribution: pwolanin commentedok, I was discussing this with Gabor this morning, and it seems to me that you are all too stuck in the Drupal 5 way of doing things.
Attached is a demo module I wrote in the last couple hours that I think shows the essential features you need. You need to make a new links menu, and store your stuff there. For the moment at least, throw out all your caching stuff and just let the menu system do its thing.
Obviously, this needs fine tuning, and you may need to code in the exclusion or renaming of some links, as well as handling the wildcards. Note that you get all the local tasks (excluding the ones with wildcards at the moment).
Enable the block to see the menu.
Comment #57
pwolanin CreditAttribution: pwolanin commentedthis still has some cruft that i'm not done fixing, but it works well enough to show the principle.
Comment #58
pwolanin CreditAttribution: pwolanin commentedA little more progress- but for some reason the .info files are not found - I really don't understand why.
Comment #59
pwolanin CreditAttribution: pwolanin commentedfor some reason, the info files cannot be parsed during the shutdown phase. New patch deals with this problem.
a couple TODOs left, but it's pretty well along.
Comment #60
pwolanin CreditAttribution: pwolanin commentedDynamic user link and devel links seem to be working.
Comment #61
pwolanin CreditAttribution: pwolanin commentedIt could be a security hole to turn HTML on for all links. Let's just do it for those that need it.
Comment #62
pwolanin CreditAttribution: pwolanin commentedhere is a simpletest - needs to be committed in a tests subdirectory
Comment #63
Gábor HojtsyTested functionality of patch in #60:
- menu did not show up at all first time I enabled the module (worked after reload)
- user count placed in "favicon menu" not at the place it used to be in D5
- altered menus are missing (content management submit content links, drupal.org issue links)
Comment #64
Gábor HojtsyAlso looked at the code, and it looks pretty nice. There are some functions without phpdoc I am wondering whats their role exactly (like admin_menu_link_build()). Some more docs there will probably help.
Noticed, that calling of the admin_menu hook looks broken. That is supposed to let any more to add items however they want. It is called with the $info_files array, which is probably unintended. Then returned links are not saved with the same API as used above. Not sure what you envisioned here.
Comment #65
pwolanin CreditAttribution: pwolanin commentedok, here's a new patch - move the rebuilding to hook_footer, which should avoid a delay where new links weren't showing until the next page view.
Comment #66
pwolanin CreditAttribution: pwolanin commentedAdd links to create each content type.
Comment #67
pwolanin CreditAttribution: pwolanin commentedslightly improved test with added code comments.
Comment #68
pwolanin CreditAttribution: pwolanin commentedreworked test to use D7 API, per http://drupal.org/node/241156 which will simplify future updating
Comment #69
eigentor CreditAttribution: eigentor commentedHeyho, Dev Snapshot works for me. Great. As Admin Menu cannot be beaten as the top downloaded Module on drupalmodules.com, this will be good News to many.
Comment #70
Gábor HojtsyEigentor: the dev snapshot is a quick hack as acknowledged by sun above. The patch improves on it considerably, bringing back quite a few of the lost features you got used to in D5. So it would be great, if you could test this patch.
Comment #71
sunWhoa! This looks and works really nice already! Thanks, Peter! Thanks, acquia!
To make further development easier, I've committed the patch in #66 with (absolutely) minor changes to HEAD and DRUPAL-6--1 branches. Also committed test file in #68.
I think this port revealed two bugs in Drupal core:
due to a missing check in menu.inc:
Comment #72
pwolanin CreditAttribution: pwolanin commented@sun - that path naming in D6 is not a bug - it was a choice to avoid namespace problems. We did the same with some of the menu stuff. However, you are right that it makes some of this sort of thing a little more difficult.
The problem with menu.inc will be fixed by this patch that is already RTBC for D6: http://drupal.org/node/259483
Comment #73
pwolanin CreditAttribution: pwolanin commented@sun - you'll notice that the current code just strips out all those edit links that were in the wrong place. Probably the simplest thing to do is just add them back later in the right place. Maybe it would be easier to do if we kept track of them and then passed them along?
Comment #74
sunUgh. That sounds awkward. While we could alter those paths for admin menu to get all items displayed in the proper menus, I think that such kind of alterations can break pretty much, f.e. if another contrib module depends on
arg(2) == 'node-type' && arg(3)
to add JavaScript or whatever else to the page.Or, could we use that 'parent_path' property for links that you've already implemented in admin_menu_admin_menu() to alter their position in the menu without changing their paths?
Comment #75
pwolanin CreditAttribution: pwolanin commented@sun - here's a patch that just removes them for now. As far as I can tell, they don't appear in the D5 version of this module when CCK is enabled?
Comment #76
pwolanin CreditAttribution: pwolanin commentedRight - you (or another module even) could add them back, as I did in this last path for 'admin/by-module', as one option.
Comment #77
sunThe current development snapshot for D5 contains a bunch of improvements -- also those links I was referring to.
AFAIK, no other contrib module implements hook_admin_menu() yet. However, since CCK/Views/Panels belong to the de-facto standard modules, admin_menu provides enhanced support for them.
Attached patch tries to re-add content-type items below 'Content types'. Works for "Edit" items, but not for items of CCK.
Comment #78
pwolanin CreditAttribution: pwolanin commented@sun - you are doing something funny with the paths - you cannot make arbitrary changes to the paths, since corresponding router paths do not exist.
How about this - just construct them and put them under admin/content/types/list
Comment #79
sunHm... this does not respect access permissions for the current user. Both node/add/* and admin/content/node-type/* links are displayed for content-types the user is not allowed to create/edit.
Attached patch tries to circumvent this for edit content-type items by checking for a deleted item. However, adding them below "List" is unnecessary deep. The menu should really look like:
Content management
Content management » Content types
Content management » Content types » ...
Content management » Content types » Edit <content-type>
Content management » Content types » Edit <content-type> » Manage fields
I've basically figured out how to accomplish this structure now. But some issues remain:
- node/add* as well as admin/content/node-type/* links are not checked for sufficient access permissions.
- Content type names in title arguments for Edit <content-type> items are not injected.
I still didn't understand yet at which time title callbacks and access callbacks for links are actually executed.
Comment #80
pwolanin CreditAttribution: pwolanin commented@sun the access checking seems ok for me for node/add/* - are you sure you are not looking with an account with administer nodes permission?
Comment #81
pwolanin CreditAttribution: pwolanin commented@sun, this looks wrong in your patch:
Only links under admin/* might ever appear in $deleted.
And you cannot define a title callback for links - title callbacks are a bit funky. Basically they are meant for setting the page title for callbacks and such.
Comment #82
sunHrm... I also added
Comment #83
pwolanin CreditAttribution: pwolanin commentedok, this patch gets the logout link and type names in place. CCK yet to come.
Comment #84
pwolanin CreditAttribution: pwolanin commentedok, I think this handles the CCK-type links in a pretty general way that should also extend to other modules that add paths under admin/content/node-types
I understand now why you were deleting and re-adding the node/add paths - I have not put that logic back at this point, but it's fine if you think it's needed.
Comment #85
sun#84 has been committed.
Attached patch implements menu links below node/add* dynamically, so we finally support any menu items below node/add. This is needed for modules like Advertisement, eCommerce, Ubercart, aso. that register items like node/add/ad/image.
As outlined by pwolanin in IRC, the last remaining issue is to fork http://api.drupal.org/api/function/_menu_navigation_links_rebuild/6 into admin_menu_link_save() to prevent neverending increment of mlids in {menu_links}, each time the cache is cleared or a module is (un)installed.
Comment #86
sun#85 has been committed.
@pwolanin: Is it possible that it's that simple?
Based on a quick test, there were no new mlids created after clearing the cache. After enabling a module, only a small amount of new mlids have been created. After disabling and re-enabling some modules, the amount of new mlids increased, but still wasn't as much as during each clear-cache previously (approx. 200).
I know that it's missing the part that checks for stale menu links, which no longer have a corresponding menu_router entry. However, as modules can be enabled/disabled/installed/uninstalled at any time, that might not be needed at all.
Query used to check for new mlids:
Comment #87
pwolanin CreditAttribution: pwolanin commented@sun - yes, it should be about that simple - the main thing is to get the existing mlid and plid (and has_children)
As alternative would be do iterate over the existing item and assign to the new item any keys that don't exist.
Comment #88
pwolanin CreditAttribution: pwolanin commented@sun - I re-ordered some of the operations a little so that things should work if the parent_path is changed, plus added the possibility of '' for 'parent_item' so you can specify a top-level item.
Comment #89
sunWorks! Committed!
Peter, thank you very much for this awesome job! I think this port will have a noteworthy impact on the amount of users who think about upgrading to Drupal 6.x. (btw: to everyone who cares I recommend checking out http://drupal.org/project/upgrade_status)
I just found out that the new menu system in D6 finally takes care of the actual access permissions for items below #167020: Inaccessible site configuration menu items are displayed.
, which is (for the n'th time) outlined inFor a stable 6.x-1.0 release there are still some minor todos, primarily regarding re-implementing the improved features of admin_menu 5.x-2.5, and cleaning up the code and documentation. However, I'd like to move this to a new issue (#268373: Re-implement improvements from 5.x-2.5), since the preliminary goal of rewriting admin_menu for D6 has been accomplished now.
Comment #90
alliax CreditAttribution: alliax commentedThank you very much pwolanin and sun, I knew it was possible to do it, but we have to wait because too many module developpers still have most of their sites in drupal 5. Next time when drupal 7 will be out, I will be extra cautious and will not do the same mistake I made with drupal 6, thinking that I could start sites with D6 and that useful/essential modules will be updated in the following weeks, that's just not true, we're talking months!
Comment #91
Anonymous (not verified) CreditAttribution: Anonymous commentedAutomatically closed -- issue fixed for two weeks with no activity.