Posted by beginner on July 16, 2008 at 2:43am
| Project: | Tagadelic |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (won't fix) |
Issue Summary
in hook_menu():
<?php
$items['tagadelic/chunk/%tagadelic_vocs'] = array(
'title' => 'Tags',
'page callback' => 'tagadelic_page_chunk',
'page arguments' => array(2),
'access callback' => 'user_access',
'access arguments' => array('access content'),
'type' => MENU_SUGGESTED_ITEM,
);
?>however, when visiting admin/build/menu-customize/navigation those suggested items do not show.
I am not sure how this is supposed to be done in D6.
Comments
#1
For comparison purposes, this is how it was done in D5:
<?phpforeach (taxonomy_get_vocabularies($type = NULL) as $vocabulary) {
$items[] = array(
'title' => $vocabulary->name,
'path' => "tagadelic/chunk/$vocabulary->vid",
'callback' => 'tagadelic_page_chunk',
'access' => user_access('access content'),
'type' => MENU_SUGGESTED_ITEM);
}
?>
#2
I had the same problem with directory.module.
The solution is to completely restore most of the D5 code which, in this case, is completely valid for D6.
Don't use the wildcard loader.
Here is the valid D6 code for directory.module:
<?php$vocabularies = taxonomy_get_vocabularies();
foreach ($vocabularies AS $vid => $vocabulary) {
$items['directory/vocabulary/' . $vocabulary->vid] = array(
'title' => t('Directory for !vocabulary-name', array('!vocabulary-name' => $vocabulary->name)),
'page callback' => 'directory_vocabulary_page',
'page arguments' => array(2),
'access arguments' => array('access content'),
'type' => MENU_SUGGESTED_ITEM,
);
}
?>
#3
Did you actually create a tagadelic_vocs_load() function? The menu system kinda needs that if you're going to declare a menu hook that way. Although you probably want vocabulary_load() instead (which may or may not exist yet; not sure off hand).
A foreach loop inside a D6 menu handler is 99% of the time the WRONG way to do it.
#4
I noticed this bug with my own module (directory.module). I knew that tagadelic has a similar menu structure and that's why I checked it out.
I was using core taxonomy_vocabulary_load() function (used with the wildcard). The code worked fine as a callback. The problem is that there is no menu item ready to be enabled, like it is supposed to do.
This is the code I had just after upgrading to D6:
<?php$items['directory/vocabulary/%taxonomy_vocabulary'] = array(
'title callback' => 'directory_vocabulary_title',
'title arguments' => array(2),
'page callback' => 'directory_vocabulary_page',
'page arguments' => array(2),
'access arguments' => array('access content'),
'type' => MENU_SUGGESTED_ITEM,
);
?>
As I said, it worked nice as a MENU_CALLBACK, but did not fulfill its function as MENU_SUGGESTED_ITEM.
The HEAD version of directory.module uses the code in #2, which works as intended.
So, if the foreach loop is wrong, how do I get MENU_SUGGESTED_ITEM without it?
tagadelic has exactly the same problem.
#5
subscribing
#6
Here is the official way to fix this particular bug.
Thanks to karoly for the feedback.
#7
Sorry but I didn't quite get what to do with MENU_SUGGESTED_ITEM and a wildcard loader here.
#8
Interesting. I did not test tagadelic on 6 myself, because I havo no 6 sites yet. It may have skipped past the people who reviewed the upgrades, though.
Lets solve this fast!
#9
Here is the patch.
It enabled menu items like in D5, but using menu_link_maintain() function.
Not sure it it a good solution so the menu item enabled by default.
But menu_link_maintain() does not include a mechanism to enable/disable a link.
#10
oops, forgot to change status.
#11
I am not very enthusiastic about the solution with menu_link_maintain(). AFAIK we should be able to fix this within the hook_menu very simple.
#12
hopefully this one can be explained to me. I'm upgrading a module, and a function in it is getting flagged for its use of the foreach loop, but i dont really understand whats happening in the function, so i'm not sure how to approach getting rid of the foreach.
function &ninjitsu_newsletter_get_issue_drop_down_data() {$issue_vocab_id = variable_get('ninjitsu_newsletter_issue_vid', '');
$vtree = taxonomy_get_tree($issue_vocab_id);
$drop_down_data = array();
foreach ($vtree as $issue) {
$nodes = taxonomy_select_nodes(array($issue->tid));
while ($nnode = db_fetch_object($nodes)) {
$nnode = node_load($nnode);
// only display issues that have nodes of the correct type
if ($nnode->type == variable_get('ninjitsu_newsletter_main_node_type', 'newsletter')) {
$drop_down_data[$issue->tid] = $issue->name . ' - ' . $nnode->title;
break;
}
}
}
return $drop_down_data;
}
#13
Closing "needs work" that has been open for a long time, without anyone working on it.
#14