TinyMCE results in lost data if post save fails and back button is clicked

[archon810]

Here's a pretty annoying and destructive behavior that I'd like to figure out how to address.

Consider a scenario where an error occurs when you submit an update (let's say what's about to become revision 2) to a post (revision 1). When you click Back to go to the previous screen, expecting your updated version (2) to still be there, you'll actually observe that the original revision is there instead (1), thus losing your changes to 2 entirely.

That's exactly what's happening to us because of a content validation function that returns a wp_die() with a relevant error message displayed to the writers when an error is detected (there is a set of house rules, like no hotlinking of images, no double spacing, etc.). The idea is that if an error is detected, a message is shown, the error is corrected, and only then is the post allowed to save.

So what is causing the revision that was there when the page originally loaded rather than the most recently entered text to show up when you return to the page? Is that TinyMCE logic clobbering what one would expect to be saved submitted form data?

Is there a workaround to disable this behavior?

[archon810]

We actually tried to change this otherwise ideal behavior to return admin_notices instead, thus saving the revision instead of wp_die()ing, and forcing a draft status if you're trying to publish, but ran into a deal-breaking limitation.

If an update is done to an already published post, there's no way to use the above functionality because either:
a) the post will get unpublished (BAD) or
b) the update is allowed to go through, which means even though there may be an admin_notices message, it may go unnoticed, and an update that failed validation may be live for some time.

Any suggestions?

[azaozz]

Sounds like this is related or a duplicate of #35852. Can you re-test to see if it was fixed with [37619].

[archon810]

@azaozz I was just going to mention it - I don't think this is a duplicate and your fix works in this case because the save is blocked by the server (or a timeout or some other error), so when you go back, with the new refresh or without, the data will be lost.

[azaozz]

Replying to archon810:

Right, not exact duplicate, just related. I've repeated the same steps:

• Edit or create a post.
• Trigger wp_die() on attempting to save or update it.
• Press the Back button in the browser.

At this point the last saved revision (or autosave) is loaded in the browser, and the "restore from the data in browser storage" notice is triggered.

I'm not sure we can do anything more in WordPress to remedy this situation. Restoring from the backup in the browser works properly and brings the post content back.

We actually tried to change this otherwise ideal behavior to return admin_notices instead, thus saving the revision instead of wp_die()ing...

This should work: when the post is being saved (as draft or published), you can block updating it in the DB, save it somewhere temporary (post-meta perhaps), then load the rejected post_content in the editor when the Edit Post page loads after the redirect, and delete the meta.

[archon810]

Replying to azaozz:

Replying to archon810:

Right, not exact duplicate, just related. I've repeated the same steps:

• Edit or create a post.
• Trigger wp_die() on attempting to save or update it.
• Press the Back button in the browser.

At this point the last saved revision (or autosave) is loaded in the browser, and the "restore from the data in browser storage" notice is triggered.

I'm not sure we can do anything more in WordPress to remedy this situation. Restoring from the backup in the browser works properly and brings the post content back.

Wait, are you saying it brings the post content that you've edited or the post content that was available as of the last recorded db revision, as in losing all the changes you've just put in? Because if the latter, that's exactly the bug I'm trying to get sorted - WP should save the changes to the form using the browser cache and restore than instead of the original data. I think it's a TinyMCE issue because the logic there goes through the original data and uses JS to populate it in the form, actively disregarding any edited form data.

We actually tried to change this otherwise ideal behavior to return admin_notices instead, thus saving the revision instead of wp_die()ing...

This should work: when the post is being saved (as draft or published), you can block updating it in the DB, save it somewhere temporary (post-meta perhaps), then load the rejected post_content in the editor when the Edit Post page loads after the redirect, and delete the meta.

That's an idea, but it feels so dirty and prone to breaking. Ughhh. :( But without introducing more post states (like published but a new revision pending), it's probably the best we can do. However, maybe there's some merit in looking into the ability to draft updates to existing published posts?

[azaozz]

Wait, are you saying it brings the post content that you've edited or the post content that was available as of the last recorded db revision

Test it in trunk now, after [37619]. When the user clicks the Back button it loads the post as last saved on the server, then the "disaster recovery" in session storage kicks in and offers to restore post_content to the last state saved there. This is (should be) the same as the very last content the user submitted for saving.

but it feels so dirty and prone to breaking. Ughhh. :(

Huh, why? That's exactly what you're doing now (as far as you describe it and I understand it): when the user attempts to save an "improper" post you're blocking it by calling wp_die(). This is pretty bad UX :)

Best would be to use JS for the validation and run it before the form is submitted. If you want to do in in PHP, you can improve the UX significantly by returning the rejected content to the user and showing error message(s) what is wrong, so they can fix it and save. To do that you need to keep that rejected content until the page is reloaded after the first saving attempt. That's all :)

[archon810]

The reason I want to do it in PHP is because then there's a) no way to circumvent it by turning off JS or editing the code on the fly and b) it would also catch xmlrpc cases like Open/Windows Live Writer.

I'll test to see what happens after your changes, though I'll probably wait until it's in a release build.

The reason I said it feels dirty and prone to breaking is because right now we're not reinventing the wheel and touching any of the revision functionality - we simply deny some edits from going through and adding an error message. Once you start saving the content elsewhere and basically emulating saving/restoring revisions, that to me throws a red flag (as in, there are probably many more caveats here compared to our solution). It's a good suggestion, don't get me wrong, but much more complex IMO.

[azaozz]

Still thinking that calling wp_die() during saving of a post is not the best way to do things, perhaps only for security/permissions violations.

In case the edited post_content has to be rejected, it would be best to redirect and load post_content as submitted by the user. Even perhaps with highlighted "errors" that prevented it from being saved.

Closing as worksforme for now. Feel free to reopen if we can improve handling of similar cases on the old Edit Post screen, but keep in mind this is handled differently in the block editor.