Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
็Symptoms:
- $picture and $submitted variables in node template of a forum are not set.
Cause:
- $node->type in phptemplate_node(phptemplate.engine) and $type in theme_get_settings(theme.inc) are different.
Fix:
- change 'name' in forum_node_info function from 'forum topic' to 'forum'. (forum.module)
Comment | File | Size | Author |
---|---|---|---|
#19 | theme_node_type_47.txt | 529 bytes | Heine |
#18 | theme_node_type_HEAD.txt | 518 bytes | Heine |
#11 | test.png | 30.53 KB | xtony |
#10 | phptemplate.engine_15.patch | 659 bytes | xtony |
#5 | drupal4.7.2.patch | 1.01 KB | xtony |
Comments
Comment #1
webchickPlease roll a proper patch. Details at HOWTO: Create patches.
Comment #2
xtony CreditAttribution: xtony commentedThis is a follow up patch..
Comment #3
Dries CreditAttribution: Dries commentedI think 'forum topic' is more accurate than 'forum'. The latter is a collection of forum topics. Can't we fix the template system instead?
Comment #4
beginner CreditAttribution: beginner commentedFirst, this is hardly critical.
Then, what is broken exactly?
Here is an example of a forum node with the user picture:
http://www.reuniting.info/node/386
I recently set this very similar issue as fixed:
http://drupal.org/node/45070
What am I missing?
If something is really broken, cvs should be patched first and the fix backported.
Comment #5
xtony CreditAttribution: xtony commentedThe problem is due to the $op arguement of _node_names function (node.module) is not well defined and there is conflict in 'name' and 'list' operation. In the follow up patch, I added the operation 'base_list' which lists 'base' variable of all nodes instead of 'name'.
In the patch, I use node's 'base' for its 'type' variable instead of 'name'.
Comment #6
beginner CreditAttribution: beginner commentedsetting 'critical' only for a missing picture will not get your patch committed faster ;)
Again, how to reproduce the error? I don't see any problems in my sites.
Comment #7
webchickAlso, if this is an API change, it'll only get into CVS, not 4.7 -- 4.7 is stable and will take no API changes unless they are critical security fixes.
I second beginner's comments. Please provide details (with source code if possible) of how you feel this is a bug, and stop marking issues critical that are not.
Comment #8
Dries CreditAttribution: Dries commentedThe problem is due to the $op arguement of _node_names function (node.module) is not well defined and there is conflict in 'name' and 'list' operation.
xtony, can you be more verbose about this? I'd like to understand the problem. Thanks, Tony!.
Comment #9
drummThis seems like a fairly deep change for just user pictures. I'm wary of side effects.
Toggling post information is a similar mechanism. Does that work?
Comment #10
xtony CreditAttribution: xtony commentedSorry, i'm new to drupal development. Please be patient with me :).
I think i found the real problem. It's in phptemplate.engine, line 246:
if (theme_get_setting('toggle_node_info_' . $node->type)) {
the function "theme_get_settings" should be called instead of "theme_get_setting".
I also attached the patch with this post too.
Please review the patch again and sorry for my misunderstanding.
Comment #11
xtony CreditAttribution: xtony commentedThis is a picture of the problem. perhaps, this could help reviewer.
Thank you.
Comment #12
drummtheme_get_settings() returns an associative array of settings. I don't think thats what we would want to use as the condition for an if statement.
The documentation for theme_get_setting() makes it seem like the right thing. Maybe the problem is that it seems that the post information ("Submitted by ...") needs to be showing for the picture to show?
Comment #13
xtony CreditAttribution: xtony commented'theme_get_setting' does nothing to 'toggle_node_info_' variable but 'theme_get_settings' does. I've checked these two functions's code in theme.inc.
Comment #14
xtony CreditAttribution: xtony commentedOk. I checked the code again. in the 'theme_get_setting' function, the 'toggle_node_info_forum topic' is set. but '$node->type' in "if (theme_get_setting('toggle_node_info_' . $node->type))" is "forum" instead of "forum topic".
The souce of problem is the function which calls "phptemplate_node" in phptemplate.engine.
This is my problem :(... I dont know how drupal's theme engine works...
Can someone tell me how "phptemplate_node" is called?
Any suggestion please.
Comment #15
Heine CreditAttribution: Heine commentedFrom this code in phptemplate.engine
it is clear that the current settings needed to get user pictures in forum nodes is to enable
1) Display post information on forum topic
2) User pictures in posts
If you want user pictures in posts to be sufficient take the $variables['picture'] = ... out of the if (theme_get_setting('toggle_node_info_' . $node->type)) clause.
Comment #16
xtony CreditAttribution: xtony commentedYes, that's an easy fix. But should this be considered as a bug? should it be patched?
I digged into deep and found that $node variable is loaded from database but "theme_get_setting" uses the name of a node. for example, in the database $node->type of forum is "forum" but the "theme_get_setting" uses "module_invoke_all('node_info')" function to load node's type variable which is "forum topic". I have to take more time for this. But so sleepy now. The problem must be how a forum's node is saved to the database.
Any idea?
Comment #17
Heine CreditAttribution: Heine commentedOke, I see what you mean (though the setting toggles 'toggle_node_info_forum'). Since I was already mucking with the theme_settings code, I'll look into it further.
Comment #18
Heine CreditAttribution: Heine commentedThe reason why everything works is because of system_theme_settings (system.module: 1043)
where toggle_node_info_forum is defined in the theme settings form and saved upon submit.
This is of course inconsistent. Attached patch modifies theme_get_settings to use the node $key as node type.
Comment #19
Heine CreditAttribution: Heine commentedAnd a patch against a fresh checkout of the 4.7 branch. The original title doesn't reflect the issue accurately; user pictures display fine in (forum) nodes.
Comment #20
xtony CreditAttribution: xtony commentedsorry about the title :).
And thank you for the patch.
Comment #21
Dries CreditAttribution: Dries commentedLast patch looks good to me. Will commit unless someone objects.
Great collaboration folks! And xtony, welcome on board. ;)
Comment #22
drummCommitted to HEAD.
Comment #23
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedalso to 4.7.
Comment #24
(not verified) CreditAttribution: commented