Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#23906 closed defect (bug) (worksforme)

post_save callback is passed post instead of revision, breaking back-compat

Reported by: iandunn's profile iandunn Owned by:
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Revisions Keywords:
Focuses: Cc:

Description

Running the following snippet on 3.5.1 and 3.6-alpha-23875 produces different results, and the change can break backwards compatibility for plugins that have logic that targets the revision rather than the canonical post.

function print_revision_parent( $revision_id, $revision ) {
	var_dump( $revision->post_type );
	var_dump( $revision->post_parent );
	wp_die( '' );
}
add_action( 'save_post', 'print_revision_parent', 10, 2 );

In 3.5.1, the output is:

string 'revision' (length=8)
int 324

In 3.6-alpha-23875, it's:

string 'post' (length=4)
int 0

I wasn't able to find the changeset that caused it, but I'm assuming it's related to all the work around revisions for 3.6.

Change History (5)

#1 @SergeyBiryukov
11 years ago

  • Milestone changed from Awaiting Review to 3.6

#2 @kovshenin
11 years ago

  • Milestone 3.6 deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Looks like this is caused by [23842], however, wp_insert_post is running more than once, but you're running wp_die causing it to stop execution after the first round, which looks wrong. Prior to that change, the revision is saved first, then the post. After the change it's vice versa, but I confirmed that both the post and the revision are passed through the save_post hook.

Closing as worksforme, feel free to reopen.

#3 follow-up: @iandunn
11 years ago

The snippet above with the wp_die() call was just an example of how to reproduce the problem; you can view a real world example at http://pastebin.com/Et7VvABY.

The save_post callback at that link is targeting the 'post' post type only, so it checks the value of $post->post_type and returns if it doesn't match. (In this case, $post refers to the object that was passed in as the 2nd callback arg, not the global $post object).

Is checking the post type on the passed post object the wrong way to achieve that? If so, I'm happy to update my plugins. I'm wondering if there's a significant number of other plugins that would also be affected, though.

That callback in the link above is also intentionally only running on the first save_post call, which is just a general precaution I setup in most of my action callbacks. So, maybe this is only an issue when: 1) The save_post callback returns after the first run; and 2) The callback checks the value of the post_type on the object passed to the callback. If that's the case, then I would guess the number of plugins affected would be fairly small.

#4 in reply to: ↑ 3 @SergeyBiryukov
11 years ago

Replying to iandunn:

1) The save_post callback returns after the first run

Seems like that's the issue in your example. The assumption that the revision is saved first is no longer correct.

#5 @iandunn
11 years ago

Cool, thanks for helping me understand the problem.

Note: See TracTickets for help on using tickets.