Clicking "preview" multiple times on a post causes an identical autosave revision to be created and destroyed repeatedly

[inwerpsel]

How to reproduce:
Click the preview button multiple times in a row, without making any changes to the post in between. This will result with the autosave revision being created and destroyed repeatedly (check the response of the api request that does the autosave, it alternates between 200 and 400).

Detailed explanation:

Suppose the post has no autosave revision yet.

Clicking preview the first time results in a request being sent to "/wp-json/wp/v2/post_type/{id}/autosaves?_locale=user". In that request the autosave revision is looked up for the post.
​https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L360

Then, because there is no record, it will create a new one "as a special post revision" at

​https://github.com/WordPress/WordPress/blob/master/wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php#L393

After creating the request returns 200 and the autosave revision record is in the database.

Pressing the second time it will again look up the autosave revision and this time find it. Then an object is created with the $_POST data, but which has the old autosave revision's ID. That object is then compared to the current post, and if it's not different, wp_delete_post_meta is called with the old autosave ID and the record is deleted.

Notice how now we're back in the same state as at the start (a post with no autosave revision). So we can keep on pressing the preview button and the record will keep on getting created and deleted, even though it contains the exact same data as the post of which it is an autosave.

Why is this an issue:
This creates and deletes records in the wp_posts table with no real use, causing unwanted load and exhausting the ids of the wp_posts table.

Proposed solution:
Either no record should be created in the first place if there is no difference, or the existing record shouldn't be deleted, even if it doesn't have any changes.

[aduth]

Upstream (Gutenberg) issue: ​https://github.com/WordPress/gutenberg/issues/20505

As noted in the original report, I can see how there may be multiple solutions to this implementation that may or may not take place in Gutenberg, or via change to the REST API to be "smarter" about how it decides to create revisions.

[aduth]

Related previous discussion: https://core.trac.wordpress.org/ticket/43316#comment:90

[inwerpsel]

Looks like this issue occurs in more cases than I previously found. The autosave record is also temporarily cleaned up when loading the block editor in post.php. The autosave is looked up there to see if it is newer than the latest edit of the post, if it's not then it's deleted there.

​https://github.com/WordPress/WordPress/blob/master/wp-admin/edit-form-blocks.php#L323

Will anything break if the autosave isn't deleted in both cases? I would guess not at first glance, but maybe I'm missing something. I forced this locally by adding a filter:

<?php
// ...
        add_filter('pre_delete_post', [$this, 'do_not_delete_autosave'], 1, 3);
// ...
        public function do_not_delete_autosave( $something = null, $post = null, $force_delete = null ): ?bool {
                if ( preg_match( '/autosave-v\d+$/', $post->post_name ) ) {
                        return false;
                }

                return null;
        }

This doesn't seem to break anything. The only downside I see is that for each different author the autosave will be preserved indefinitely. However this is already the case for some of the autosaves, if no request that deletes the autosave is done after it has been created. Also that still seems preferable compared to deleting and creating new records many times over.

[inwerpsel]

The filter I proposed above resulted in the autosave not being deleted if the post is permanently deleted. Unfortunately the arguments of the filter are identical in both that case and the unwanted deletions, so I couldn't do anything but using $_GET to check if the current request is one for permanently deleting posts.

<?php
        private function add_filters() {
                add_filter('pre_delete_post', [$this, 'do_not_delete_autosave'], 1, 3);
        }

        public function do_not_delete_autosave( $something = null, $post = null, $force_delete = null ): ?bool {
                if (
                        $force_delete
                        || ( isset( $_GET['action'] ) && $_GET['action'] === 'delete' )
                        || ( isset( $_GET['delete_all'] ) )
                        || !preg_match( '/autosave-v\d+$/', $post->post_name ) ) {

                        return null;
                }

                return false;
        }

[adamsilverstein]

Proposed solution:
Either no record should be created in the first place if there is no difference, or the existing record shouldn't be deleted, even if it doesn't have any changes.

@inwerpsel thank you for opening this ticket and all the details around your efforts to prevent the behavior you described. Indeed: creating an autosave, then deleting it, then recreating it in an endless cycle is not the ideal (or intended) behavior.

I came across this ticket when working on previewing featured images in block themes because the underlying preview code that adjust post terms and the "thumbnail" (featured) image was failing when the autosave is missing. As you point out this happens every other autosave and autosaves are fired every time you preview a post.

I believe your suggestion that no autosave should be created when the autosave content matches the post content would be the cleanest way to resolve the issue, I'll look into that.

Trac ticket: https://core.trac.wordpress.org/ticket/49532

[adamsilverstein]

In 49532.diff​:

Revisions: don't create autosaves when all revisioned fields match the post

• In the autosave controller (wp-includes/rest-api/endpoints/class-wp-rest-autosaves-controller.php), adjust the existing check to see if the autosave exactly matches the post content to run a little earlier, setting autosave_is_different appropriately.

Then, when the autosave content matches the post content:

• Do not create an autosave, since that data is redundant (all revisioned fields match)
• Persist the current behavior of removing any existing autosaves for the current user that (again, because the data us redundant)

Prevents a cycle bug where the autosave would be added and removed with each autosave trigger, needlessly consuming resources and potentially exhausting post ids.

@inwerpsel if you are able, it would be great to have you confirm this patch resolves the issue you reported.

[adamsilverstein]

Marking this for 6.0 since its probably too late to commit something like this for 5.9.

[kirasong]

Hi @adamsilverstein ! We talked about ​this ticket today in a bug scrub.

Do you want to land this for 6.0, so it makes it before RC, or do you think it's too late to merge at this point?

[costdev]

As we are approaching RC1 and this ticket still needs testing, I'm moving this to the 6.1 milestone so that this will hopefully make it during the next cycle.

[mukesh27]

@adamsilverstein I tested the patch. I follow below steps.

1. Create new post with title and content
2. Save post.
3. Click on Preview button (Multiple time) it return "There is nothing to save. The autosave and the post content are the same." with no revision create.
4. Update content
5. Click on Preview button (Multiple time) it save the revision.
6. Again update some content
7. Click on Preview button (Multiple time) it save the this new revision and remove previous revision.
8. Again update some content
9. Click on Preview button (Multiple time) it save the this new revision and remove previous revision.

Is it valid? Do you think we have to save that revision?

[adamsilverstein]

@mukesh27 Thanks for testing!

Why is this an issue:
This creates and deletes records in the wp_posts table with no real use, causing unwanted load and exhausting the ids of the wp_posts table.

did you verify that this no longer happens?

[adamsilverstein]

49532.2.diff​ refreshes the patch against trunk and cleans up things a bit further.

@mukesh27 can you test again and confirm the reporter's issue is resolved? I think we can add a unit test here to verify the new behavior vs the old (maybe check the post id for the user's autosave doesn't change), let me know if you want to work on that.

@desrosj or @azaozz - any feedback on this change, anything I might be missing? I don't see a valid reason to keep deleting and recreating the autosave for the user when the post content matches; this change move the check earlier and returns early if the autosave being sent matches the post content.

[adamsilverstein]

49532.3.diff​ is a refresh against trunk.

I tested this again and verified it works as expected.

I'm a bit concerned about unintended side effects though, since the endpoint now returns an error where previously it did not. For this reason, I'm going to punt this to 6.2 and commit early in the release cycle so this change gets through testing.

Trac ticket: https://core.trac.wordpress.org/ticket/49532

[mukesh27]

@adamsilverstein I have added unit test in PR.

[adamsilverstein]

Excellent @mukesh27 - thank you.

I'll get this reviewed and pending any feedback commit it.

[audrasjb]

This patch was reviewed during today's bug scrub.
Even with the early workflow, we're going to keep it in the milestone for now. @adamsilverstein please feel free to move it to Future Release if you have any doubt about the proposed patch :)

[ironprogrammer]

Thanks for the latest updates to this, @adamsilverstein and @mukesh27! The steps below were adapted to include confirmation of autosave revision record changes.

Test Report

Patch tested: ​https://github.com/WordPress/wordpress-develop/pull/3794 👍🏻

Steps to Reproduce or Test

1. Create a new post.
2. Enter a post title, press Return or move the cursor to the block inserter, and publish the post. Make note of the post's ID.
3. Do not make any changes to the post. Click Preview > Preview in new tab.
4. 🐞 Observe that:
   • In browser dev tools that the request to /wp-json/wp/v2/posts/{id}/autosaves?_locale=user resulted in an HTTP 200 (okay) response. The response body contains details about the new autosave revision. Make note of the autosave's post ID.
   • In the database an autosave revision record was created for the post (where post_parent matches the post's ID, and the post_name is formatted as "{post_parent}-autosave-v1").
5. Repeat Step 3.
6. 🐞 Observe that:
   • In dev tools the new request to /wp-json/wp/v2/posts/{id}/autosaves?_locale=user resulted in an HTTP 400 (error) response. The response body contains the code rest_autosave_no_changes.
   • In the database the autosave revision record was deleted.
7. Repeat Step 3.
8. 🐞 Observe that:
   • In dev tools the new request to /wp-json/wp/v2/posts/{id}/autosaves?_locale=user resulted in an HTTP 200 (okay) response, with details about a new autosave revision. Note that the autosave's post ID is new.
   • In the database a new autosave revision record was created for the post.

Expected Results

When reproducing the bug:

• ❌ When there have been no changes to the post, previewing the post causes a new autosave to be created.
• ❌ If an autosave existed, previewing the post causes the autosave to be deleted.
• ❌ Subsequent previews continue to create new and then delete autosave records when no changes have been made.

When testing a patch:

• ✅ When there have been no changes to the post, previewing the post should not create a new autosave.
• ✅ If the post is changed, previewing should create an autosave, or update the existing autosave if it exists.
• ✅ Subsequent previews should not create a new autosave when no changes have been made.

Environment

• Hardware: MacBook Pro Apple M1 Pro
• OS: macOS 12.6.2
• Browser: Safari 16.2
• Server: nginx/1.23.3
• PHP: 7.4.33
• WordPress: 6.2-alpha-54642-src

Actual Results

When reproducing the bug:

• ❌ Before patching, testing matched the Expected Results above. 👍🏻

When testing a patch:

• ✅ After patching, previewing the unedited post did not create an autosave.
• ✅ When the post was modified, previewing the post created an autosave, or updated the existing autosave record.
• ✅ Clicking preview multiple times did not alternate between creating and deleting new autosave records in the database.

I've left some feedback regarding the unit test in a PR review. This is almost there!

Thanks @ironprogrammer for the feedbacks, I have address all the feedback in ​https://github.com/WordPress/wordpress-develop/pull/3794/commits/df9457f216cfca57e9bffc8489fefeda1c219032.

Ready for review and merge. Thanks!

[adamsilverstein]

Thnaks @mukesh27 and @ironprogrammer! I'll get this committed.

49532.4.diff​ is the latest code from ​the PR

[adamsilverstein]

In 55154:

Revisions: only create autosave when content changed.

In the autosave REST API endpoint, avoid excessive database writes when an autosave is sent with content that is unchanged from the saved post.

Before this fix, clicking "preview" in the editor (which uses the autosave feature) multiple times would cause an identical autosave entry to be deleted and re-created repeatedly.

Props inwerpsel, aduth, mukesh27, ironprogrammer.
Fixes #49532.

Merged in https://core.trac.wordpress.org/changeset/55154

merged in https://core.trac.wordpress.org/changeset/55154

[Mamaduka]

Hi, @adamsilverstein

I think r55154 introduced a regression with the post-thumbnail previews. If I preview a published post and the only change is the post thumbnail, it's not reflected in the preview. The preview works if you change the title or content.

My guess why it's happening:

Now that we bailout early from creating an autosave, the $post->ID != $post_id comparison in _wp_preview_post_thumbnail_filter will be false, when previewing the post thumbnail only changes.

How to reproduce

1. Edit an already published post.
2. Change only the post-thumbnail and click preview.
3. Notice that the preview displays an old thumbnail.

[adamsilverstein]

@Mamaduka interesting, I’ll take a look. Did this work correctly before that change?

[Mamaduka]

@adamsilverstein, I can confirm that post-thumbnail previews work in WP 5.9.

[adamsilverstein]

Ok. I’ll check if that commit is the culprit and see how we can best fix.

Trac ticket:

[adamsilverstein]

​https://github.com/WordPress/wordpress-develop/pull/4789 removes the changes. I expect the unit tests to fail and will remove the added tests as well.

We need a different way to fix the issue on this ticket. the previous approach failed because it didn't take into account updates to post meta completely.

[adamsilverstein]

@Mamaduka 49532.5.diff​ is a slightly different approach - the original revision id is returned when the autosave is unchanged - instead of an error. Can you give it a test/review please?

[adamsilverstein]

@adamsilverstein, I can confirm that post-thumbnail previews work in WP 5.9.

@Mamaduka in my testing the latest patch fixed featured image change previewing, can you please confirm?

ps. I'm going to open a follow up ticket to fix this bug.

[Mamaduka]

Thanks, @adamsilverstein!

The patch works as really well.

• The 400 error isn't displayed when previewing the already published post without any changes.
• The thumbnail previews work again.

[adamsilverstein]

The patch works as really well.

Great, thanks for testing. I'll make sure all the tests are passing then get this bug fix committed.

[adamsilverstein]

Follow up ticket to fix this: https://core.trac.wordpress.org/ticket/58739