Droppable: Large number of droppable elements causes crash

[whschultz]

When a user clicks in the header on a draggable panel that contains a large number of items in Chrome (verified in 32.0.1700.41 beta-m Aura on Windows 7), the page stops responding, and after about ten seconds, the Chrome task crashes.

This has been narrowed down a bit in this jsfiddle:

​http://jsfiddle.net/vuX59/6/

This does not crash in Firefox and appears not to crash in earlier versions of Chrome (eg 31, the current release), though the call is quite slow in all browsers.

It appears the issue (at a somewhat high level) comes down to a call to

(t.currentItem || t.element).find(":data(ui-droppable)")

which takes a while to run. I don't know enough about the internals of the jQuery code to know if this is a valid fix, but in my own code, changing it to this made the call much faster without (obviously) breaking anything:

(t.currentItem || t.element).find(".ui-droppable").filter(":data(ui-droppable)")

Obviously, since Chrome is crashing, I've reported this to the Chromium project as a defect, but I'm reporting it here, too, since it means there may be room for improvement in jQuery itself.

[tj.vantoll]

Hi whschultz,

Thanks for taking the time to contribute. We need to use :data(ui-droppable) as it finds actual droppable widgets rather than elements that happen to have the ui-droppable class name. That being said, :data is definitely more processing intensive and is probably leading to the issues you're seeing.

Since the only "problem" in the jQuery UI code is that it does this potentially expensive query, and since this is definitely a bit of an edge case, I'm going to close this as patcheswelcome for now. If someone can come up with a reasonable means of improving the efficiency of this query while maintaining the functionality we'd be happy to review a pull request.

If anyone else on the team disagrees please comment.

[whschultz]

I get that you need to find actual droppable widgets, but searching first by class name before filtering by :data(ui-droppable) should give the same results (while being significantly faster on large sets), so long as users haven't removed the ui-droppable class name, right?

[tj.vantoll]

Replying to whschultz:

I get that you need to find actual droppable widgets, but searching first by class name before filtering by :data(ui-droppable) should give the same results (while being significantly faster on large sets), so long as users haven't removed the ui-droppable class name, right?

That would be faster, but we can't rely on the class name.

[Scott González]

I'm ok with relying on the class name. Classes are part of our API; they're documented, tested, and any change is treated just like a change to a JS API.

[Scott González]

Actually, as long as we have the addClasses option, we can't rely on this.

[mikesherov]

Although, couldn't we see if addClasses was used, and if not, first filter by class? Also, how would this jive with the classes option we're thinking of adding?

[Scott González]

We can do that for now. In the future, we can always rely on the class since addClasses is being removed in the rewrite and the classes option doesn't allow removing the structural classes.