thank you for your effort at cleaning up the code, its definitely getting better!

i did a little testing and found a few problems, not sure of the source but i can try to dig a little deeper into the code to see whats happening.

here is what i saw:

in single handle mode, the Restore button should be hidden when the handle is in the right most position - the current revision; it doesn't make sense to restore the current revision so the button is hidden. everything else in the single handle mode seems fine.

in the two handled mode there is an issue with the left handle which seems to be stuck showing [Current Revision]; when dragging the left handle the 'To:' part changes when only the 'From:' part should change; it looks like the ajax data calls are off, i see right_handle_at=0 in both initial two handled calls, one should not have this variable at all, one should have right_handle_at=[wherever the handle is at]; something is broken with the url. if i discover more i will contribute to your branch on github

Another bug: when there is a notice "There is an autosave of this post that is more recent than..." the autosave is not listed in the revisions postbox. Clicking the "View the autosave" link doesn't let you restore that autosave. There is a "Restore This Revision" button for the [Current Revision] but not for the [Autosave].

Indeed, Diff.rightDiff === Diff.revisions.length is not the best way to toggle the "Restore" button visibility, since the last revision is not always the current one. 23901.5.diff​ adds a new is_current_revision property to the Revision model, and uses it to determine the "Restore" button display.

When there is a notice "There is an autosave of this post that is more recent...", clicking the "View the autosave" link shows the wrong diff: switched left/right parts, the content that is new in the autosave shows as deleted on the left instead of as added on the right.

When there is a notice "There is an autosave of this post that is more recent...", clicking the "View the autosave" link shows the wrong diff: switched left/right parts, the content that is new in the autosave shows as deleted on the left instead of as added on the right.

Clicking on the Restore button restores the autosave properly.

question:

is the id of the revision correct? that is, when you click the 'view the autosave' link, is the id in the link url the same as the stored autosave?

it almost seems like the comparison direction left/right is reversed? might it be an ordering issue with the list of revisions ( ala #23923 )? would be interesting to see if changing the ordering to ID fixes this.

is the id of the revision correct? that is, when you click the 'view the autosave' link, is the id in the link url the same as the stored autosave?

Yes, it's the correct post ID.

it almost seems like the comparison direction left/right is reversed? might it be an ordering issue with the list of revisions ( ala #23923 )? would be interesting to see if changing the ordering to ID fixes this.

No, changing the sorting doesn't seem to fix it. There are 6 revisions on the post, one is autosave:

ID

post_author

post_name

9298

4

9289-revision-v1

9297

1

9289-revision-v1

9296

4

9289-revision-v1

9295

1

9289-revision-v1

9294

1

9289-autosave-v1

9290

4

9289-revision-v1

Usually autosaves are not the latest revisions as they get updated while the user is editing. Revisions are never updated.

Also noticing we don't show the autosave(s) when listing the revisions in the Revisions postbox. In some cases there may be many autosaves (for different users) and no revisions. All autosaves should be listed there imho, why have them if we don't tell the user he can restore?

is the id of the revision correct? that is, when you click the 'view the autosave' link, is the id in the link url the same as the stored autosave?

Yes, it's the correct post ID.

it almost seems like the comparison direction left/right is reversed? might it be an ordering issue with the list of revisions ( ala #23923 )? would be interesting to see if changing the ordering to ID fixes this.

No, changing the sorting doesn't seem to fix it. There are 6 revisions on the post, one is autosave:

ID

post_author

post_name

9298

4

9289-revision-v1

9297

1

9289-revision-v1

9296

4

9289-revision-v1

9295

1

9289-revision-v1

9294

1

9289-autosave-v1

9290

4

9289-revision-v1

Usually autosaves are not the latest revisions as they get updated while the user is editing. Revisions are never updated.

Also noticing we don't show the autosave(s) when listing the revisions in the Revisions postbox. In some cases there may be many autosaves (for different users) and no revisions. All autosaves should be listed there imho, why have them if we don't tell the user he can restore?

thanks for the details, i will take a look and fix the bug.

i agree the autosaves should be shown in the post box and will fix that as well

is the id of the revision correct? that is, when you click the 'view the autosave' link, is the id in the link url the same as the stored autosave?

Yes, it's the correct post ID.

it almost seems like the comparison direction left/right is reversed? might it be an ordering issue with the list of revisions ( ala #23923 )? would be interesting to see if changing the ordering to ID fixes this.

No, changing the sorting doesn't seem to fix it. There are 6 revisions on the post, one is autosave:

ID

post_author

post_name

9298

4

9289-revision-v1

9297

1

9289-revision-v1

9296

4

9289-revision-v1

9295

1

9289-revision-v1

9294

1

9289-autosave-v1

9290

4

9289-revision-v1

Usually autosaves are not the latest revisions as they get updated while the user is editing. Revisions are never updated.

Also noticing we don't show the autosave(s) when listing the revisions in the Revisions postbox. In some cases there may be many autosaves (for different users) and no revisions. All autosaves should be listed there imho, why have them if we don't tell the user he can restore?

is the id of the revision correct? that is, when you click the 'view the autosave' link, is the id in the link url the same as the stored autosave?

Yes, it's the correct post ID.

it almost seems like the comparison direction left/right is reversed? might it be an ordering issue with the list of revisions ( ala #23923 )? would be interesting to see if changing the ordering to ID fixes this.

No, changing the sorting doesn't seem to fix it. There are 6 revisions on the post, one is autosave:

ID

post_author

post_name

9298

4

9289-revision-v1

9297

1

9289-revision-v1

9296

4

9289-revision-v1

9295

1

9289-revision-v1

9294

1

9289-autosave-v1

9290

4

9289-revision-v1

Usually autosaves are not the latest revisions as they get updated while the user is editing. Revisions are never updated.

Also noticing we don't show the autosave(s) when listing the revisions in the Revisions postbox. In some cases there may be many autosaves (for different users) and no revisions. All autosaves should be listed there imho, why have them if we don't tell the user he can restore?

Looking at 23901.8.diff: changing from ID to post_date in wp_ajax_revisions_data() fixes the above problem.

Not sure about the changes in wp_list_post_revisions(). Seems the $parent arg there is not needed now as the revisions always include a copy of the current post as a "real" revision. Checking the plugins repo, this function doesn't seem used anywhere except in core, so deprecating one arg won't bring any back-compat problems.

As this function is called after the revisions have been converted, it's necessary to check the revisions version and prepend @post.

Looking at 23901.8.diff: changing from ID to post_date in wp_ajax_revisions_data() fixes the above problem.

Not sure about the changes in wp_list_post_revisions(). Seems the $parent arg there is not needed now as the revisions always include a copy of the current post as a "real" revision. Checking the plugins repo, this function doesn't seem used anywhere except in core, so deprecating one arg won't bring any back-compat problems.

As this function is called after the revisions have been converted, it's not necessary to check the revisions version and prepend @post.

Revisions: compare revisions by date in wp_ajax_revisions_data(), deprecate the $parent arg in wp_list_post_revisions() as now revisions always include a copy of the current post, props adamsilverstein, see #23901