Ticket #9041 (closed bug: fixed)

Opened 3 years ago

Last modified 10 months ago

Sortable: Unstable dragging / sorting with jQuery UI 1.9.1 and above

Reported by: marventus.wp Owned by:
Priority: minor Milestone: 1.10.2
Component: ui.sortable Version: 1.9.1
Keywords: Cc:
Blocking: Blocked by:

Description

There seems to be a bug involving the sorting of elements in jQuery UI > 1.9.0 (affected versions: 1.9.1, 1.9.2, 1.10.0). Dragging and sorting are very jerky and unstable in source container are incredibly hard to resort after original sorting in target container.
I created a small demo tester to try to narrow down the problem:  http://elblawg.com/jqueryui-test/.

That's how I was able to determine the issue starts occurring with 1.9.1 and above. The offending line is 4310 of the full 1.10.0 version (ui-sortable section). Here is the diff I created:

--- C:/xampp/htdocs/blogs/elblawg/jqueryui-test/js/jquery-ui-1.10.0.js	vie feb  1 01:55:45 2013
+++ C:/xampp/htdocs/blogs/elblawg/jqueryui-test/js/jquery-ui-1.10.0_2.js	vie feb  1 01:55:31 2013
@@ -4307,7 +4307,7 @@
 		if(this.containers.length === 1) {
 			this.containers[innermostIndex]._trigger("over", event, this._uiHash(this));
 			this.containers[innermostIndex].containerCache.over = 1;
-		} else {
+		} else if(this.currentContainer != this.containers[innermostIndex]) {
 
 			//When entering a new container, we will find the item with the least distance and append our item near it
 			dist = 10000;

LMK if you need further assistance.

Marv

Change History

comment:1 Changed 2 years ago by marventus.wp

I was also able to recreate the issue in  JSFiddle.

comment:2 Changed 2 years ago by tj.vantoll

  • Status changed from new to open
  • Summary changed from Unstable dragging / sorting with jQuery UI 1.9.1 and above to Sortable: Unstable dragging / sorting with jQuery UI 1.9.1 and above

I can see the issue as well, here's a fiddle you can play with the version numbers in  http://jsfiddle.net/tj_vantoll/FeTr2/.

This broke with this commit that landed in 1.9.1:  https://github.com/jquery/jquery-ui/commit/bae06d2b1ef6bbc946dce9fae91f68cc41abccda#ui/jquery.ui.sortable.js. That same commit is also blamed for #8792 with had a pull that attempted to re-add the same line you're trying to add, but unfortunately re-adding that line causes other regressions.

#8792 was related to floating and this one isn't although it appears the root cause is the likely the same.

Last edited 2 years ago by tj.vantoll (previous) (diff)

comment:3 Changed 2 years ago by jeff.hagen

I also have the same issue. I've recreated the issue here:  http://jsfiddle.net/2rt9Z/ I had to downgrade to 1.9.0.

comment:4 Changed 2 years ago by jeff.hagen

This is also the cause of another issue. As of version 1.9.1, the over event fires on every pixel movement. I've recreated the issue at  http://jsfiddle.net/PZB5x/. Watch the console output.

comment:5 follow-up: ↓ 6 Changed 2 years ago by marventus.wp

Hello again. Ok, so it wasn't just me then, ;-P

@tj.vantoll: I understand what you mean. Could you provide more information on the other regressions you noticed? I tested the fix and it didn't seem to generate any issues, but, of course, it is impossible to contemplate every possible scenario. Thanks!

Last edited 2 years ago by marventus.wp (previous) (diff)

comment:6 in reply to: ↑ 5 Changed 2 years ago by tj.vantoll

Replying to marventus.wp:

Hello again. Ok, so it wasn't just me then, ;-P

@tj.vantoll: I understand what you mean. Could you provide more information on the other regressions you noticed? I tested the fix and it didn't seem to generate any issues, but, of course, it is impossible to contemplate every possible scenario. Thanks!

The commit that caused this issue was a fix for #8572, #8573, and #8574. What we really need are tests that show that re-adding that line fixes this issue, and doesn't cause any regressions with #8572 / #8573 / #8574.

comment:7 Changed 2 years ago by marventus.wp

Hello again.

tl;dr: Re-adding the old line I mentioned (4310) does not seem to cause any serious trouble in #8572, #8573, or #8574
.

Detailed: I ran some tests on JsFiddle based on the tickets you provided and the demos created by @zhizhangchen and the proposed fix does not seem to revert sorting action to any of the situations described in any of them. I did notice that, in respect to #8572, the placeholder takes a while before "coming down" to a new position, but sorting works well regardless of that. IMHO, restoring line 4310 the way it was in 1.9.0 leads to a more stable sorting experience overall.

Tests:
1.  http://jsfiddle.net/Marventus/ZRz5W/1/
2.  http://jsfiddle.net/Marventus/ytxEG/
3.  http://jsfiddle.net/Marventus/RMVpV/

Last edited 2 years ago by marventus.wp (previous) (diff)

comment:8 Changed 2 years ago by zhizhangchen

I've sent a pull request to fix these problems:  https://github.com/jquery/jquery-ui/pull/916

comment:9 Changed 2 years ago by John Chen

  • Status changed from open to closed
  • Resolution set to fixed

Sortable: Skip triggering over event if it's alreay over the continer. Fixes #9041: the over event fires on every pixel movement

Changeset: 07ce771a13504b851bb9f74c8ce8e960d207384a

comment:10 Changed 2 years ago by mikesherov

  • Milestone changed from none to 1.10.2

comment:11 Changed 10 months ago by begizi

I believe this bugfix introduced a new bug that causes the "out" event not to fire with connected lists.

comment:12 follow-up: ↓ 13 Changed 10 months ago by begizi

I believe this bugfix introduced a new bug that causes the "out" event not to fire when using connected lists.

comment:13 in reply to: ↑ 12 Changed 10 months ago by tj.vantoll

Replying to begizi:

I believe this bugfix introduced a new bug that causes the "out" event not to fire when using connected lists.

Please create a  new bug with a reduced test that shows this issue if you'd like us to look into this.

Note: See TracTickets for help on using tickets.