[wp-trac] [WordPress Trac] #23497: Revisions Rewrite using JS/Backbone

Sat Mar 9 16:11:44 UTC 2013

#23497: Revisions Rewrite using JS/Backbone
---------------------------------------+------------------------
 Reporter:  adamsilverstein            |       Owner:  westi
     Type:  enhancement                |      Status:  reviewing
 Priority:  normal                     |   Milestone:  3.6
Component:  Revisions                  |     Version:  3.5.1
 Severity:  normal                     |  Resolution:
 Keywords:  needs-patch needs-testing  |
---------------------------------------+------------------------

Comment (by adamsilverstein):

 Thanks for your testing and detailed feedback. Here are some responses to
 your specific points:

 A. Thanks for catching that issue with the autosaves, not sure the best
 way to fix as you mentioned its an ajax save and the logic about showing
 the meta box happens at page load (are there x revisions for this post?).

 B. 1. JS off - this is a JS/Backbone rewrite of revisions and no-js has
 not been addressed at all. not sure what the approach will be, either no
 support or fallback to old code. 2. you shouldn't even be able to get to
 the revisions page unless you have at least two revisions

 C. that should NOT happen as you mention. if you change one line in each
 revision, that line change should be visible in each diff. if you can
 reproduce, please tell me steps.

 D. reverting a post may mess with the revisions page, aware of the issue.
 part of the problem is that currently the most recent revision represents
 the previous update, not the current post data - except after a restore.
 we are addressing this issue in #16215 and once that patch rolls i believe
 the logic will be cleaner.

  * from your description though, wouldn't 'four' show in any position?
 note that ''the diff view in single handle mode shows what has changed
 between the current version and the selected revision''. if the current
 version contains the 'four' edit, that will show on the +Added side of the
 diff for every revision comparison. again:  the view shows compare to
 current, not what happened in the revision (i tried it that way, but it
 wasn't liked)

 E. you are correct that no matter which revision link you click on, the
 revisions page always loads with the most recent revision selected; this
 is a known issue and will be addressed in an upcoming patch

 F. all comparisons are to the final revised post in one handle mode
 (currently actually uses post since no revision is available), thats the
 way the comparisons are currently set up. maybe we need to make that logic
 more apparent in the interface. also trying reversing the order so newest
 revisions are on the right

 G. Agreed 'Restore Revision' button is super confusing. in my upcoming
 patch you will see we changed what that button says 'Restore This
 Revision' and placement- next to the compare to revision data, hopefully
 making it clearer what happens when you click the button. i think my
 screencast may display a bug with which revision was restored, i'm looking
 into that.

 really appreciate the feedback and testing, obviously some kinks to work
 out! i should have another patch on the ticket early next week and could
 use more testing help if you are willing...

 Replying to [comment:61 bpetty]:
 > Replying to [comment:60 adamsilverstein]:
 > >  * did you do this test using the latest trunk? the code was updated
 recently and some of this sounds like buggy behavior that was fixed,
 please try with latest updates to see if you still have the issues; i was
 not able to reproduce the bug you described creating a new post and
 revising it --  the revisions screen worked as expected (although with the
 direction reversed)
 >
 >
 > I just did some testing in (completely) up-to-date SVN trunk, and I
 don't know how you didn't see errors because I spotted *several* problems
 right away. Both in my own testing, and in your own video.
 >
 > A. When creating a new post or page (with JS turned on), there are no
 revisions of the post, so the "Revisions" screen option isn't even
 available unless you have two revisions. The problem here is that if the
 second revision is an autosave (from JS), the revisions still don't show
 up on the edit screen like they do when you publish a second revision
 since it doesn't actually post the entire page, it's just an AJAX request
 (this might be an old problem though). This isn't a big deal since compare
 doesn't work at all with only one non-autosave revision anyway, however,
 this points me to a second problem...
 >
 > B. (with JS off) When opening a page or post with one revision and an
 additional autosave revision, the "Revisions" widget on the edit page
 lists the revisions (properly), but with links to compare, which end up on
 a blank compare page with no explanation about what happened, but there's
 actually two problems here. First, JS was off, so it just doesn't load
 anything, hence the blank page. Second, if JS is actually on, it *does*
 load a comparison with only one revision, but you're saying that shouldn't
 even be possible without at least two non-autosave revisions to compare -
 but it happens (and in fact, it's actually correct about the revision
 info, and the comparison diff content for once surprisingly - hey,
 creation of a page/post is an edit from nothing to something and even
 Wikipedia and all VCS diff apps will show a diff on that).
 >
 > C. As hinted to in (B), the diff content is entirely incorrect in
 several different situations (which I am finding tough to consistently
 reproduce, however, one bug or another keeps popping up with *every*
 single test I do, I just don't always know which one is next - so they
 aren't hard to spot). In one case, with 3 non-autosave revisions each only
 changing one line, and with only one line of content, I have been able to
 get the comparison to somehow end up with *two* lines of content from two
 different revisions on the same side of the diff, one line right after the
 other. That should definitely never happen. Fix the other bugs mentioned
 here first, and then it might be easier to reproduce and squash this bug
 (I'll come back again later to test again I'm sure).
 >
 > D. I did one test (without autosaves) where I created a post with "one",
 updated with "two", reverted to "one" (which worked), and updated the post
 with "four" (since it's the fourth revision), and while in comparison view
 (no matter how I got there from any revision), if the slider is on any of
 the revisions, it always showed "four" as being added even though it was
 only added in the last revision.
 >
 >
 > >  * the 'compare two revisions' should not be available if there are
 fewer than three revisions
 >
 >
 > This could be a little hard to test since it's tough to ensure an
 autosave doesn't sneak in right now, maybe just disable JS while testing
 (and re-enable on compare). That may have happened in Mike's test. In my
 own testing, I never did see the "compare two revisions" option unless I
 had at least 3 revisions.
 >
 >
 > > here is a screen recording of my test trying to reproduce the error
 you outline: http://cl.ly/NQb0
 >
 >
 > There are definitely errors right here in your own video...
 >
 > E. After you save the "revise" content, you clicked on the "22:03:08"
 revision, which was the oldest and first revision, however, the comparison
 page always opens on the latest revision first every time (which in your
 case was the "22:03:11" revision - from "test" to "revise").
 >
 > F. The first time you move the slider to the right, it says it's
 comparing the oldest "22:03:08" revision, which should have been the
 change from nothing to "test", not from nothing to "revise" - in fact,
 this removed/added combo *never* happens in any single revision - that's
 only if you compared two revisions from the non-existent post to the final
 "revise" post.
 >
 > G. When you click "restore revision", this is super confusing... what
 are you restoring? If it was the oldest revision's content, shouldn't your
 content have been "test" and not an empty post? Why was your content
 completely empty, but the title wasn't? If you click "restore" with the
 latest revision selected, doesn't that actually mean that nothing should
 happen? Shouldn't the restore button be disabled on the latest revision
 diff?

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/23497#comment:62>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software