Description

View revision history for that post. Note that the revisions are attributed to the wrong authors.

The revision's post_author is currently being set to whoever edited the post away from the state represented by that revision row. It should be set to the person who edited the post *to* the state represented by that row.

This bug has been around since the introduction of post revisions.

To fix for future posts, I can think of two options.

We should be able to pull the correct author for a new revision row from the _edit_lock/_edit_last post meta of the post (as long as we grab that before we update the post row).

Currently, when a post is updated, revisions first grabs a copy of the current state of the post, stores it as a revision, then updates the post row. To fix this bug, we could instead update the post row first then store a copy of that new state as a revision. That would mean the most recent revision for all posts would be a duplicate of the actual post.

Option 2 would be cleaner code, option 1 would be cleaner data.

To fix for existing posts, we need to go through each post and fix each revision. That's incredibly expensive to do on upgrade, so I suggest doing it per post on the fly when the post edit screen or post revisions screen is loaded.

If we fix on the fly, we have to be able to keep track of which posts/revisions have been fixed and which haven't. We could track that with:

An option that is set on upgrade with a timestamp. Compare post_modified to that timestamp. This seems fragile to me since I bet there are plugins that override post_modifed.

Post meta. Easy, but adds one post meta per post just to fix a lame bug.

Bump the menu_order of each fixed revision row from 0 -> 1. (A version number for the revisioning system :)). Hacky.

I like 3: menu_order.

We could also leave everything as is, allow the data to be wrong, and "fix" it on display or even in get_post().

Aside: While we're in there, we may want to "fix" revisions' post_modified columns. Currently, post_modified is identical to post_date, which is the time the post was put into the state represented by the revision. We could make post_modified the time the post was edited away from the state represented by the revision.

For future posts, it seems to me that easiest would be simply to add post_author to the versioned fields in _wp_post_revision_fields(), and then the existing post author would be saved rather than the current one. But in that function it says for post_author:

// WP uses these internally either in versioning or elsewhere - they cannot be versioned

For the record, I don't see a reason to fix this for "existing posts". This problem has gone unresolved from day one when post versioning was conceived. If WordPress could just save the correct freakin username when I edit posts, I would be very happy.

I took the menu_order approach outlined above in the ticket, but used comment_count instead since a page's menu_order is actually versioned by the revisioning system. This comment_count trick won't work if plugins (or future core) allow comments on revisions. As an alternative, we could hack the version into post_name.

I chose not to update the old revisions' DB rows so we could try the patch out without committing ourselves to the (destructive) UPDATEs. UPDATEs should be reasonably straightforward to add, and we should do UPDATEs since the way the patch fixes on display could mix poorly with persistent caches.

i'm working on getting this ticket resolved! thought i would start by testing the patch provided by mdawaffe.

the patch did not apply cleanly, but i was able to get it applied to current and tested locally using three users. the patch works, with some minor issues, i think its easiest to see in screen shots.

for each of the tests i created a post with the content 'admin' as the user admin, next i switched to user 'testuser' and edited the content to be 'testuser', next i switched to 'testuserb' and changed the content to 'testuser B', finally i logged as the user admin and checked revisions. the screenshots show the meta box on the edit post screen and the revisions review screen as well as each revision comparison.

note: there two revisions listed for my initial admin action of creating the post, the first is triggered when i left the title field, the second being my 'publish' action (publishing with only a title in another test removed this extra initial revision) . this seems like it would be confusing to end users, shouldn't the initial revision (shown) be my first publish?

also note another problem here, the meta-box on the post edit page shows one admin revision, one testuser revision, but no testuserb revision, which was the last revision. i'm logged back in as admin and haven't hit update yet - i should see testuserb's change in the revision list, and I DO in fact see it once i go to the compare revisions screen. this seems like a new bug/regression and needs fixing, might just be showing one too few revisions.

is using comment_count for revisions a good idea? will we ever want comments on revisions? any good ideas for another approach?

here were comments from original patch "I took the menu_order approach outlined above in the ticket, but used comment_count instead since a page's menu_order is actually versioned by the revisioning system. This comment_count trick won't work if plugins (or future core) allow comments on revisions. As an alternative, we could hack the version into post_name."

any other input here or leave as is?

address this issue from original ticket?: "Currently, post_modified is identical to post_date, which is the time the post was put into the state represented by the revision. We could make post_modified the time the post was edited away from the state represented by the revision." comments? is this issue in the post or in the revisions? move to another ticket

attached combined patch for testing, also fixes (related) #9843; removes initial revision (last one in list) from display so after test scenario above you see only three revisions, admin save, testuser save, testuserb save.

discovered the lovely 'self comparison detected' easter egg along the way! he he cute! :}

Cool. Let's work on one bug at a time. Smaller, focused patches will move forward more quickly.

The most important thing to fix is the revisions' authorship. Once we have that done, we can look at any UI/UX issues.

I'm working on a patch that moves the revision version data to post_name. We already take that field over for revisions.

The patch also takes a different approach in displaying the correct author (one that won't poison any persistant caches), and will have a way to upgrade each revision row. I'm not yet sure when we should call that upgrade function.

Stores revision_version wp_posts.post_name. Revisions already completely takes over that field, so we may as well use it.

Updates _edit_last post meta during wp_insert_post() so that meta value is always correct.

For new revisions, uses _edit_last post meta as the revision author (the user who edited the post to that state).

For old revisions, fixes the post_author on the fly during display using an expanded get_the_modified_author.

If the correct author cannot be determined, it makes an essentially arbitrary guess) and appends a '?' to the end of the author's display name.

The post_author as corrected by get_the_modified_author is not stored in the post object to avoid cache pollution.

The post_author as corrected by get_the_modified_author is not stored in the DB.

As a side effect of using get_the_modified_author everywhere, the display of the "current revision" author in wp_list_post_revisions() is fixed

This addition to get_the_modified_author could be made a filter.

Includes a new _wp_upgrade_revisions_of_post() to upgrade a post's revisions. Currently uncalled.

Goes through each revision from most recent to oldest and determines if and how the revision needs to be upgraded.

Uses a options table hack to get an exclusive lock on the upgrade routine for that post. Calling this function multiple times in parallel would be racy.

Here's how I tested:

New blog with 3 users running unpatched trunk.

Create a new post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

View the revisions for that post. Notice how they are off by one.

Apply the patch.

View the revisions for that post. Notice how they are correct.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

View the revisions for that post. Notice how they are correct.

Revert the patch.

View the revisions for that post. Notice how the revisions created before the patch was applied are off by one.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

Apply the patch.

View the revisions for that post. Notice how all are correct except one: whenever there is a revision_version=1 revision chronologically followed by a revision_version=0 revision, we do not know what author created the revision_version=0 revision. Authorship for revision_version=0 revisions is stored in the chronologically preceding revision. Notice the question mark after that revision author's name. It's a guess (a bad one).

Run the upgrade function on that post.

View the revisions for that post. Should be the same as in step 21.

In the above testing procedure, we end up with revisions having something like the following revision_versions going from oldest to newest:

0,0,0,0,0,0,1,1,1,0,0,0

After the upgrade function is run, we end up with:

1,1,1,1,1,1,1,1,1,0,1,1

That revision on the boundary between 1 &rarr; 0 is the revision for which we have to guess the author.

Those guessed authors should be rare. They only occur when the site is upgraded to include this patch, then downgraded, then upgraded again.

In the normal wp-admin editor, a revision is created essentially every time a post is first created. Without that initial revision, it's possible that for posts created pre-patch, get_the_modified_author() will have to guess at the author of the first revision. So created pre-patch with XML-RPC, for example, might have guessed first revision authorship. I'm not sure: I haven't tested that.

I'm not sure when to run the upgrade function. On post edit? On revision listing? Asynchronously on post edit via WP_Cron?

In theory we could let the data in the DB be incorrect for ever, and always correcty it dynamically on display or even in WP_Post.

Stores revision_version wp_posts.post_name. Revisions already completely takes over that field, so we may as well use it.

Updates _edit_last post meta during wp_insert_post() so that meta value is always correct.

For new revisions, uses _edit_last post meta as the revision author (the user who edited the post to that state).

For old revisions, fixes the post_author on the fly during display using an expanded get_the_modified_author.

If the correct author cannot be determined, it makes an essentially arbitrary guess) and appends a '?' to the end of the author's display name.

The post_author as corrected by get_the_modified_author is not stored in the post object to avoid cache pollution.

The post_author as corrected by get_the_modified_author is not stored in the DB.

As a side effect of using get_the_modified_author everywhere, the display of the "current revision" author in wp_list_post_revisions() is fixed

This addition to get_the_modified_author could be made a filter.

Includes a new _wp_upgrade_revisions_of_post() to upgrade a post's revisions. Currently uncalled.

Goes through each revision from most recent to oldest and determines if and how the revision needs to be upgraded.

Uses a options table hack to get an exclusive lock on the upgrade routine for that post. Calling this function multiple times in parallel would be racy.

Here's how I tested:

New blog with 3 users running unpatched trunk.

Create a new post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

View the revisions for that post. Notice how they are off by one.

Apply the patch.

View the revisions for that post. Notice how they are correct.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

View the revisions for that post. Notice how they are correct.

Revert the patch.

View the revisions for that post. Notice how the revisions created before the patch was applied are off by one.

Edit the post as user_id=1.

Edit the post as user_id=2.

Edit the post as user_id=3.

Apply the patch.

View the revisions for that post. Notice how all are correct except one: whenever there is a revision_version=1 revision chronologically followed by a revision_version=0 revision, we do not know what author created the revision_version=0 revision. Authorship for revision_version=0 revisions is stored in the chronologically preceding revision. Notice the question mark after that revision author's name. It's a guess (a bad one).

Run the upgrade function on that post.

View the revisions for that post. Should be the same as in step 21.

In the above testing procedure, we end up with revisions having something like the following revision_versions going from oldest to newest:

0,0,0,0,0,0,1,1,1,0,0,0

After the upgrade function is run, we end up with:

1,1,1,1,1,1,1,1,1,0,1,1

That revision on the boundary between 1 &rarr; 0 is the revision for which we have to guess the author.

Those guessed authors should be rare. They only occur when the site is upgraded to include this patch, then downgraded, then upgraded again.

In the normal wp-admin editor, a revision is created essentially every time a post is first created. Without that initial revision, it's possible that for posts created pre-patch, get_the_modified_author() will have to guess at the author of the first revision. So created pre-patch with XML-RPC, for example, might have guessed first revision authorship. I'm not sure: I haven't tested that.

I'm not sure when to run the upgrade function. On post edit? On revision listing? Asynchronously on post edit via WP_Cron?

In theory we could let the data in the DB be incorrect for ever, and always correcty it dynamically on display or even in WP_Post.

I just discovered there's a bug in the upgrade script: the very first revision is never upgraded... I think that may be intentional :) I'll look into it later.

i don't think we need to worry about upgrade/downgrade/upgrade you mentioned. i'm not even sure we need to upgrade the old db entries, i think just showing them correctly and storing them correctly from now on might suffice.

i am trying to create a unit test for this bug, but need a little help. my attached test goes thru the steps outlined above to reproduce the bog, and should fail if the bug exists, but it passes... (usually anyway - in my test environment it sometimes fails, i'm not sure why).

i must be doing something, or several things, wrong because the assertions all pass before the patch and fail after the patch. if someone can help me out here, i'm happy to write more tests for the revisions bugs we are fixing.

Currently, when a post is updated, revisions first grabs a copy of the current state of the post, stores it as a revision, then updates the post row. To fix this bug, we could instead update the post row first then store a copy of that new state as a revision. That would mean the most recent revision for all posts would be a duplicate of the actual post.

This was suggested as having cleaner code, but less clean data.

I want to argue that this is actually the best approach in terms of data as well. When talking extensively with benbalter about revisions, conflicts, locking, post forking, and editorial flows (author of the Post Forking plugin and contributor to both #18515 and #12706), a big issue pointed out is that you don't actually have a revision ID in hand until the post is edited an additional time. This makes it fairly difficult to do anything interesting. Ben ended up writing a routine that calculates "what is the revision after revision X, because that's the one I actually wanted but it didn't exist at the time, I only had X". Instead of storing Y, you have to store X+1. It makes everything simpler if you have a revision for everything.

Currently, when a post is updated, revisions first grabs a copy of the current state of the post, stores it as a revision, then updates the post row. To fix this bug, we could instead update the post row first then store a copy of that new state as a revision. That would mean the most recent revision for all posts would be a duplicate of the actual post.

This was suggested as having cleaner code, but less clean data.

I want to argue that this is actually the best approach in terms of data as well. When talking extensively with benbalter about revisions, conflicts, locking, post forking, and editorial flows (author of the Post Forking plugin and contributor to both #18515 and #12706), a big issue pointed out is that you don't actually have a revision ID in hand until the post is edited an additional time. This makes it fairly difficult to do anything interesting. Ben ended up writing a routine that calculates "what is the revision after revision X, because that's the one I actually wanted but it didn't exist at the time, I only had X". Instead of storing Y, you have to store X+1. It makes everything simpler if you have a revision for everything.

i will try to rearrange the order of post update/revision save as per suggestion 2 above. any idea why the current order is used and if i'm likely to break anything by making this change?

this should fix the issue with authors on new revisions, but we still need a way to correct old data unless we decide old bad data is ok. it looks like the existing patch does a reasonable job of that, so i'm going to leave that code and just update the part where revisions are created.

this fix should also address the post_modified issue mentioned in the original ticket.

16215.2.diff​ shifts the revision save until after the post update and seems to make sense based on discussion above: namely by shifting until after the post is actually updated, we should capture the latest post info - including post_author and post_modified.

note: the current save revisions code compares the passed data to the current post data and if there is no change doesn't store a revision (see #7392 and #9843). 16215.2.diff​ would break that. if we are storing the revision after the post is updated we need a different way to check for changes. i will work on that.

under the old system when users clicked update on a post the system did two things:

saved a copy of the post information stored in the database as a revision (eg. the previous post update data)

saved the current post editor information to the database post record

This resulted in:

revisions stored for the previous edit only, not for the current post information

however, the current post information IS stored in the database as the post data, it's just not stored as a revision

in addition, the old system stored two revisions for the first save, further confusing things!

Furthermore:

The old revisions compare system did a poor job of displaying the revisions, treating the last revision as the current post even though it was not, meaning all revisions were one position off - hence the current ticket.

However:

The new revision version in #23497 works around this data issue by prepending a copy of the current post to the list of revisions when building the diffs for display.

testing existing post data with the new revisions system the data (i believe) looks fine, restores work correctly, and the current version of the post is used for comparisons: in the one handle mode the current post is compared to all previous revisions; in the two handle mode, the leftmost handle position represents the current post version (and data is pulled from). the language properly indicates the 'current version' is being compared. this could use more testing!

I already created 16215.3.diff implementing fix 2 above, after this fix when users click update on a post the system does two things:

saves the post to the database

stores a copy of the post data as a revision

This results in:

one revision stored for each update... sounds good so far, except that:

the new revision data no longer matches the old revision data, with messy results

the code adds a new hook and breaks anything that assumes wp_save_post_revision is going to run at pre_post_update

Therefore, I suggest closing this ticked as fixed in #23497, but would appreciate a second option on this conclusion

can't we just treat the currently stored post data as the most recent revision, why do we need to store an additional copy of the current post data as a revision? i think the real issue this ticket exposes was on the revision display/compare side, the storage logic makes sense.

Currently, when a post is updated, revisions first grabs a copy of the current state of the post, stores it as a revision, then updates the post row. To fix this bug, we could instead update the post row first then store a copy of that new state as a revision. That would mean the most recent revision for all posts would be a duplicate of the actual post.

This was suggested as having cleaner code, but less clean data.

I want to argue that this is actually the best approach in terms of data as well. When talking extensively with benbalter about revisions, conflicts, locking, post forking, and editorial flows (author of the Post Forking plugin and contributor to both #18515 and #12706), a big issue pointed out is that you don't actually have a revision ID in hand until the post is edited an additional time. This makes it fairly difficult to do anything interesting. Ben ended up writing a routine that calculates "what is the revision after revision X, because that's the one I actually wanted but it didn't exist at the time, I only had X". Instead of storing Y, you have to store X+1. It makes everything simpler if you have a revision for everything.

wp_save_post_revision is now called before and after post update is fired, the pre post update call corrects old revision data on the fly by saving a copy of the previous post save as a revision before the post data is updated. once the most recent revision modified date matches the current post modified date, the pre update calls stops doing anything. the post update call always saves a copy of the updated post as a revision.

after updating a post, the most recent revision will always match the current post; for old posts two revisions will be added: one for the previous save (not yet stored) and one for the current version.

a new helper method wp_first_revision_matches_current_version tests if the most recent revision matches the post (new data), the revisions display code in #23497 will also need to know if the current post is stored as a revision.

two revisions are still created for the 1st post publish since post update gets called when users leave the title field, one revision is title only with blank content, the next includes the content.

latest patch on this ticket does correct data for existing posts, the first time a post is updated.

had asked @nacin about upgrading post data on WordPress database upgrade, but he said no. all thats required is running wp_save_post_revision on every post. wp_save_post_revision checks if a current post copy exists as a revision and saves a copy if not.

developers can safely call wp_save_post_revision() to make sure a current revision exists (it will not create a duplicate if one already exists).

16215.7.diff​ is a refresh of 16215.4.diff​ diffed against the current trunk. i tested and it indeed fixes the author off by one issue; also after applying patch from 23497 it continues to show correct revisions.

i also updated ajax-actions to use the corrected call for correct display of the revision author. 16215+23497.diff​​ combines the two tickets for testing, applying both patches doesn't work directly.

I'm very happy to see this bug getting the attention it deserves. Thanks, adamsilverstein, for championing it :)

&gt;
&gt; With the latest commit at #23497[23769], what patch should I apply to test things?

yes, this ticket is really going to help button up the new revisions updates!

now that the latest patch on #23497 was committed, this patch needs one more refresh; i am planning to work on it tonight. in addition to your code - which works perfectly as far as i can tell, thanks - i made the requested change so that a current copy of the post is stored as a revision, instead of the previous changes being the most recent revision.

for a while i thought that was all that was required, but I was very wrong! both fixes together yield the best results - corrected authors for old revision data (and correct authors for new revisions), and revisions data that includes the current post data as a revision (once a post is updated or a developer calls wp_save_post_revision())

i should have a refreshed patch for testing for you tonight, or if you want you can piece it together:) i also need to update ajax-actions to get the new revisions ui to use your function call to get the post author and will include that in my patch.

did initial testing, created post before patch with several authors, off by one; applied patch, authors corrected (in display, on the fly); added new revisions, attribution correct, newest revision matches post

When a new revision is saved, the old incorrectly attributed revisions aren't fixed.

correct. the update code is there, but inactive - it needs review to see if a. its coded well and b. its needed.

the names are always corrected on display, even though old revisions have the wrong data and new revisions contain the correct data, the author in the list and revisions ui should always show the correct author. i will work on correcting the gravatar and dates mentioned above.

corrects autor name and gravatar in revision list and revision comparison, on the fly

on first post update old data is corrected, further updates leave data intact

author and gravatar dislpay correctly before and after data update

before patch, create post edit as user admin, user1, user2
note: user ID and content showing who updated are 'off by one' (shown in reverse order, bottom 'blank' content is 1st revision title only made by admin)

I worry about race conditions when multiple people (or multiple windows) open the same edit screen. Has anyone reviewed my option-based locking code?

i originally placed the update call at pre_post_update thinking that would be a safer spot, but markjaquith pointed out that if we can update the data before the post edit screen displays, there is no need for the on the fly correction at all.

Wondering if we can use the actual post lock instead of an option. By the time the revisions version update functions are called, the post is already locked to the user.

wp_check_post_lock() would return false when checked for the current user but we can look at get_post_meta( $post->ID, '_edit_lock', true ) for the lock "freshness" (regardless of the user) and prevent updating the revisions if the post was locked a few seconds ago.

Edit: on second thought, that can miss if two requests come in at the exact same second, so probably not good enough.

Wondering if we can use the actual post lock instead of an option. By the time the revisions version update functions are called, the post is already locked to the user.

wp_check_post_lock() would return false when checked for the current user but we can look at get_post_meta( $post->ID, '_edit_lock', true ) for the lock "freshness" (regardless of the user) and prevent updating the revisions if the post was locked a few seconds ago.

Edit: on second thought, that can miss if two requests come in at the exact same second, so probably not good enough.

16215.11.diff​​ fixes an issue where the 1st revision saved after applying had the wrong post_author (unless you had an autosave) - this was the replacement missing current revision; removed post_updated action, instead wp_save_post_revision in _wp_upgrade_revisions_of_post;

If we are saving a new revision after saving the main post (the change of filter in default-filters.php), there's no need to use get_post_meta( $post['ID'], '_edit_last', true ); in _wp_post_revision_fields(). In this case the revision will be exact copy of the last saved post and will have the right author. This also is not needed for autosaves, they use _wp_post_revision_fields() too.

the $last_revision could be an autosave. In some cases that works, in other, not.

_wp_upgrade_revisions_of_post() seems to be running each time a post is loaded for editing. It needs to run only once per post and update the revisions versions. Also some of the logic there is "backwards" :) First we save a revision, then check if the post_type supports revisions...

Edit: _wp_upgrade_revisions_of_post() also uses wp_update_post() to update revisions when fixing the author and revision version. This results in changing post_modified on all revisions to the current date/time. When listing them that looks pretty weird. Perhaps we can do a custom db query to only update the two fields that need updating?

&gt;
&gt; - Always update the revision version when updating the post authors.
&gt; - Check if revisions have been updated and return early in _wp_upgrade_revisions_of_post().
&gt; - Update the revisions by direct query to avoid resetting post_modified in wp_update_post() and making all revisions show as saved right now. A lower level API would have been good here :)
&gt; - Removed copying the post to a revision when no revisions exist. Can be added after this conversion if needed.
&gt;

think you still need the wp_save_post_revision( $post-&gt;ID ); line (removed in 16215-13.patch), but haven't tested yet. i expect removing it will mess up some data:

the reason is that under the old system there was no revision matching the current post data, it was always saving the previous, but not the current post. this line adds a copy of the current post as a revision so the last post save data gets stored and can have its author data updated (it will be off initially because the current user might not match the user who made the last changes).

in other words, this call needs to happen before the revisions data is corrected, if we do it after we will have one revision with the wrong author data, or if it doesn't happen before the next update, we will have a gap with one missing revision as the new system saves a revision after the post update occurs so no current data revision will ever get saved.

think you still need the wp_save_post_revision( $post-&gt;ID ); line... so the last post save data gets stored and can have its author data updated...

What's happening at the moment is:

New (last) revision is created matching the post data.

That revision has post_author set from post_meta '_edit_last' (so the author is correct) and -v1 in post_name.

When we get to the conversion code block, this revision is bypassed with:

// 1 is the latest revision version, so we're already up to date
if ( 0 < $this_revision_version )
continue;

We can still add that before the conversion but it won't affect it. I see it's convenient to have this in _wp_upgrade_revisions_of_post() so once that function runs, all the needed conversions are done.

Seems good place for it is right after we check whether the revisions for the current post have been converted so it doesn't run all the time. Then $revisions = wp_get_post_revisions( $post->ID ); is already set and we won't look at it while converting the rest of the revisions.

think you still need the wp_save_post_revision( $post-&gt;ID ); line... so the last post save data gets stored and can have its author data updated...

What's happening at the moment is:

New (last) revision is created matching the post data.

That revision has post_author set from post_meta '_edit_last' (so the author is correct) and -v1 in post_name.

When we get to the conversion code block, this revision is bypassed with:

// 1 is the latest revision version, so we're already up to date
if ( 0 < $this_revision_version )
continue;

We can still add that before the conversion but it won't affect it. I see it's convenient to have this in _wp_upgrade_revisions_of_post() so once that function runs, all the needed conversions are done.

Seems good place for it is right after we check whether the revisions for the current post have been converted so it doesn't run all the time. Then $revisions = wp_get_post_revisions( $post->ID ); is already set and we won't look at it while converting the rest of the revisions.

ok, great. i didn't realize the new last revision was already being created, as long as thats happening we are fine.

i did notice that removing the revision save entirely from pre_post_update removed the initial title only revision for new posts, which messed up the revision UI because it expects that revision (and its present on all old posts). see ​http://core.trac.wordpress.org/ticket/23497#comment:135

We have unit test failures surrounding revisions. Some of them are indubitably because we're storing an extra revision now, so the expectation of the unit test is no longer valid. But some of them seem wrong in the wrong direction.

As far as I can see the above tests fail because they don't expect the extra revision added after saving the post.

Moving from 'pre_post_update' to 'post_updated' is a big change and there would probably be more bugs and inconsistencies. This also creates a copy of the post when it may not be needed, like publishing from QuickPress and PressThis and updating a post programmatically.

Another inconsistency: for some reason the guid for revisions is a permalink (instead of ?p=123 like drafts). That makes all revisions for a post have the same guid: 123-revision-v1.

As far as I can see the above tests fail because they don't expect the extra revision added after saving the post.

Moving from 'pre_post_update' to 'post_updated' is a big change and there would probably be more bugs and inconsistencies. This also creates a copy of the post when it may not be needed, like publishing from QuickPress and PressThis and updating a post programmatically.

Another inconsistency: for some reason the guid for revisions is a permalink (instead of ?p=123 like drafts). That makes all revisions for a post have the same guid: 123-revision-v1.

i agree, moving from saving revisions from pre to post update is a big change. i specifically asked nacin about this in IRC and the answer was essentially, 'now is the time'.

a current revision is needed when you want to add information to the current revision - whats currently stored in the db - as it is, you can't do this until the post is updated, because you don't have a revision yet. in addition, the old pre_ system introduced the bug we are correcting - old post data was saved ('authored') by the current user, hence authors off by one.

the current package of fixes brings the old data up to date (once), and starts keeps current revisions. plugin authors will have to adapt, but will hopefully cheer the new current revision.

We have unit test failures surrounding revisions. Some of them are indubitably because we're storing an extra revision now, so the expectation of the unit test is no longer valid. But some of them seem wrong in the wrong direction.

It's actually a bit heavy. It seems to run on every admin edit, and causes queries for revisions and some other execution, even after a post has been upgraded.

Yeah, was looking into that too. _wp_upgrade_revisions_of_post() has to run before the revisions postbox is shown, so we show the corrected authors and have the extra revision in there.

We are calling wp_get_post_revisions() on every load of edit-form-advanced.php (when determining whether to show the postbox), perhaps we can move the call to _wp_upgrade_revisions_of_post() there. We can use the loaded revisions to check the version, then upgrade them if needed.

Another option is to check the version and upgrade on the first call to wp_get_post_revisions(), maybe using a static, etc.

Move the call to _wp_upgrade_revisions_of_post() to wp_get_post_revisions() and run it after getting the revisions and checking their version, once per post.

While I'd tend to think that wp_get_post_revisions() should be accurate and thus is a great place to run the upgrade code, that also means we might be running upgrade code — that does write operations that requires a lock — on the frontend. No bueno.

[23842] correctly ensures that a first call to wp_insert_post() inserts the first revision. But, this should not happen on auto-drafts. (Per IRC discussion.) To fix this, in get_default_post_to_edit(), we can simply remove then re-add this filter.