PHP notice in forum.module if $term is not loaded
daouverson2 - March 7, 2008 - 19:13
| Project: | Drupal |
| Version: | 6.x-dev |
| Component: | forum.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
error:
notice: Trying to get property of non-object in /var/www/vhosts/drupal6/modules/forum/forum.module on line 473.
screenshot or error:
http://www.screencast.com/users/daouverson2/folders/Jing/media/f6437e48-...
#1
If I don't put image in a gallery, the error doesn't occur.
#2
This is a duplicate of #231337: conflict with Forum module?
#3
Forum tries to replace all taxonomy links with own path at forum_link_alter
But image gallery already replaced own links with image_gallery_term_path
So we need different way to replace links or just to check term for existence!
$tid = str_replace('taxonomy/term/', '', $link['href']);$vid = variable_get('forum_nav_vocabulary', '');
$term = taxonomy_get_term($tid);
// if ($term->vid == $vid) {
if ($term && $term->vid == $vid) {
#4
subscribing. where this fix should go? is it possible to post a patch file?
#5
i would not dare to change the status of this issue, but considering that two major and very popular modules like forum and image modules are in conflict, may i set to critical. change back if i am wrong.
#6
Patch applied - need testing!
#7
This patch makes no sense. Why does adding a check for $term fix anything?
The original code:
if ($term->vid == $vid) {
The modified code:
if ($term && $term->vid == $vid) {
if($term->vid == $vid) does the exact same check (and more) than if($term). The only case it makes a difference, is where both $term->vid and $vid are NULL. And this SHOULDN'T be happening. Image needs to take this into account, not the other way around.
Additionally, why are we patching core for a contributed ALPHA release module? The Image module needs to be fixed, plain and simple.
#8
Duplicate of http://drupal.org/node/231338.
#9
boydjd, take a closer look to the problem! Patch is fixing a notice caused by error in code
Remember, if you trying to check a property of object you SHOULD first to check the OBJECT.
So read [#3] this notice is caused by hook_term_path - forum SHOULD BE fixed not all other modules that use this hook!
#10
Come on, this is a PHP notice, not a critical error. Should use !empty() instead of plainly checking against the variable.
#11
Also retitling.
#12
I didn't find this issue (sorry...), so I posted a new one a while back: #335111: Forum causes notices with image_gallery
It has some more detailed analysis as well as a different patch — I don't think we should call taxonomy_get_term($tid) if $tid (the result of trying to remove 'taxonomy/term/' from the path) is not numeric, because if it isn't, we're outside of what we expect.
#13
Suppose patch from #13 still actual so let's check this
result of taxonomy_get_term possible only Object or False so why not to check 'is there result?' before check it's properties
<?phpfunction forum_link_alter(&$links, $node) {
foreach ($links as $module => $link) {
if (strstr($module, 'taxonomy_term')) {
// Link back to the forum and not the taxonomy term page. We'll only
// do this if the taxonomy term in question belongs to forums.
$tid = str_replace('taxonomy/term/', '', $link['href']);
$vid = variable_get('forum_nav_vocabulary', '');
$term = taxonomy_get_term($tid); //Here is $term set to Term-Object or False
if ($term->vid == $vid) {
$links[$module]['href'] = str_replace('taxonomy/term', 'forum', $link['href']);
}
}
}
}
?>
#14
Yes, the assumption is that
$tid = str_replace('taxonomy/term/', '', $link['href']);operates on something like taxonomy/term/NNN where NNN is a number (the $tid).
In the case of image_gallery.module, str_replace() operates on 'image/tid/NNN', returning the unchanged 'image/tid/NNN'. What's the point of calling taxonomy_get_term('image/tid/NNN')?
#15
Made obsolete by #197864-13: forum module uses hook_link_alter instead of taxonomy_term_path hook.