WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 5 years ago

#3884 closed defect (bug) (fixed)

Refactor the_content_rss() to the_content_feed()

Reported by: jeroyang Owned by: Viper007Bond
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.1.1
Component: Feeds Keywords: has-patch
Focuses: Cc:

Description (last modified by westi)

Plugins using the filter "the_content_rss" are never applied in the RSS2 and Atom feeds, and still not working in the <content:encoded> tag of the RDF feed.

Line 45 in wp-rss2.php should be:

<content:encoded>
<![CDATA[<?php the_content_rss('', 0, '',, 2) ?>]]>
</content:encoded>

Line 39 in wp-atom.php should be:

<content type="<?php bloginfo('html_type'); ?>" mode="escaped" 
xml:base="<?php permalink_single_rss() ?>">
<![CDATA[<?php the_content_rss('', 0, '',, 2) ?>]]>
</content>

Line 52 in wp-rdf.php should be:

<content:encoded>
<![CDATA[<?php the_content_rss('', 0, '',, 2) ?>]]>
</content:encoded>

or something like that...

Attachments (3)

patch.diff (2.0 KB) - added by joostdevalk 7 years ago.
Patch.
3884.patch (10.4 KB) - added by Viper007Bond 5 years ago.
3884.diff (10.8 KB) - added by ryan 5 years ago.

Download all attachments as: .zip

Change History (35)

comment:1 westi7 years ago

  • Description modified (diff)
  • Milestone set to 2.2
  • Summary changed from the_content_rss() is used correctly in wp-rdf.php, wp-rss2.php and wp-atom.php to the_content_rss() is used incorrectly in wp-rdf.php, wp-rss2.php and wp-atom.php

comment:2 in reply to: ↑ description jeroyang7 years ago

Line 45 in wp-rss2.php should be:

<content:encoded>
<![CDATA[<?php the_content_rss('', 0, '',0 , 3) ?>]]>
</content:encoded>

Line 39 in wp-atom.php should be:

<content type="<?php bloginfo('html_type'); ?>" mode="escaped" 
xml:base="<?php permalink_single_rss() ?>">
<![CDATA[<?php the_content_rss('', 0, '',0 , 3) ?>]]>
</content>

Line 52 in wp-rdf.php should be:

<content:encoded>
<![CDATA[<?php the_content_rss('', 0, '',0 , 3) ?>]]>
</content:encoded>

by the way, the_content_rss() should always apply "the_content" filter first and then "the_content_rss" filter to keep almost all plugins involving content filter works on feeds.

On the other hand, to apply "the_content_rss" filter only in the_content_rss() is logically correct but we have to fix hundred of plugins to make wanted output.

comment:3 Otto427 years ago

the_content_rss() is doing some weird things that should probably be fixed before making this change. It seems to change links into references, while putting those links at the bottom of the post. It's just unusual and I don't think we should switch to using it until we know why it's doing that.

comment:4 jhodgdon7 years ago

Note that the "weird things" are only happening in the_content_rss under some circumstances. The latest suggested fix sets the last ($encode_html) input argument to 3, and the 4th ($cut) argument to 0, which basically circumvents all the weirdness.

With those inputs, the_content_rss function is basically just doing get_the_content (which is unfiltered), then filtering using "the_content_rss" filter, then making sure any ]]> strings are escaped with entities.

The above note about not filtering with "the_content" seems on the mark to me. But my concern would be that adding "the_content" as a filter to the_content_rss might break something else. I suppose it could be added only if $encode_html had the value of 3 (previously unused).

One other note: the suggested fixes above will need to be added to different files on different lines. As of trunk SVN 4984, all the content of the 3 files mentioned above has been moved into corresponding files in wp-includes:

wp-includes/feed-rss2.php wp-includes/feed-atom.php wp-includes/feed-rdf.php

Thoughts?

comment:5 foolswisdom7 years ago

  • Milestone changed from 2.2 to 2.4

comment:6 follow-up: joostdevalk7 years ago

  • Cc joostdevalk added
  • Owner changed from anonymous to joostdevalk
  • Summary changed from the_content_rss() is used incorrectly in wp-rdf.php, wp-rss2.php and wp-atom.php to the_content_rss() is used incorrectly in non RSS feeds.

This fix is heavily needed as there is no way to change the content of an rss2 feed at the moment other than hacking the source...

joostdevalk7 years ago

Patch.

comment:7 joostdevalk7 years ago

  • Keywords needs-testing has-patch added

comment:8 in reply to: ↑ 6 Otto427 years ago

Replying to joostdevalk:

This fix is heavily needed as there is no way to change the content of an rss2 feed at the moment other than hacking the source...

Wha? Sure there is. Add a filter to the_content and use an if (is_feed()) type of statement.

comment:9 joostdevalk7 years ago

Yeah found that out too, not the way you'd want it though...

comment:10 jeroyang7 years ago

To Qtto42

Wow! That is really a simple way!!

I have solved this problem in my plugin "sataytube",
I tried to overwrite the wp-rdf.php, wp-rss2.php and wp-atom.php by adding actions through the hooks of atom_head, rss2_head, and rdf_head. It's too fat.

comment:11 santosj6 years ago

  • Component changed from General to Template

comment:12 fuggi6 years ago

I also thing that the rss2 feed should use the_content_rss function and not the_content, the attatched diff looks fine for me!

the_content filters should not be applied to the the_content_rss function, that does not make sense and the is_feed() check is only a workarround for that. Plugin developers should have possibility to add filters to get_the_content if they want to have their filter applied all the time or they have to add their filter twice. This ticket goes into the same direction (filter with the excerpt and post): http://trac.wordpress.org/ticket/4679

comment:13 thee176 years ago

  • Milestone changed from 2.5 to 2.6

comment:14 scribu6 years ago

  • Milestone changed from 2.9 to 2.6.1

comment:15 westi6 years ago

  • Milestone changed from 2.6.1 to 2.9

Setting back to 2.9

comment:16 dougal6 years ago

  • Cc dougal westi added

So, is there any reasoning behind the inconsistent usage of the_content_rss()? It's used in some feed types (rdf and rss) but not others (rss2 and atom). What's up with that?

I ran across this today when I needed a plugin to modify feed content differently than regular page views. Ended up using is_feed() in my the_content filter but that just seems clunky to me.

Either we can rely on the_content_rss, or we can't. Which is it? The function doesn't need to do anything fancy. Just get_the_content(), and apply any 'the_content_rss' filters to it. All those parameters seem like overkill to me. Refactor those into their own filter functions. It would break backwards compatibility, but I figure this function is broken already anyways.

Oh, and it should probably be deprecated in favor of renaming it to 'the_content_feed', anyways, right? That could fix the backwards compatibility issue.

get_the_content_feed()
the_content_feed()
the_content_rss() -- deprecate

comment:17 Denis-de-Bernardy5 years ago

Cross-referencing: #8706, #4967

comment:19 Denis-de-Bernardy5 years ago

  • Component changed from Template to Feeds

comment:20 follow-up: ryan5 years ago

dougal's suggestion of moving to the_content_feed sounds good.

comment:21 scribu5 years ago

  • Keywords needs-patch added; needs-testing has-patch removed
  • Summary changed from the_content_rss() is used incorrectly in non RSS feeds. to Refactor the_content_rss() to the_content_feed()

comment:22 in reply to: ↑ 20 westi5 years ago

Replying to ryan:

dougal's suggestion of moving to the_content_feed sounds good.

+1

comment:23 hakre5 years ago

Related: #3260

comment:24 ryan5 years ago

  • Owner changed from joostdevalk to ryan
  • Status changed from new to assigned

comment:25 Viper007Bond5 years ago

  • Owner changed from ryan to Viper007Bond

Patch is in the works per lloyd's request.

Viper007Bond5 years ago

comment:26 Viper007Bond5 years ago

Attached patch should output identical content compared to pre-patch, except for two differences:

  1. the_content_feed filter is run on the content before it's outputted (all feeds).
  2. Shortcodes are now stripped from the RSS1 and RDF feeds (previously they were left raw and that's just ugly).

comment:27 Viper007Bond5 years ago

  • Keywords has-patch added; needs-patch removed

I tested this patch by saving the before and after HTML of each feed and doing a diff on them BTW, so I'm pretty confident this patch is good to go.

ryan5 years ago

comment:28 ryan5 years ago

Looks like we both worked on the same thing. FWIW, here's my patch.

  • Deprecates the_content_rss()
  • Introduces the_content_feed() and get_the_content_feed() which are just wrappers around get_the_content() that call an extra filter
  • Converts places that called the_content_rss() with an excerpt length to the_excerpt_rss()
  • Gets rid of the rss_use_excerpt conditional in the RSS feed. Both conditions were doing pretty much the same thing except one used CDATA and the excerpt lengths were slightly different
  • Removes the rss_excerpt_length option
  • Uses the_content_feed() where the_content() was previously used

comment:29 ryan5 years ago

I don't think we need to bother with having "short" support in the_content_feed(). the_excerpt_rss() seems like the right thing to use for that scenario. Plus, it's only done in rdf and rss, which people want to rip out anyway.

I don't think your patch does the replace for ']]>'. I kept it since the_content() does it. We're calling get_the_content(), which doesn't do it for some reason, so we have to add the filter on the_content and the replace on ']]>' back in.

comment:30 Viper007Bond5 years ago

I considered using the_excerpt_rss(), but didn't want to change current behavior. If one were to define a custom excerpt, that would display with your patch (up to any length) and it wouldn't in mine or the current. I'm not familiar enough with the RSS1/RDF spec to know if that's okay or not (i.e. why it was length limited in the first place). Then again (as you said), should we really care about RSS1/RDF?

]]> was an oversight on my part. I always forget that the_content() does extra things over get_the_content().

I do like what feed it is being passed to the filter.

comment:31 Viper007Bond5 years ago

Oh, one other thing:

You left out the first parameter on the_content_feed() for the RSS2 feed. It shouldn't be assumed that the RSS2 feed is the default feed.

comment:32 ryan5 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.