Sortable: over & out events does not consistently fire

Confirmed. This appears to have been introduced by the conditional added here: https://github.com/jquery/jquery-ui/commit/07ce771a13504b851bb9f74c8ce8e960d207384a.

saluce65 could you reduce your test case so that it has the minimum amount of code to show this problem? There's a lot going on in your examples and it'd be easier to debug this if it were simplified.

I have reduced the code down to a bare minimum to show the issue.

http://jsbin.com/ahetev/9/edit

Replying to [comment:2 saluce65]:

I have reduced the code down to a bare minimum to show the issue. http://jsbin.com/ahetev/9/edit

Perfect, thank you.

#9445 is a duplicate of this ticket.

#9462 is a duplicate of this ticket.

Per #9462 this affects more than just draggables connected to sortables. If you drag a sortable item outside of a list then back in the over event will not fire.

Test case from Skaffen http://jsfiddle.net/3X2bL/2/.

My patch from #9462 seems to sort the issue with this ticket too. Am looking at doing a pull request in due course.

Done. First time I've used github for doing anything, so hopefully I did it all right :). https://github.com/matt-hoskins/jquery-ui/commit/885b90e753ed2bea49de8d62bf693d8b924e5f77

#9717 is a duplicate of this ticket.

The same commit in 1.10.2 also broke the

out

#9732 is a duplicate of this ticket.

Submitted a pull request https://github.com/jquery/jquery-ui/pull/1170 that includes a fix and additional tests.

I'd submitted a pull request with decent test coverage (as requested by tj.vantoll) 5-6 months ago:

https://github.com/jquery/jquery-ui/pull/1038

Nothing seemed to have happened on it though.

Replying to [comment:13 Skaffen]:

I'd submitted a pull request with decent test coverage (as requested by tj.vantoll) 5-6 months ago: https://github.com/jquery/jquery-ui/pull/1038 Nothing seemed to have happened on it though.

Apologies Skaffen. These sortable issues are time consuming to look through, and they often end up causing more problems than they fix. We have a rewrite of all of our interactions that's going on as well.

I will say that this issue in particular is something we should address in some fashion because it has come up numerous times. We'll try to get around to this.

That's ok - I figured it was just down to a lack of time. As an aside - I've noticed there were jshint issues with the new tests I'd written and I've just quickly fixed those with another commit on my branch (I think I may have inadvertently reverted a couple of information text corrections I'd made to a couple of the original tests while doing that, but that's very minor).

I've checked that my changes should still apply ok against the current master with only one conflict (which is down to the renaming of jquery.ui.xxxx.js to just xxxx.js so the TestHelpers.loadResources block changed) and that the tests still pass against the current master.

NiGhTTraX's patch takes a similar approach to mine but NiGhTTraX's patch will not trigger the "over" event when "(this.currentContainer === this.containers[innermostIndex])" is false whereas mine will - as it's been 6 months since I looked at that area so everything I'd figured out about sortables has fallen out of my brain so I'm not sure if that difference is important. Note that tests also cover interactions with draggables as this bug affected those as well as just sortable ones.

Replying to [comment:15 Skaffen]:

NiGhTTraX's patch takes a similar approach to mine but NiGhTTraX's patch will not trigger the "over" event when "(this.currentContainer === this.containers[innermostIndex])" is false whereas mine will - as it's been 6 months since I looked at that area so everything I'd figured out about sortables has fallen out of my brain so I'm not sure if that difference is important. Note that tests also cover interactions with draggables as this bug affected those as well as just sortable ones.

The triggering in that case is done further down the code, specifically at line 925. I also tested that it works with draggables as well.

Both fixes seem equivalent to me.

You're quite right NiGhTTrAX - my mistake for just looking at the patch and not the code that follows.

Actually where our patches differ, I think, is that in the case where "!(this.currentContainer === this.containers[innermostIndex])" and "this.containers[innermostIndex].containerCache.over" your patched version will still trigger "over". I think that's why I rearranged the code how I did in my patch. I can't remember if it's possible for those two things to be true though (i.e. I can't recall if I was seeing redundant "over" events in my testing) or was just erring on the side of caution.

Replying to [comment:17 Skaffen]:

Actually where our patches differ, I think, is that in the case where "!(this.currentContainer === this.containers[innermostIndex])" and "this.containers[innermostIndex].containerCache.over" your patched version will still trigger "over".

You're right, but that was the original logic so I didn't want to mess with that. I did test that the over event is fired only once in my test cases, so either those 2 things can't ever happen at the same time, or more tests need to be written.

#10541 is a duplicate of this ticket.

Sortable: fire "over" and "out" even when a connectWith hasn't changed

Fixes #9335

Changeset: 1cfebf803beedef05e8dcdd54e34d10c412a9a2b

#10558 is a duplicate of this ticket.