WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 5 years ago

#3020 closed enhancement (fixed)

Autosave

Reported by: masquerade Owned by:
Milestone: 2.1 Priority: normal
Severity: normal Version: 2.1
Component: Administration Keywords:
Focuses: Cc:

Description

As per the meetup discussion. This includes Prototype 1.5.0, and if committed, can be marked as fixing #3017 as well.

Attachments (4)

autosave-and-prototype.diff (64.5 KB) - added by masquerade 8 years ago.
autosave-2.diff (64.5 KB) - added by masquerade 8 years ago.
post.php.diff (642 bytes) - added by masquerade 8 years ago.
Apply along with above.
post-new.php.diff (438 bytes) - added by masquerade 8 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 ryan8 years ago

Retaining wp_die() somehow would be nice since it helps with i18n character encoding issues. Can we mark up the error string somehow to make it easier to parse out?

comment:2 ryan8 years ago

Maybe change write_post() to return WP_Error and leave it to the caller how the error should be handled though. This change might break plugins though, so how about have wp_write_post() return WP_Error and make write_post() a wrapper around wp_write_post() that just wp_die()s with the error.

comment:3 ryan8 years ago

Looking in phpmyadmin, autosave looks to be emptying post_content. I'll poke around and see what I can find.

comment:4 masquerade8 years ago

I found the culprit, in copying code over I missed out on two lines that copy $_POSTcontent? to $_POSTpost_content?. Inconsistent naming is great. :)

In the process, I also realized that updating the post could overwrite things like the ping status with the default because they are not sent along with the autosave, and so if someone is editing a draft where they changed these settings they could be lost. I personally would rather we not send all this information along in the autosave, so my first thought is to patch wp_insert_post to use the old values instead of inserting the defaults, as this makes sense to me for plugins that may want to update a post with wp_insert_post. Thoughts?

comment:5 mdawaffe8 years ago

masquerade, can you just use wp_update_post()?

comment:6 masquerade8 years ago

New patch, autsave-2.diff

  1. Adds wp_write_post() which returns WP_Error.
  2. write_post() calls wp_write_post and uses wp_die as before, to keep i18n and such alive.
  3. Fixes emptying post_content
  4. Autosave now uses wp_update_post instead of wp_insert_post
  5. wp_insert_post now better supports updating posts. This is great for the plugin API because now if a plugin author wants to update only one field, that one field should be able to be passed in to wp_insert_post and the rest of the post data remains the same. This needs another set of eyes run over it.

I might've forgotten something, but that just means its not as important :)

masquerade8 years ago

comment:7 masquerade8 years ago

Erm, relooked at wp_update_post, which I had completely forgotten about. Nice catch mdawaffe, overwrote autosave-2.diff with the wp_insert_post changes reverted.

masquerade8 years ago

Apply along with above.

masquerade8 years ago

comment:8 ryan8 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [4082]) Autosave and prototype. Props masquerade. fixes #3020 #3017

Note: See TracTickets for help on using tickets.