WordPress.org

Make WordPress Core

Opened 9 years ago

Last modified 21 months ago

#10984 assigned defect (bug)

If content uses the nextpage tag then only the first page is shown in feeds

Reported by: simonwheatley Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 2.8.4
Component: Feeds Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

If content uses the nextpage tag then only the first page is shown in feeds if the "full text" option is selected in "Settings > Reading > Show Full Text (in feed)".

No links are displayed to read the full content and no indication is given in the feed that it isn't the full content.

I think the behaviour should be to ignore pagination in feeds which are set to "full text".

I have attached a patch which alters the behaviour of the_content() so that if it is used in the context of a feed it concatenates the pages to form the full content and returns that.

Attachments (9)

full text for feeds.diff (972 bytes) - added by simonwheatley 9 years ago.
Amends the_content to return full content in a feed context
full text for feeds.2.diff (839 bytes) - added by simonwheatley 9 years ago.
Amended patch to use $post->content, rather than concantenating the $pages (which sometimes drags in unwanted content)
10984.diff (1.8 KB) - added by westi 8 years ago.
Alternative patch - add support for get_the_content ignoring pagination.
10984.1.diff (2.0 KB) - added by Latz 5 years ago.
10984.2.diff (2.5 KB) - added by jubstuff 2 years ago.
Patch refreshed
10984.3.diff (2.1 KB) - added by jubstuff 2 years ago.
10984.4.diff (4.3 KB) - added by swissspidy 2 years ago.
10984.5.diff (4.4 KB) - added by swissspidy 2 years ago.
10984.6.diff (5.9 KB) - added by swissspidy 2 years ago.

Download all attachments as: .zip

Change History (60)

@simonwheatley
9 years ago

Amends the_content to return full content in a feed context

#1 @scribu
9 years ago

  • Milestone changed from Unassigned to 2.9

#2 follow-ups: @westi
9 years ago

I wonder if we should show a link to the full post if we detect next page instead maybe?

@simonwheatley
9 years ago

Amended patch to use $post->content, rather than concantenating the $pages (which sometimes drags in unwanted content)

#3 in reply to: ↑ 2 @simonwheatley
9 years ago

Replying to westi:

I wonder if we should show a link to the full post if we detect next page instead maybe?

The setting says "full text". Therefore we should (in my opinion) provide full text... if we did provide a link instead then the option wording should be amended.

#4 in reply to: ↑ 2 @Denis-de-Bernardy
8 years ago

Replying to westi:

I wonder if we should show a link to the full post if we detect next page instead maybe?

why not use more...?

#5 @westi
8 years ago

I don't think get_the_content should have feed specific behaviour in it.

I think it may be better to add an argument to get_the_content - which makes it ignore pagination in the post and return everything so that the behaviour is explicit on that and not done based on is_feed() - I can see that there maybe a use for this in other places by plugins.

@westi
8 years ago

Alternative patch - add support for get_the_content ignoring pagination.

#6 @westi
8 years ago

  • Keywords early added
  • Milestone changed from 2.9 to 3.0

Moved to early 3.0 for now.

I think this change is the correct behaviour but would like it to get more review and I don't think it is a blocker as 2.8 had the same behaviour.

#7 @hakre
8 years ago

Nice argumentation, but I for me this looks like while the paged posts feature was integrated, feeds were forgotten.

Bump to commit this now into the 3.0 trunk sothat it can actually get more review now.

#8 @hakre
8 years ago

Looks like we have no interest on this one getting committed. Close as wontfix?

#9 @Denis-de-Bernardy
8 years ago

I like neither patch. If I use a nextpage tag, I'd expect the RSS feed to output a More... link to page 2.

#10 follow-up: @dd32
8 years ago

  • Keywords 2nd-opinion added

I think it should work as outlined above, The full text should be displayed.

Pagination of the post is used to seperate pages of the post, The feed is designed to display that post.

I would expect the feed content to display something like:

..Title..
Page 1 of 3
Page1 Content ..

Page 2 of 3
Page2 Content ..

Page 3 of 3
Page3 Content ..

#11 in reply to: ↑ 10 ; follow-up: @simonwheatley
8 years ago

Replying to hakre:

Looks like we have no interest on this one getting committed. Close as wontfix?

Ahem. I'm still interested, sorry for the silence. :)

Replying to Denis-de-Bernardy:

I like neither patch. If I use a nextpage tag, I'd expect the RSS feed to output a More... link to page 2.

I don't like this from a UX and publisher intent point of view. Feeds are about syndicating content not drawing people back into the site (no matter how much some publishers might want that); if you want to draw people back into the site then don't show full content in your feeds.

Replying to dd32:

I think it should work as outlined above, The full text should be displayed.

Pagination of the post is used to seperate pages of the post, The feed is designed to display that post.

I would expect the feed content to display something like:

..Title..
Page 1 of 3
Page1 Content ..

Page 2 of 3
Page2 Content ..

Page 3 of 3
Page3 Content ..

I feel this is diluting the content in the feed without adding significant advantage. I prefer showing the whole post (if show full text in feed is selected).

#12 in reply to: ↑ 11 @Denis-de-Bernardy
8 years ago

Replying to simonwheatley:

Replying to Denis-de-Bernardy:

> > I like neither patch. If I use a nextpage tag, I'd expect the RSS feed to output a More... link to page 2. > > I don't like this from a UX and publisher intent point of view. Feeds are about syndicating content not drawing people back into the site (no matter how much some publishers might want that); if you want to draw people back into the site then don't show full content in your feeds.

but in this case, don't use the nextpage tag either. its only point lies in getting more page views to display more ads.

imo we should add a more... link and that's it.

#13 follow-up: @dd32
8 years ago

  • Milestone changed from 3.0 to 3.1

This isnt a regression from a previous release, I'm moving to 3.1 because of that. We can decide a clear resolution then.

Personally, i'm leaning towards just displaying the entire page, full well knowing that theres a filter so people can achieve the current result easily.

#14 in reply to: ↑ 13 @valendesigns
8 years ago

  • Cc valendesigns added
  • Version changed from 2.8.4 to 2.9.2

Replying to dd32:

This isnt a regression from a previous release, I'm moving to 3.1 because of that. We can decide a clear resolution then.

Personally, i'm leaning towards just displaying the entire page, full well knowing that theres a filter so people can achieve the current result easily.

Is there a filter you can add to the functions.php that would show the full content if your viewing a feed?

#15 follow-up: @dd32
8 years ago

  • Version changed from 2.9.2 to 2.8.4

Is there a filter you can add to the functions.php that would show the full content if your viewing a feed?

There isnt a filter, But you could certainly do it with sub-10 lines of code i'd expect. I dont have a example handy however, or time to test it.

Please leave the version at whatever it was first reported at/first known at, It makes it easier to track when bugs were introduced.

#16 @valendesigns
8 years ago

Sorry, I will leave the version alone.

What do you mean by "sub-10"? Are you saying that it can't be done from the functions.php and you have to hack the core? I'm confused with that response. Thank you for being so quick with a reply though.

#17 follow-up: @nacin
8 years ago

No, not hacking core... < 10 lines in a plugin (or functions.php) to implement this.

#18 in reply to: ↑ 17 @valendesigns
8 years ago

Replying to nacin:

No, not hacking core... < 10 lines in a plugin (or functions.php) to implement this.

Is there any way you might help make that happen?

#19 in reply to: ↑ 15 ; follow-up: @simonwheatley
8 years ago

Replying to dd32:

Is there a filter you can add to the functions.php that would show the full content if your viewing a feed?

There isnt a filter, But you could certainly do it with sub-10 lines of code i'd expect. I dont have a example handy however, or time to test it.

This works for me:

<?php
/*
Plugin Name: Full Text Feeds
Plugin URI: http://simonwheatley.co.uk/wordpress/full-text-feeds
Description: Fixes a bug in WP's feeds whereby they are only served with the first page.
Version: 1.1
Author: Simon Wheatley
Author URI: http://simonwheatley.co.uk/
*/

function ftf_full_text_for_feeds( $content ) {
	if ( ! is_feed() )
		return $content;
	global $post;
	$content = $post->post_content;
	return $content;
}

add_filter( 'the_content', 'ftf_full_text_for_feeds', -100 );

?>

#20 in reply to: ↑ 19 @valendesigns
8 years ago

Replying to simonwheatley:

Replying to dd32:

Is there a filter you can add to the functions.php that would show the full content if your viewing a feed?

There isnt a filter, But you could certainly do it with sub-10 lines of code i'd expect. I dont have a example handy however, or time to test it.

This works for me:

Unfortunately, that plugin does not seem to work for me.

#21 @valendesigns
8 years ago

Never mind, that plugin works perfect. the only issue is that other plugins are applying/adding their own filters to 'the_content' and so I made the -100 even lower and got it working. Thanks for the help!!!

#22 @nacin
8 years ago

  • Milestone changed from Awaiting Triage to Future Release

#23 @mdgl
6 years ago

This bug seems to be languishing because of disagreements on the best way to fix the problem.

I suggest this issue is quite important, however as it may lead to data loss. People may be relying on syndicated feeds as a way of copying information around. Presently, multi-page posts are effectively destroyed when included in feeds, with no indication of the data loss whatsoever!

I vote that we increase the priority of this a bit and get something committed soon.

Regarding the last comment from Denis-de-Bernardy (comment:12), like the other respondents I believe that if the user has requested full content in feeds, that's what we should provide and not just some link to see the remainder of the content.

In my opinion, westi's approach to the fix is the most elegant and should be taken, although the patch needs a little update for 3.4.1. Since #12651 [3.0] there is no issue with using the 'pages' array as suggested by simonwheatley and this seems preferable to leaving the '<!--nextpage-->' comments within the content.

There is a minor issue with simonwheatley's plugin example (comment:19) as this will not respect posts protected with a password. Personally, I use a filter on 'the_content_feed' and check the 'multipage' global (and for a post password!) before resurrecting the missing content.

A more general point is that our whole handling of "the content" is starting to get rather complex and unwieldy with far too many things happening in filters and all over the place - one for another day perhaps!

#24 @Latz
5 years ago

This ticket seems to be a bit forgotten so I updated it to be patchable with v3.6.

Additionally: WordPress puts the_excerpt() in the description of the item if there is no explicit excerpt given by the user. Fixed that.

@Latz
5 years ago

#25 @Latz
5 years ago

  • Keywords 2nd-opinion early removed

#26 @jamesbe
3 years ago

I'd like to see this fixed. We are syndicating a feed but it doesn't work because of this.

#27 @jamesbe
3 years ago

Actually I'd like to note that I have applied Latz' fix and it works. Could we not just merge this into the main dev branch?

#28 @swissspidy
2 years ago

  • Keywords needs-patch good-first-bug added; has-patch removed

So the consensus is to add a $ignore_pagination parameter to get_the_content.

The current patch needs to be refreshed. Needs proper indentation and docs.

@jubstuff
2 years ago

Patch refreshed

#29 @jubstuff
2 years ago

  • Keywords needs-patch removed

Patch refreshed.

Should we add the $ignore_pagination parameter to the_content function, also?

Last edited 2 years ago by jubstuff (previous) (diff)

#30 follow-up: @swissspidy
2 years ago

  • Keywords has-patch added

Sweet, thanks for updating the patch! What's with that wp_trim_excerpt() change though? Seems unrelated.

Since this ticket was especially for the_content_feed(), I don't think we need to change the_content().

@jubstuff
2 years ago

#31 @jubstuff
2 years ago

@swissspidy You are right, that was unrelated. Fixed that.

#32 @swissspidy
2 years ago

  • Milestone changed from Future Release to 4.5

#33 @swissspidy
2 years ago

  • Keywords needs-unit-tests added

#34 in reply to: ↑ 30 ; follow-up: @stevenkword
2 years ago

Replying to swissspidy:

Sweet, thanks for updating the patch! What's with that wp_trim_excerpt() change though? Seems unrelated.

Since this ticket was especially for the_content_feed(), I don't think we need to change the_content().

I'm more in favor of putting this functionality into the_content() via a new optional parameter ($ignore_pagination), which is then always passed via the call inside the_content_feed(). There are cases outside of feeds where developers may want to analyze or parse the full content of a post, and I'd like to support those cases in this ticket.

However, that isn't a huge sticking point for me. As this relates to feeds, I believe the latest patch 10984.3.diff provides a solution that is superior to the functionality we have now in trunk. When users are leveraging feed aggregators to read content from multiple sources, I don't think we should be forcing them to leave their aggregator to get the complete story.

@swissspidy
2 years ago

#35 in reply to: ↑ 34 @swissspidy
2 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Replying to stevenkword:

There are cases outside of feeds where developers may want to analyze or parse the full content of a post, and I'd like to support those cases in this ticket.

Agree with that, 10984.4.diff implements this and adds a unit test.

#36 @swissspidy
2 years ago

  • Owner set to stevenkword
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by stevenkword. View the logs.


2 years ago

#38 @swissspidy
2 years ago

  • Keywords commit added
  • Owner changed from stevenkword to swissspidy
  • Status changed from reviewing to accepted

Considering for commit if there are no objections

@swissspidy
2 years ago

#39 @swissspidy
2 years ago

  • Keywords commit removed

Patch didn't apply cleanly anymore, which 10984.5.diff fixes (although using setup_postdata() because $pagesis null seems hacky).

Needs some tests in Tests_Feed_RSS2 for the feed part.

#40 @swissspidy
2 years ago

  • Keywords needs-unit-tests added; has-unit-tests removed

@swissspidy
2 years ago

#41 @swissspidy
2 years ago

  • Keywords has-unit-tests added; good-first-bug needs-unit-tests removed

Just added an additional test in 10984.6.diff.

#42 @swissspidy
2 years ago

  • Milestone changed from 4.5 to Future Release

#43 follow-up: @swissspidy
2 years ago

If we were to consider this for 4.6, it would need to go in early. Any objections?

#44 @swissspidy
2 years ago

  • Owner swissspidy deleted
  • Status changed from accepted to assigned

#45 in reply to: ↑ 43 @stevenkword
2 years ago

Replying to swissspidy:

If we were to consider this for 4.6, it would need to go in early. Any objections?

Do you think it's still early enough? I'd like to see this go in now before it goes stale, so I'm good with milestoning it if you are.

I can confirm that the patch still applies cleanly and that tests pass.

Last edited 2 years ago by stevenkword (previous) (diff)

#46 @swissspidy
2 years ago

  • Milestone changed from Future Release to 4.6

Alright, let's do this!

#47 @swissspidy
23 months ago

  • Keywords 4.7-early added
  • Milestone changed from 4.6 to Future Release

Totally forgot about this one :-(

The "early" ship has definitely sailed now that beta 2 is available.

Worth keeping in mind that #36934 might add changes to get_the_content() too. Perhaps the function signature should change to get_the_content( $post = null, $args = array() ) if it can be done in a backwards compatible way. Need to think about that a bit more.

This ticket was mentioned in Slack in #core-multisite by achbed. View the logs.


23 months ago

This ticket was mentioned in Slack in #core by helen. View the logs.


21 months ago

#50 @helen
21 months ago

@swissspidy: Just a nudge to see if you still want to do this early and to get it on your radar as you look at #36934.

#51 @swissspidy
21 months ago

  • Keywords 4.7-early removed

Thanks for the ping @helen! I still like to get this in early enough, but I first need to wrap my head around #36934 first. I'd actually say this ticket here is blocked by it.

Note: See TracTickets for help on using tickets.