[development] hook_menu split is inevitable

Karoly Negyesi karoly at negyesi.net
Sun Jan 21 22:53:29 UTC 2007


Hi,

The current menu patch does work well. You see a navigation block and you  
can click links, the menu expands, you can visit pages and they load. All  
is well.

Try this: give administer comments permission to anonymous (no other admin  
perms) and log out . Observe the comments link in the navigation block on  
the home page in the Navigation block for Drupal 5. The new patch will not  
even consider admin/content/comment for display. To do so, it would need  
to access check every single visible item in the menu table. As the new  
system does access checking on run time via callbacks, this will lead to  
slowdowns. We could attack this with various caching strategies but that's  
not the solution.

The simple fact is that hook_menu need and hook_router should not use the  
same permission system. hook_menu should only do permissions, after  
http://drupal.org/node/73874#comment-116054 this patch is done it will be  
just a JOIN. This way, in a sense, hook_menu indeed can access check every  
single menu item. We will give the permissions a numeric ID and the JOIn  
can happen over menu_links, permissions_roles, user_roles -- a three table  
JOIN over numeric keys. Likely to be reasonably fast.

So, what will happen eventually is that you are displayed a tree of links  
that you have permission to see. When you click on an item you might end  
up with a 403. The 403 comes from hook_router which will be able to use  
access callback/arguments for access checking because we need to do this  
only once per page so it wil not be slow. Note that in Drupal 5 no access  
check is performed on custom items, so for example if you put a node into  
the menu that the user can't access then the user will see the link but  
clicking on it will give her a 403. Even if the user has no access content  
permission, this will happen with Drupal 5. The new menu system will at  
least check access content, if not node_access.

But this makes the split absolutely ineviatable.

There are other fun problems with visible links and router items mingled  
into one: if foo is not defnied, foo/bar is not visible but foo/bar/this  
is visible and again foo/bar/this/that is not visible then how should we  
sort these paths? For inheritance purposes, the order I listed them is a  
must, but visibility purposes foo/bar/this actually appears on the first  
level. This is another sure sign that menu and router structures are very  
much not the same.

I still propose to get the patch in as it stands. Before the navigation  
block was added back it was actually hook_router and it was pretty much  
without problems. Accept that the navigation block I added back yesterday  
is just a development tool and not the Real McCoy yet -- eventually the  
quirks above will be smoothed out. It's sheer impossible to do all changes  
at once.

Regards

NK


More information about the development mailing list