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

AttachmentSize
trackback_use_content_arr.patch1.7 KB

#1

zorac - April 12, 2007 - 10:41

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

fax8 - April 16, 2007 - 06:49

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

dkruglyak - June 27, 2007 - 23:05
Title:print informations on $node->content[] instead of altering $node->body» save trackback output to $node->content[] instead of altering $node->body
Category:feature request» bug report
Status:needs review» reviewed & tested by the community

@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.

AttachmentSize
trackback.module_7.patch 2.06 KB

#4

zorac - June 29, 2007 - 13:42
Status:reviewed & tested by the community» needs work

This patch should append received trackbacks to node body in RSS feeds. And received trackbacks URL will be deteted by the auto-detection.

#5

dkruglyak - June 29, 2007 - 20:48

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

zorac - June 30, 2007 - 17:53

What problem does this patch solve?
I don't understand your problem. If you tell me it, I wish to think.

#7

dkruglyak - June 30, 2007 - 23:07

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

zorac - July 4, 2007 - 09:21
Category:bug report» feature request

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

zorac - July 6, 2007 - 18:09

@dkruglyak: Now the output of trackback module is saved into $node->trackback array. Does this change solve your problem?

#10

dkruglyak - July 6, 2007 - 18:35

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

chellman - July 17, 2007 - 18:10

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

dkruglyak - July 29, 2007 - 05:45
Status:needs work» reviewed & tested by the community

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!

AttachmentSize
trackback.module_8.patch 1.64 KB

#13

chip - February 12, 2008 - 02:32

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

AttachmentSize
trackback.module_9.patch 2.07 KB

#14

dkruglyak - February 12, 2008 - 03:07

chip: Thanks for updating the patch!

zorac: This is commit-worthy and bug-free. Please get the patch into CVS!

#15

fax8 - February 13, 2008 - 11:48

@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

zorac - February 13, 2008 - 14:52
Status:reviewed & tested by the community» needs work

This patch has a problem to send trackbacks to the sender page of recieved trackbacks by auto-detection.

#17

dkruglyak - February 13, 2008 - 20:44
Status:needs work» needs review

@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.

AttachmentSize
trackback.patch 1.67 KB

#18

sun - March 2, 2008 - 04:02

+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

dkruglyak - March 2, 2008 - 23:55
Status:needs review» reviewed & tested by the community

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

zorac - March 5, 2008 - 13:56
Status:reviewed & tested by the community» needs work

Please look at line 677 of trackback.module.

#21

dkruglyak - March 5, 2008 - 17:05
Category:feature request» task
Status:needs work» reviewed & tested by the community

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.

AttachmentSize
trackback.patch 2.15 KB

#22

zorac - March 6, 2008 - 11:29
Status:reviewed & tested by the community» needs work

@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

dkruglyak - March 6, 2008 - 16:36
Status:needs work» reviewed & tested by the community

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.

AttachmentSize
trackback.patch 2.11 KB

#24

zorac - March 6, 2008 - 18:30
Priority:normal» critical
Assigned to:Anonymous» zorac
Status:reviewed & tested by the community» active

Okay, I'm floored ;-)
I understand your idea and I will work for this task to fix other little problems.

#25

dkruglyak - March 6, 2008 - 18:57

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

zorac - March 13, 2008 - 08:46
Status:active» fixed

#27

Anonymous (not verified) - March 27, 2008 - 08:51
Status:fixed» closed

Automatically closed -- issue fixed for two weeks with no activity.

 
 

Drupal is a registered trademark of Dries Buytaert.