Does not know context when hook_term_path is used
douggreen - April 21, 2007 - 20:30
| Project: | Taxonomy context |
| Version: | 5.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | postponed (maintainer needs more info) |
Description
When a module implements hook_term_path and returns an alternate path for the term, taxonomy context doesn't know about this, and thus fails to display context.
I'm not sure that there is a clean solution to this. In my case, I always replace 'taxonomy/term/$tid' with something like 'mymodule/term/$tid', in which case it is possible to figure out the context. But since hook_term_path has no such restriction, I don't know that there is a general solution to this. My recommendation is to remove the case for 'taxonomy' and use the default for this. Thus, if the implementor of hook_term_path sticks to this naming convention, taxonomy_context will still work.
Patch is attached.
| Attachment | Size |
|---|---|
| term_path_0.patch | 947 bytes |
#1
Attached is a more extensible solution that implements a new hook_taxonomy_context. This allows you to create your own hook_taxonomy_context. The following is an example of a hook_taxonomy_context that gets the tid from a views exposed filter:
function psn_taxonomy_context() {
global $_psn_tid;
if (!isset($_psn_tid)) {
if (is_numeric($tid = arg(2))) {
$_psn_tid = $tid;
}
elseif ($_GET['q'] == 'search/all') {
$args = array();
_psn_parse_args($args, $_GET['filter2']); // core issues
_psn_parse_args($args, $_GET['filter3']); // states
$_psn_tid = implode('+', $args); // OR is '+', AND is ','
}
}
return $_psn_tid;
}
function _psn_parse_args(&$return, $append) {
if (is_array($append)) {
foreach ($append as $arg) {
if (is_numeric($arg)) {
$return[] = $arg;
}
}
}
elseif (is_numeric($append)) {
$return[] = $append;
}
}
#2
Thanks for looking into this issue.
I wonder if it's feasible to determine the logic of a
hook_term_path()implementation and go with that. Something like:<?phpforeach (module_implements('term_path') as $module) {
$term = (object) array('tid' => 1);
$path = module_invoke($module, 'term_path', $term);
$arg = explode('/', $path);
// then somehow determine what should be used for context...
}
?>
Messy though.
#3
I kinda like the proposed hook_taxonomy_context solution. If someone implements hook_term_path (which I think will mostly/only occur in custom project related modules), and if they want to use taxonomy_context, they must also implement a hook_taxonomy_context. Does the call to module_invoke_all('taxonomy_context') add any real overhead that I'm not aware of?
#4
Okay, I'm convinced. The patch needs some work. Here are some suggestions and questions.
The first added 'default' means that all pages will be interpreted as having taxonomy. Instead we need to test if something appropriate is returned from the hook and only set the taxonomy context if so. Also, the implementation of the new hook needs to test for
arg(0) == 'taxonomy && arg(1) == 'term' && is_numeric(arg(2))before returning something.Finally, these two lines don't look quite right to me:
<?php+ $tid = module_invoke_all('taxonomy_context');
+ $tidcurrs = preg_split('/[+ ,]/', implode(',', $tid));
?>
I'd expect it to be instead
<?php+ $tid = module_invoke_all('taxonomy_context');
+ $tidcurrs = preg_split('/[+ ,]/', implode(',', $tid[0]));
?>
That is, we only want the first of the returned array of tid strings (each of which may be a comma-separated array). Does that sound right? If not, pls explain what I've missed.
Want to do up a new version? Once we've figured out these details you can go ahead and apply it.
#5
I may not have my site set up correctly, but it looks to me like this is corrected in Version 2.