Make WordPress Core

Opened 2 years ago

Closed 2 years ago

Last modified 2 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 2 years ago.
Simple patch which seems to fix the issue.

Download all attachments as: .zip

Change History (9)

@westi2 years ago

Simple patch which seems to fix the issue.

comment:1 @johnjamesjacoby2 years ago

  • Cc johnjamesjacoby added

comment:2 @ocean902 years ago

Seems like just a typo.

comment:3 @ocean902 years ago

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

comment:4 @ocean902 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.

comment:5 @adamsilverstein2 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 :)

comment:6 @adamsilverstein2 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 2 years ago by adamsilverstein (previous) (diff)

comment:7 @ocean902 years ago

In 24116:

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

Note: See TracTickets for help on using tickets.