#10727 closed bug (fixed)

Opened December 15, 2014 07:55PM UTC

Closed March 30, 2016 10:53PM UTC

Last modified March 31, 2016 12:43AM UTC

Sortable: placeholder offset incorrect

Reported by: dstrohl Owned by: dstrohl
Priority: minor Milestone: 1.12.0
Component: ui.sortable Version: 1.11.2
Keywords: Cc:
Blocked by: Blocking:
Description

In trying to troubleshoot a bug in a page I may have found a bug in jQueryUI.Sortable. When I try to drop an object onto a sortable object, it will often drop higher in the stack than it should. (I am holding the item under all of the items in the stack, but the placeholder is showing up at the top of the stack.

In troubleshooting this, I added some console output lines to the jqueryui code and found that it seems like when it is comparing the position of the event to the position of the item (around line 14330 or so in my copy) it is using the event[clientx/y] position compared to the item.offset() position. if I am reading the docs correctly, the clientX/Y position is based on the offset from the current window, and the item.offset() is based on the offset of the document (is that correct?) This seems like it might be valid as by moving the window up and down relative to the document, the placement of the placeholder changes.

Since I am not an expert in JS or JQuery, it is entirely possible that I am missing something here, or have mis-read the code or docs...

This seems like it might be a dupe of several other reported bugs, but none of them seemed to indicate that the problem had to do with the offset position.

I also verified that this happens with versions 1.10.3, 1.10.4, 1.11.1, and 1.11.2

I apologize for not creating a jfiddle for this, hopefully the description is good enough.. if not, I can see what I can hack together.

Attachments (0)
Change History (9)

Changed December 15, 2014 07:58PM UTC by dstrohl comment:1

Darn it, I created it in the wrong place, I tried to change it to ui.sortable but dont see where I can do that. another thing to apologize for.

Changed December 15, 2014 08:29PM UTC by tj.vantoll comment:2

owner: → dstrohl
status: new → pending

Hi dstrohl,

Unfortunately we do need a reduced test case in order to look into this. You can use this as a starting point: http://jsfiddle.net/tj_vantoll/fwk8R/. If you need support please try our forums or Stack Overflow.

Changed December 15, 2014 10:45PM UTC by dstrohl comment:3

status: pending → new

No problem, I understand (I was hoping... but I get it <grin>);

so, here is a jsfiddle of the problem:

http://jsfiddle.net/dstrohl/17n55z5u/7/

Most of this is based on one fo the sortable examples from the main site.

So, after going to the example, the top sortable works as expected, but the bottom one (you may have to scroll down) does not. *(Make sure to have the window SMALL enough to require scrolling, it works as expected when you can see both).

I also put some basic instrumentation in to show my theory, based on what I could get from the jQueryUI code, it is comparing the clientX/Y to the item offset, so I am showing client X/Y and offset top/left in a box on the side.

You can see that the client x/y numbers are generally similar for both sortables, but the targetloc (from the ui.item.offset) is significantly different between the two.

Does this help?

Changed December 15, 2014 10:46PM UTC by dstrohl comment:4

(by the main site, I meant the jQuery main demo site)

Changed December 16, 2014 04:10PM UTC by dstrohl comment:5

Per the instructions, I tried creating a fix for this after forking the repo. (https://github.com/dstrohl/jquery-ui/blob/master/ui/sortable.js). Basically I just changed:

axix = floating ? "clientX" : "clientY";

to

axix = floating ? "pageX" : "pageY";

This worked on my test page, but what I dont know is if there was a reason for this being clientX/Y in the first place, and if therefore I would have just broken something else. I don't know how to run the tests in the system or I would have run those to see if it passed the tests, and I don't know how to submit my changes back to the core repo (or even if I should). Thoughts?

Changed January 10, 2015 10:42PM UTC by scottgonzalez comment:6

component: ui.core → ui.sortable

Changed April 05, 2015 03:15PM UTC by tj.vantoll comment:7

status: new → open
summary: Sortable placeholder offset incorrect → Sortable: placeholder offset incorrect

Verified this in master and verified that the fix corrects the issue: http://jsfiddle.net/9a1Lk6bd/

Changed March 30, 2016 10:53PM UTC by arschmitz comment:8

resolution: → fixed
status: open → closed

In [changeset:"9aca7067321643dcb4b5db03855591c35cd85d69" 9aca706]:

#!CommitTicketReference repository="" revision="9aca7067321643dcb4b5db03855591c35cd85d69"
Sortable: Update _contactContainers to use page clientX/Y values

The sortable was using the window position compared with page position to
determine where to drop objects.  this was only a problem for sortables far
enough down to require scrolling.

Fixes #10727
Fixes #5039
Closes gh-1475
Closes gh-1585

Changed March 31, 2016 12:43AM UTC by scottgonzalez comment:9

milestone: none → 1.12.0