save trackback output to $node->content[] instead of altering $node->body
fax8 - April 10, 2007 - 21:01
| Project: | TrackBack |
| Version: | 5.x-1.x-dev |
| Component: | Other |
| Category: | task |
| Priority: | critical |
| Assigned: | zorac |
| Status: | closed |
Description
Hi all,
the trackback module prints its contents directly at the bottom of $node->body.
A more general approach and default with drupal 5.x would be using $node->content[].
I'm not really experienced with this module ... however I created a patch for this...
Please review and test carefully.
Thanks for your work,
Fabio
| Attachment | Size |
|---|---|
| trackback_use_content_arr.patch | 1.7 KB |

#1
Hi Fabio,
IMO, 'view' is the operation to show the node content. However trackback isn't a content of node, but it relates to node like comment. Therefore the trackback module uses 'alter' for printing it.
#2
I wasn't able to use the $node->content[] from the alter operation but only on the view.
I think that the view operation should be used every time a module display informations on the node.
Using $node->content[] is good because helps a lot in theming.
#3
@zorac: This is a serious shortcoming of the module. In 4.7 I had to hack around this to achieve very basic theming.
I re-rolled the patch based on the latest in CVS, improving it to make each distinct element themeable. Default behavior is unaffected and has been tested. This is ready to commit.
#4
This patch should append received trackbacks to node body in RSS feeds. And received trackbacks URL will be deteted by the auto-detection.
#5
Why should they be appended to RSS feed? Comments are not, so I do not think this is the right behavior. As for the note on auto-detection, I am not sure I understand what the problem is.
Even if these points are valid, perhaps the best approach is the create an admin setting to choose the mode of trackback output.
#6
What problem does this patch solve?
I don't understand your problem. If you tell me it, I wish to think.
#7
The problem is that right now trackbacks are impossible to theme and they can look very ugly, depending on how the rest of the theme is organized. Moving the module output out of body / teaser fields gives control over output to themer, where it belongs.
Of course if you are not doing heavy theming this may not be a problem for you.
#8
theme_trackback, theme_trackbacks and theme_trackback_url functions can be overridden for theming.
You want to move the trackback list, don't you?
#9
@dkruglyak: Now the output of trackback module is saved into $node->trackback array. Does this change solve your problem?
#10
As I look at your latest code (Revision 1.64.2.15), this is getting close but not really there. The problem is explicit modification of $node->body, which is still present in your code:
$node->body .= implode('', $node->trackback);This really negates any gains for theming, since the module outputs trackback markup before theme can move it around. The whole point of adding $node->content in 5.0 API was to serve as a repository of these fragments written before / after nodes. See details in 5.0 module upgrade guide: http://drupal.org/node/64279#node-view
In 4.7 I did a hack by creating a similar $node->trackback array and used the theme (not trackback.module) to output it. However the advantage of $node->content in 5.0 is that is it handled very well by default - even if theme does nothing.
#11
I'll just throw in my two cents that I would like to be able to move trackbacks around. I think it would be great to have a trackbacks block for a node, but the feature request here is a good start.
#12
Re-rolled my patch against the latest. Real control over theming requires changing to 'view' hook and using proper content[] approach.
Tested and works just fine. Please commit!
#13
zorac - Can you please review the "view" action in the hook_nodeapi() documentation? That describes the proper way to change node output. It should be done through $node->content[], not by modifying $node->body.
http://api.drupal.org/api/function/hook_nodeapi/5
Attached is a respin of the patch that fixes this problem, applied against 5.x-1.4
#14
chip: Thanks for updating the patch!
zorac: This is commit-worthy and bug-free. Please get the patch into CVS!
#15
@chip: you should probably use the -u option to the diff command to generate a little bit more readable diff output (see drupal handbook for detailed info)
#16
This patch has a problem to send trackbacks to the sender page of recieved trackbacks by auto-detection.
#17
@zorac - could you explain what the problem is and how *exactly* to reproduce it? have you tried the patch?
To those looking for a more readable patch file, I am attaching it now.
#18
+1
@zorac: The patch does not seem to change functionality of TrackBack module; it's just moving the module output into a proper location. Your past follow-ups are very incomprehensible. Operation 'view' is definitely the right way to go, but you seem to have issues with that. Please elaborate.
#19
Just commit it already!
There are no problems here. Please read up on hook documentation if you do not understand how $node->content[] works.
#20
Please look at line 677 of trackback.module.
#21
That's a no-brainer. Just use node_view in trackback_urls_via_nodebody function. Look at that function's documentation to see how it invokes everything necessary, including hooks and theming: http://api.drupal.org/api/function/node_view/5
Updated patch attached.
#22
@dkruglyak: I seem that you don't understand the behavior of trackback_urls_via_nodebody function.
By this patch, it sends unexpected trackback pings when trackback_urls_via_nodebody function works with a node that has received trackbacks.
#23
OK, now I think I understand what you are trying to do with trackback sending.
We need to build content[] array without any of that trackback stuff you originally appended with your 'alter' hook. I made a very simple change that removes trackback-specific parts of content[] array before parsing for URLs.
I verified that now parsable text is identical to the one before the move to 'view' hook.
Now this should be ready to commit.
#24
Okay, I'm floored ;-)
I understand your idea and I will work for this task to fix other little problems.
#25
Why floored? Do you see any other problems with this solution?
I verified that the text generated for URL detection is the same as in your original code. This is ready to go - just give it a try.
#26
#27
Automatically closed -- issue fixed for two weeks with no activity.