When adding the post_formats_compat the_content filter a post ID argument should be included

[gcorne]

The new post_formats_compat() function relies on knowing the $post_id to get meta data for the post format. By default, the function is bound as a the_content filter, which doesn't pass a $post_ID. One possible solution is to change the number of accepted arguments when adding the filter to 2, although this will require third-party developers that are calling apply_filters( 'the_content', $content ) with an unset $GLOBALS['post'] or the global set to a different post to pass in the post id: apply_filters( 'the_content', $content, $post_id ).

wp-includes/comment.php:1752 is an example from core of using the_content on content that is not pulled from the global $post.

For completeness, it may make sense to do the same with the prepend_attachment filter.

[markjaquith]

WordPress core has, for years, relied on the convention that the_content filters are meant to be run inside a loop context with a valid global $post object. Multiple WordPress core functions already make that assumption, like gallery_shortcode() and prepend_attachment(), as well as all our embed handlers (which do caching by post ID). Any third party shortcode that touches post meta will be in the same boat, and won't work if $post isn't as expected.

It's not very clean, but it is expected and well-established behavior.

[gcorne]

I agree that it is well established behavior, but I don't agree that it is expected. The usage in do_trackbacks() is evidence that it is easy to not be aware of the state needed to properly apply the_content. It is also unfortunate that we are adding *new* code that is relying on $GLOBALS['post']. At this point, isn't everyone in agreement that usage of the global $post should be frowned upon if not deprecated?

[ryan]

Pass the post ID down the the_content() stack. Add phpdoc for the_content filter explaining portability issues of the ID arg

[ryan]

I didn't using real phpdoc params to avoid possibly polluting the generated phpdoc with things it does not understand.

[ryan]

Introduce wp_parse_post_content() and use it in get_the_content() to avoid global $pages

[ryan]

Global $pages in setup_postdata() is a problem. Passing an ID makes no difference since get_the_content() goes to $pages for the content. 24330.2.diff works around this, but the surgery is not minor.

The new $format_pages variable is usually a straight up duplicate of $pages. Storing the content twice in globals could use up a lot of memory.

[ryan]

Unit tests

[DrewAPicture]

Better phpdoc, remove extra $page global in the else

[DrewAPicture]

24330.3.diff​ fleshes out the phpdoc for wp_parse_post_content(). I also removed the extra $page global in the else.

[ryan]

I'll use this ticket for some quick notes on the guts of post formats.

It'd be nice if $format_pages is only set when absolutely necessary, such as when $post->split_content is set. Even better would be for this global to not exist. We have enough problems with $pages as it is. Perhaps we run with the patch here after all and extend it to make the_content() and the_remaining_content() independent of the globals in setup_postdata().

$post->split_content and $post->format_content[ $cache_key ] are bummers. If we're going to decorate WP_Post with these perhaps they should be declared as vars and documented with phpdoc. Also, are these getting into the object cache? Should they?

[ryan]

The patch here plus abstracting some stuff out of get_the_content() as hinted at in the phpdoc for get_the_remaining_content() could allow us to avoid shipping with $format_pages. I would really like to avoid introducing that global.

$post->format_content[] could go into a non-persistent cache bucket with a key like $post_id:media:$type to avoid decorating the object.

[ryan]

Avoid $format_pages. Avoid number of times content is copied around in wp_parse_post_content()

[ryan]

Pass post ID to the_remaining_content()

[ryan]

Declare format_content and split_content as vars for WP_Post and provide phpdoc

[ryan]

​[UT1286]

[ryan]

Use the id arg in get_the_password_form(). Props duck_.

[ryan]

Indicate that post_password_required() and get_the_password_form() can accept an ID or WP_Post object.

[aaroncampbell]

Seems to work as expected for me.

One thing I noticed: here you pass two parameters to post_formats_compat - ​http://core.trac.wordpress.org/attachment/ticket/24330/24330.9.diff#L9

But when you remove it and put it back you don't: ​http://core.trac.wordpress.org/attachment/ticket/24330/24330.9.diff#L234

[ryan]

Re-add the filter in the_remaining_content() with the correct number of args. Props Aaron.

[ryan]

In 24301:

• Introduce wp_parse_post_content() and use it in setup_postdata(), get_the_content(), and get_the_remaining_content().
• Add a post ID argument to the_content(), get_the_content(), the_remaining_content(), and get_the_remaining_content().
• Pass the post ID to the the_content filter.
• Remove the format_pages global.
• Declare format_content and split_content as vars in WP_Post.
• phpdoc for the the_content filter that documents the new ID argument and denotes it as not-so-portable.

Props gcorne, DrewAPicture, duck_, aaroncampbell
see #24330

[ryan]

​[UT1287]

[johnjamesjacoby]

Two things:

• Missed the second argument in the remove_filter() call inside of the_remaining_content(). (Patch incoming)
• r24301 broke some code we were using in a theme, that worked previously. Looks like this is a breaking change for loops like this:

  foreach( array( 8, 10, 12 ) as $id ) {
  	$page = get_post( $id );
  	if ( $page ) {
  		setup_postdata( $page );
  		the_content();
  		wp_reset_postdata();
  	}
  }
  

[johnjamesjacoby]

Also, the use of extract() here is a time-bomb. It's not obvious (even with the inline doc in get_the_content()) that wp_parse_post_content() sets up would-be global variables.

[hissy]

Hi, I have one thing...

If global $page is not set, this code returns null. Is it correct?

get_the_content('',false,1);

It seems that $page should be tested.

	if ( !is_int($page) || $page > count( $pages ) ) // if the requested page doesn't exist
		$page = count( $pages ); // give them the highest numbered page that DOES exist

[aaroncampbell]

Replying to johnjamesjacoby:

• Missed the second argument in the remove_filter() call inside of

​remove_filter() only takes 3 arguments, so I don't think 24330.patch​ is necessary.

[johnjamesjacoby]

Replying to aaroncampbell:

Replying to johnjamesjacoby:

• Missed the second argument in the remove_filter() call inside of

​remove_filter() only takes 3 arguments, so I don't think 24330.patch​ is necessary.

You're correct. Disregard that patch.

[ryan]

​[UT1289] is a unit test for the loop problem found by johnjamesjacoby.

[ryan]

setup_postdata() sets global $id but not global $post. This mismatch is the reason the new code breaks for loops that don't set global post to match the post passed to setup_postdata(). In get_the_content(), "if ( $post->ID != $GLOBALSid? )" is true in this case and wp_parse_post_content() is run on get_post( 0 ), which is the global post rather than the post that has its content sitting in the pages global. It was sheer accident that these loops ever worked, but worked they did so I'll see what can be done for back compat.

[ryan]

​[UT1290]

[ryan]

Patch checks the global post rather than the global post ID when checking for a mismatch between the requested post and the global post. The global post is more reliable and proper than the global ID for this situation.

[markjaquith]

I agree that $post is more reliable.

[nacin]

I thought we eliminated global $id everywhere. Guess we missed this one?

One suggestion, can we use get_post()->ID?

[ryan]

In 24336:

Use the global post rather than the global post ID. They don't always match, and the global post is more canonical.

see #24330

[nacin]

In 24337:

Remove use of global $id from comment_form() and the media list table. see #24330.

[nacin]

In 24598:

Remove wp_parse_post_content(), get_paged_content(), paginate_content() from 3.6, and remove the new $id parameters for get_the_content() and the_content().

The content parsing functions are good abstractions, but are no longer needed by core and are too closely tied to legacy globals, rather than paving a new path.

For get_the_content() and the_content(), this only worsens the function prototype. It muddies theme-specific display (more links, etc) with filtered content. apply_filters( 'the_content', $post->post_content ) is sufficient practice for now.

see #24330, [24301]. see #23625, [23804].

[ryan]

In 1314/tests:

Remove test_the_content_with_id(). ID supports was removed in [24598]. see #24330

[SergeyBiryukov]

In 48113:

Posts, Post Types: Simplify test_setup_postdata_loop().

The important part here is calling the_content() after setting up post data for another post without updating global $post.

The foreach() loop is not necessary.

Follow-up to ​[UT1289].

See #47824, #24330.