In #152493: Minimalistic approach to node rendering styles for D6 we added build modes for nodes -- including NODE_BUILD_SEARCH_INDEX, NODE_BUILD_PREVIEW, and NODE_BUILD_RSS. This simplified the process of modules determining the context a given node was being displayed in, and allowed modules like CCK to give users control over how custom fields appear in different presentation contexts.

Unfortunately, the node_feed() function was never updated from the old Drupal 4.7 way of doing things: it loads the raw $node->body, runs it through filters, and then assumes other modules will hack or alter the $node->body in nodeapi op 'view'.

This is broken, defeats the purpose of the build mode, and because it runs counter to the documented purpose of the NODE_BUILD_RSS flag, it prevents CCK fields and other custom module additions from appearing in RSS feeds. The attached patch fixes the problem in both Drupal 6 and Drupal 7 -- without it, the Contemplate module is the only good option for site builders who need content-rich RSS feeds.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drewish’s picture

subscribing... if i get a minute i'd like to double check out test coverage for the feed building process.

eaton’s picture

Title: node_feed() ignores BUILD_MODE_RSS (?!?!) » node_feed() silently discards $node->content

Updating the title to better reflect the issue.

fuzzy_texan’s picture

Subscribing

eaton’s picture

Important note for those testing the patch: CCK caches node fields in fairly tricky ways sometimes. It's recommended that you both truncate your cache_content table and ensure that the fields on any nodes that youWANT showing up are set to appear in RSS fields on the CCK 'display' tab.

eaton’s picture

FileSize
1.2 KB
1.21 KB

Here's a fixed version of the patch -- it was erroneously using $node instead of $item when building the content, which worked in Views where the patch was originally built but did nothing here. Bad copy/paste!

Also moved the building inside of the $title check, so that feeds set to only display post titles won't trigger the extra work of building content.

webchick’s picture

Issue tags: +Needs tests

Since this has remained broken all the way to Drupal *6.9*, we really need some test coverage for this piece of code.

eaton’s picture

FileSize
4.98 KB

webchick, attached is a version of the D7 patch with several additions:

  1. PHPDoc for hook_nodeapi_rss_item clarifies that it is the correct place to add XML elements to the RSS item, while hook_nodeapi_view should still be used to change the content of the node itself.
  2. A node_test.module has been added, following the example of menu_test.module. It's needed in order to implement the nodeapi hooks that flush this bug out.
  3. A NodeContentRSSTest simpletest has been added to node.test, ensuring that data added to $node->content by node_test.module appears properly in the RSS feed.

node_test module can probably be fleshed out with additional tests for other node related hooks to ensure we're not dropping the ball anywhere else, but this seemed like a particularly egregious violation of our node-hook best practices.

eaton’s picture

FileSize
5.03 KB

After input from webchick, improved phpdoc for the test and slightly clarified PHPDoc for hook_nodeapi_rss_item.

HedgeMage’s picture

Status: Needs review » Reviewed & tested by the community

Tested, works nicely, /me dances with joy

--HedgeMage

webchick’s picture

Version: 7.x-dev » 6.x-dev

I confirmed that the tests fail without the patch and pass with it, and conforms to the test guidelines. :) Thanks a lot, eaton!

I also tested the 6.x patch manually using the following methodology:

1. Create a content type with a field.
2. Create a couple nodes of that content type.
3. Go to rss.xml and notice that the additional field is not in the RSS feed.
4. Apply the patch.
5. Reload rss.xml and see that it now is.

Looks good, and fixes a long-standing bug. Moving back to Drupal 6.x for consideration. (see #5)

anders.fajerson’s picture

Tested the Drupal 6 patch. Applies and works as advertised. This would be great if it was fixed in Drupal 6.

eaton’s picture

One important note for the Drupal 6 version of this patch: If it is applied and people upgrade to the new version of Drupal, some WILL begin seeing new content in their RSS feeds that was not previously there. This is an unavoidable side effect of fixing the bug. As best as I can tell from my testing, the "worst case scenario" is that administrators, when upgrading, will need to explicitly check the CCK display tab for their custom content types, and ensure that non-public fields are properly hidden in the RSS subtab.

The Views module already works this way when it generates feeds, so the change will not come as a shock to anyone who has been using Views to output their RSS feeds.

drewish’s picture

Well using the Views -dev release. I don't think there's been a stable release with that change yet.

eaton’s picture

Ah, an excellent point drewish. Thanks for the clarification.

jockox3’s picture

Should this make my additional fields show up in the Atom feed generated by teh Atom module or would there be something they have to do in that module to make it work? It seems to be working fine in RSS but not in Atom and I like to offer my readers both!

Cheers,

Jock

drewish’s picture

jockox3, the Atom module would probably need to be updated to take advantage for this.

eaton’s picture

Ping on this issue; there are definitely ramifications to be considered, but this is a rather significant bug that's been plaguing us for a while; CCK took the brunt of the complaints and issues logged, because it's what people most wanted to appear in RSS feeds, but it really is a core bug...

Any thoughts of its odds of being included in a 6.x release?

drewish’s picture

I wanted to chime in that I think this is definitely a core bug that should be fixed before D7.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed
FileSize
1.71 KB

Committed this to D6, which includes a CHANGELOG.txt change, so that we will remember to include your suggestion in the release announcement as well.

Status: Fixed » Closed (fixed)
Issue tags: -Needs tests

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