Yoast SEO breaks restoring of revisions due to processing shortcodes

[smerriman]

• I've read and understood the contribution guidelines.
• I've searched for any related issues and avoided creating a duplicate issue.

Please give us a description of what happened.

When a shortcode changes the $post variable, then resets it with wp_reset_postdata, restoring a revision of anything containing that shortcode redirects you to the wrong URL (whatever the last value of $post was).

Please describe what you expected to happen and why.

It should redirect you to the page you restored.

How can we reproduce this behavior?

1. Create a shortcode, eg:

add_shortcode('testshort',function($atts) {
	global $post; $post = get_post(1); setup_postdata($post); wp_reset_postdata();
	return '';
});

2. Add this shortcode to a post (without ID 1), and then create a couple of revisions.

3. Attempt to restore a revision of this post. It will restore correctly, but you will be redirected to the post with ID 1.

Without Yoast SEO active, restoring a revision does not process the shortcode at all. With Yoast SEO active, it appears the shortcode is processed, without the main query set, so wp_reset_postdata returns early and things break.

Used versions

• WordPress version: 5.03
• Yoast SEO version: 9.4

[Djennez]

Hi @smerriman, thank you for your submission.
I was able to reproduce the issues you described. We'll look into this a bit more.

[Djennez]

When a post gets retrieved from revisions, it is saved as the "new" version of the post. This invokes a save_post hook. Once this is done, a redirect is called to the edit page of the post that is currently in the global $post variable.

Our plugin hooks into the save_post hook. While this is done, shortcodes get executed to expand the content of the post. This is done to be able to analyze the full content of the post for our SEO purposes. Your shortcode replaces the global $post variable which is used to redirect to the correct edit.php page after saving and therefore the redirect goes to the post that you define. This happens because the wp_reset_postdata() function is a frontend function that does nothing in the backend.

To fix this particular instance, you can store the $post at the start of your function and restore it at the end. For example:

function custom_short(){
	global $post;
	$old_post = $post; // Store $post for restoration
        // Do some cool stuff
        $post = $old_post; // Restore the global $post
	return '';
};

Because we hook into the save_post hook, where any plugin would be able to change variables around, I don't think this is something that we should fix. Instead, shortcode functions should properly reset any edited variables on return.

Closing this as it's more about the code in the shortcodes than our plugin.

[smerriman]

I disagree that this is the responsibility of the shortcode. Using wp_reset_postdata is the correct method for shortcodes, and you're right; it's not meant to be called in the backend, because processing shortcodes is not meant to be done outside of the loop, and the plugin is doing that.

Gutenberg itself had this problem as they run shortcodes for the preview, and it was determined this needed to be fixed in Gutenberg, not in every single shortcode that plugins may add:

WordPress/gutenberg#7468

A similar fix therefore needs to be applied to Yoast SEO.

[Djennez]

Hi @smerriman , thanks for the reply. I contacted one of our architects about this and this is what we determined:

This is indeed not a shortcode issue but rather a core bug. See: https://core.trac.wordpress.org/ticket/18408

The Gutenberg fix was intended only as a temporary workaround. See: https://core.trac.wordpress.org/ticket/18408#comment:29

However, at this time we do not consider this issue severe enough to implement a (temporary) workaround ourselves and believe the core issue should be fixed.