Make WordPress Core

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#24136 closed defect (bug) (fixed)

Compare Two mode loads wrong post for left side when another post was created at the same time

Reported by: westi Owned by: ocean90
Milestone: 3.6 Priority: high
Severity: critical Version: 3.6
Component: Revisions Keywords: has-patch
Focuses: Cc:


In the revisions js we have the following code:

'compareTo': self.revisions.at( self.leftDiff ).get( 'ID' ) - 1,

Performing arbitrary increments or decrements on the ID won't work because there is no guarantee that they will be sequential on anything.

I'm attaching a patch that appears to fix it for me, but I'm not 100% convinced of what the logic is here - the JS could use some comments so it is clear what is going on.

Attachments (1)

24136.diff (574 bytes) - added by westi 3 years ago.
Simple patch which seems to fix the issue.

Download all attachments as: .zip

Change History (9)

3 years ago

Simple patch which seems to fix the issue.

#1 @johnjamesjacoby
3 years ago

  • Cc johnjamesjacoby added

#2 @ocean90
3 years ago

Seems like just a typo.

#3 @ocean90
3 years ago

For the right side it's correct, see trunk/wp-admin/js/revisions.js:24020#L184.

#4 @ocean90
3 years ago

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

In 24040:

Revisions JS: Fix typo. props westi . fixes #24136.

#5 @adamsilverstein
3 years ago

i believe values from the slider are off by one compared to the model index, hence the -1;

i will work to add better inline documentation :)

#6 @adamsilverstein
3 years ago

i added some inline documentation in #23897;

also did a little testing and verified the origin of all the -1 you see in the code - the diff values are indeed one off the model index.

jQuery UI slider reports leftmost position as 1 for two handled mode and 0 for one handled mode, the revisions code assumes left most position is 1 and behaves accordingly; since single handled mode returns left most as 0, it actually ADDS 1 to that value to keep it consistent with the two handled position.

it might be possible to change the stored value in the slider callbacks instead, but i'm nervous about changing/breaking what already works.

Last edited 3 years ago by adamsilverstein (previous) (diff)

#7 @ocean90
3 years ago

In 24116:

Optimize jQuery selector for checkbox synchronization. props SergeyBiryukov. see #24136.

Note: See TracTickets for help on using tickets.