Navigation links PREV/NEXT on categories broken, category_menu_tree_all_data() needs cleanup
| Project: | Category |
| Version: | 6.x-2.0-rc1 |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | active |
Jump to:
I see yet another bug in the category_display module: The "prev" / "next" links are missing. I have category_menu enabled, menu structure properly re-generated (i.e. the TOC tree shows fine), links enabled (the "UP" link shows), but still no previous/next links.
I tried to fix it, but it's too complicated (and I don't need these links that badly, so I'm going to just disable them on my site), but still posting my findings here:
The problem is in the functions category_display_prev() and category_display_next(), where a subpart of menu tree is retrieved through category_display_get_flat_menu($book_link), and then searched for the given $book_link, to finally grab a neighboring link. This search yields nothing, because the tree is only *below* the given link, with that link itself never included.
So I thought that category_display_flat_menu() should be called with the container's menu item, in order to get the whole tree for that container (therefore including the $book_link item and its neighbors). The container's item is loaded easily with $container_menu_link = category_menu_map_load_by_nid($container->nid);, which I put into template_preprocess_category_display_navigation(), so that it runs only once, and the result is then passed to both the functions for prev/next links, and used for menu tree retrieving.
The links showed up, but unfortunately there were unrelated items, such as "add content" shown. Further research shows, that the menu tree retrieved by category_display_flat_menu() (probably coming directly from category_menu_tree_all_data()) contains all menu items below the depth of given item, not necessarily having that item as parent:
[navigation menu]
[articles container]
[category 1]
[category 2]
[category 3]
[category 3-1]
[add content]
[container]
[story]
[admin]
[build]
[block]When the menu tree was initially retrieved for [category 2], it contained [category 3-1], although it's not a child of [category 2], and it even contained [admin/build/block], but [category 2] itself wasn't there, so search failed. When I changed the call to retrieve the tree for [articles container], I got the entire [navigation menu] minus top level, and [add content / container] was my "prev" link (the diagram here is not exactly ordered like my menus).
I'm quite unsure, what exactly category_menu_tree_all_data() does, and what's it supposed to do, but clearly the data returned from that function don't match what's expected in category_display_prev() and category_display_next(). Either category_menu_tree_all_data() should be fixed so that it doesn't return unrelated items outside the sub-tree, or the resulting data need further filtering in category_display_prev() and category_display_next(). Most probably it needs to get container's menu item as input in both cases.
THE END of my analysis. I'm sorry, but I don't have time to go further with this.
#1
Additional thought: Shouldn't we check the menu items for category_menu module? That might or might not be necessary, but would instantly remove completely unrelated items like 'admin' and 'add content' links. I didn't look into the code yet, though. Just thinking out loud.
#2
Now I see, that category_menu_tree_all_data() is broken in a similar way, like menu names for new links were - there's some code implying menu names per-node-type, which I guess is some cruft from development as there's no such structure now. Obviously, that code is not fully up-to-date with rest of the package.
#3
Just writing down my other small doubts about the TOC trees - too small and untested on my part to guarantee separate issues, yet worth a look if someone happens to dig in that code. This follow-up is more or less a reminder to myself, where to look.
- When I was testing TOC trees on a container with generated menu items enabled (and built) for both categories and nodes, the node items got correctly pruned out of the tree. But I seem to remember, that the tiny arrow in front of category name was still there (indicating child items), although after the nodes got pruned, there were no children left. I didn't test this again (yet).
- The TOC tree seems to lack sorting by [menu-items] weights in its deeper levels (sometimes at least?). Again, this is just something I spotted during other testing, and didn't fully confirm/describe yet.
#4
Broader title for now...