Make WordPress Core

Opened 11 years ago

Last modified 3 years ago

#25349 new enhancement

Can't retrieve calculated excerpt bound by <!--more--> on single page view

Reported by: chriscoyier's profile chriscoyier Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.6.1
Component: Posts, Post Types Keywords: has-patch needs-testing
Focuses: template Cc:

Description

Submitting as Mr. Nacin suggested would be a good idea.

The issue in words I understand is: when you use the_excerpt() on (for instance) a custom page template, it ignores the <!-- more --> comment that you might have carefully placed in your content. I would expect it to honor this, since that is kind of the point of that special comment. the_content() will honor it when used on non-single-pages, but in the case of a custom page template, the_content() will of course spit out the entirely of the content.

Andrew gave me this code which does the trick: https://gist.github.com/nacin/ab97d4b0e57b169d26d5 but is apparently hacky and not for public consumption.

This is my lame visual of where I was running into the issue: http://glui.me/?i=5bcy8yr99pl7xel/2013-09-17_at_4.28_PM_2x.png/

Also apparently I'm not the only one who's had this come up recently: https://twitter.com/ShaneHudson/status/380105193419186176

Attachments (1)

25349.patch (710 bytes) - added by Frank Klein 10 years ago.

Download all attachments as: .zip

Change History (22)

#1 @chriscoyier
11 years ago

Oh also. The docs are pretty clear on this point, so if it changes the docs would need to be updated.

http://codex.wordpress.org/Customizing_the_Read_More
http://codex.wordpress.org/Function_Reference/the_excerpt

#2 @Otto42
10 years ago

Are you referring solely to the automatically generated excerpt here, or are you trying to use more in a manual excerpt situation?

Need info, possibly example, to reproduce the problem. Input, expected behavior, actual output.

#3 @chriscoyier
10 years ago

Input and expected output: http://glui.me/?i=jufghv8i893tkux/2013-10-08_at_9.21_AM.png/

Actual output: the_excerpt() ignores my explicitly-chosen excerpt and just displays 55 words instead. the_content() honors it on non-permalink pages, but you can't always use that (e.g. you use a loop on a custom page template, it will just display the entire content, but the desire is to output an excerpt).

#4 @nacin
10 years ago

Basically, the_excerpt() should do internally what my gist above does. It should set $more to 0, then reset it to its original value when done.

These globals is some of the oldest code in core, dating back a decade. Lame, but a quick adjustment at least solves the problem here, until one day in the next few releases when we rewrite a lot of this code (we were close to doing so in 3.6).

#5 @nacin
10 years ago

  • Component changed from General to Template
  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

#6 @nacin
10 years ago

  • Component changed from Template to Posts, Post Types
  • Focuses template added

@Frank Klein
10 years ago

#7 @Frank Klein
10 years ago

  • Keywords has-patch needs-testing added; needs-patch removed

#8 @boonebgorges
9 years ago

nacin's suggested fix in the_excerpt() seems OK to me, but it also seems like it addresses the symptom rather than the root cause (namely, that $more is being set to 1 incorrectly in some cases). See https://core.trac.wordpress.org/ticket/9256#comment:18.

#9 @boonebgorges
9 years ago

In 30085:

Improve global variable setting in setup_postdata().

setup_postdata() is responsible for setting a number of global variables
that are used for post pagination ($pages, $page, $nextpage) and the
generation of post excerpts ($more). These variables should be sensitive to
the currently running instance of WP_Query - rather than the main query -
so that these features work properly inside of secondary WP_Query loops.

This changeset moves the logic of setup_postdata() into a method on WP_Query,
and converts setup_postdata() to a wrapper.

Props boonebgorges, wonderboymusic.
See #25349.
Fixes #9256, #20904.

#10 @boonebgorges
9 years ago

[30085] fixes the underlying issue. I'm leaving the ticket open in case someone thinks it's important to implement nacin's suggested fix as well. (I think it's redundant, but also harmless.)

#11 @NateWr
9 years ago

This change caused a problem with one of my plugins. While diagnosing this, I found a discrepancy I thought I would raise. The comment for lines 4561-4569 suggests the else statement should "respect" the $more global. But it actually sets it to 0 (potentially overriding it if the global has been modified elsewhere).

		// Force full post content when viewing the permalink for the $post, or
		// when on an RSS feed. Otherwise respect the 'more' tag.
		if ( $post->ID === get_queried_object_id() && ( $this->is_page() || $this->is_single() ) ) {
			$more = 1;
		} else if ( $this->is_feed() ) {
			$more = 1;
		} else {
			$more = 0;
		}

Is this the intended result? Prior to this commit, the function would leave the $more tag alone.

I appreciate the purpose of the commit was to be more forceful with the $more tag. But before I update my plugin, I wanted to check to make sure this was the intended behaviour. Should the $more global always be set back to 0 if it doesn't match these conditions?

#12 @boonebgorges
9 years ago

NateWr - Thanks very much for the report. To be clear, the 'more' referenced in the comment is the <!--more--> tag, NOT the $more global. The assumption is that when someone puts <!--more--> into their post content, it means that they want the truncated version of their post content to appear everywhere except for (a) when looking at the item's permalink, or (b) when in the RSS feed - that is, the default would be to "respect" the intention of <!--more-->.

I hadn't really thought about the case where plugins are modifying the $more global. Can you give more details about what your plugin is doing - both a description of what problem you're solving, and some technical details about how you were previously using the $more global to solve it? It would give a better sense of whether it's the kind of thing we should try to provide backward compatibility for.

#13 @NateWr
9 years ago

Thanks for the clarification boonebgorges. Since the $more global is pulled in on line 4547, I just assumed that this represented the $more global I'm familiar with in the templates. Are you sure that's not what's getting modified here?

I have a small reviews plugin designed for outputting short review snippets. In the use-cases I've envisioned for the plugin, reviews tend to be short excerpts taken from other sites, and so whether the review is displayed in a taxonomy archive or a widget, I generally want the full review displayed. So the plugin sets $more = 1 and then resets it after the reviews are printed. With the 4.1 beta, the $more var is now being reset to 0 when $reviews->the_post() is called in the loop.

I do think it's common to manipulate the $more global in this way. Though I understand this may be considered an inappropriate or abusive use of the $more global. If so, I'm happy to modify my plugin. I just wanted some clarity on this before moving forward.

#14 @boonebgorges
9 years ago

In 30367:

Add unit test files mistakenly excluded from [30085].

See #9256, #25349.

#15 @boonebgorges
9 years ago

Since the $more global is pulled in on line 4547, I just assumed that this represented the $more global I'm familiar with in the templates. Are you sure that's not what's getting modified here?

The <!--more--> *tag* is a bit of text that's entered by post authors to indicate a distinction between the "teaser" text - always displayed - and the "body" text - displayed only when full content is requested.

The $more *global* is, technically, an override for <!--more-->. If it's set to 1, then the entire post content is displayed, regardless of the <!--more--> tag. So they're closely related, but not the same thing.

I'm on the fence about what to do here. You may be right that plenty of plugins/themes are manipulating $more in order to perform an override like you're doing, and while it's not difficult to modify your own plugin to do the $more toggle after setup_postdata(), it'd be nice to avoid it.

One thought I had is to put the $more logic of setup_postdata() inside of an if ( $this->is_main_query() ) block. The thinking is that the only time core would want to force all content to be shown would be during the main $wp_query loop, and we can let plugins do whatever they want with secondary loops. The good thing about this is that I think it would fix your particular plugin and others like it, while maintaining the current <!--more--> behavior in core. The bad thing is that it will block plugins/themes from forcing $more for the main loop, though maybe this is a more acceptable break, if only because it's more rare. Do you have thoughts about it?

#16 @NateWr
9 years ago

To be honest, I would defer to those more experienced than myself regarding whether or not it's better to try to cater to the 101 ways the $more may be used or better to enforce a more consistent behaviour. The core devs have a better perspective on what kind of backwards compatibility is appropriate in a situation like this.

I don't have a strong preference as I can manage the transition in my plugin either way. If anything, this incident has shown me that my approach is probably not the best to take and I should begin thinking about how to move away from hijacking the $more global within the plugin.

Having said that, I suspect a is_main_query would go a long way. But it also might just confuse things more when trying to figure out what the $more global is going to be in any given situation.

#17 @boonebgorges
9 years ago

I scanned through the plugin directory to see how many people were manipulating $more in this way. I found quite a few examples (around 150). But of those, almost all of them will continue to work even after these changes. Most of them are manipulating $more while *inside* the loop - ie *after* setup_postdata() has run - so they will be immune. Others are forcing $more to 0, which I take to be a way of working around the bug that is fixed by this post (ie, respecting <!--more--> inside of a secondary loop). I found just a small handful that are forcing $more = 1 *before the loop begins*. Here's the list, for posterity: siteorigin-panels, post-ajax-slider, share-juice, wp-frontpage-news, nrelate-related-content, d13slideshow, wordpress-mobile-pack-hunt, baap-mobile-version, dws-testimonials, wp-enjoy-reading, random-one-cat-widget, yank-widget.

It makes sense that most plugin authors would choose to reset $more inside the loop: even before the changes in [30085], setup_postdata() would override this value in at least some situations. It just so happens that this didn't bother some plugin authors, because they wanted the value to be forced to 1 anyway. But IMO this doesn't change the fact that it's the wrong place to be overriding the value.

In light of the relative rarity of the problem, and the relatively mild consequences of failure (it's not as if sites will crash), I'm going to make the call that we'll keep things the way they are in [30085], so we can retain the broader fix. NateWr, I'd recommend that you modify your plugin so that it modifies $more while inside of the loop. Thanks for playing along :)

#18 @chriscct7
8 years ago

@boonebgorges did you want to keep working on this?

#19 @swissspidy
7 years ago

#21894 was marked as a duplicate.

This ticket was mentioned in Slack in #core-test by hellofromtonya. View the logs.


3 years ago

Note: See TracTickets for help on using tickets.