Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

1.13.0 bug in sortable, draggable: freezes page, "this.dragDirection" is undefined #1997

Closed
danbernier opened this issue Oct 19, 2021 · 11 comments

Comments

@danbernier
Copy link

Upgrading jquery-ui from 1.12.1 to 1.13.0 seems to have introduced an exception coming from sortable.

jQuery: 3.6.0
jquery-ui: 1.13.0 (not present in 1.12.1)

When you first drag a sortable component, the page freezes, and this error appears in the console:

Uncaught TypeError: Cannot read properties of undefined (reading 'vertical')
    at $.<computed>.<computed>._intersectsWithPointer (sortable.js:595)
    at $.<computed>.<computed>.<anonymous> (widget.js:123)
    at $.<computed>.<computed>._intersectsWithPointer (widget.js:123)
    at $.<computed>.<computed>._mouseDrag (jquery.mjs.nestedSortable.js:203)
    at $.<computed>.<computed>._mouseDrag (widget.js:123)
    at $.<computed>.<computed>._mouseStart (sortable.js:316)
    at $.<computed>.<computed>.<anonymous> (widget.js:123)
    at $.<computed>.<computed>._mouseStart (widget.js:123)
    at $.<computed>.<computed>._mouseMove (mouse.js:150)
    at $.<computed>.<computed>.<anonymous> (widget.js:123)

It looks to come from this line in sortable.js, while evaluating this.dragDirection.vertical.

The only recent commit to sortable.js is 70dae67, but the file history shows a few other commits that I think were added after 1.12.1, so that may be misleading.

This seems to have a lot in common with issue #1991, though I suspect they're distinct.

@mgol
Copy link
Member

mgol commented Oct 20, 2021

Thanks for the report. Can you provide a test case on JS Bin? Thanks!

@mgol
Copy link
Member

mgol commented Oct 27, 2021

@danbernier
Copy link
Author

I'll take a crack at it later today.

@mgol
Copy link
Member

mgol commented Nov 7, 2021

@danbernier A gentle reminder about a test case. :)

@mgol
Copy link
Member

mgol commented Nov 15, 2021

@danbernier Another friendly ping. :)

@jwcooper
Copy link

Here is a demo showing the issue using nestedSortable and their default example:
https://jsbin.com/mizadimobu/2/edit?html,css,js,output

With jquery ui 1.12.1 it works fine, on 1.13.0 it throws the error. Sorry I don't have a simpler example for you, this is just one that we ran into, forcing us to rollback to 1.12.1 for now.

@danbernier
Copy link
Author

Thanks @jwcooper! Sorry this got away from me @mgol. @jwcooper's looks about like what mine would've been.

@mgol
Copy link
Member

mgol commented Nov 18, 2021

@jwcooper From what I see, the jQuery UI Nested Sortable library overwrites some methods in $.ui.sortable.prototype, in particular the private _mouseDrag method. When you do that, all bets are off. And, indeed, their version of _mouseDrag doesn't set the dragDirection property which is then used in other methods. That needs to be fixed in jQuery UI Nested Sortable.

@jwcooper are you by any chance using the same jQuery UI Nested Sortable library? Ah, I see from the stack trace that you do.

@mgol
Copy link
Member

mgol commented Nov 18, 2021

I don't think there's anything to be done on the jQuery UI side here. The bug is in the jQuery UI Nested Sortable library, the way it overwrote private parts of the Sortable widget has always been risky. The library doesn't look maintained, with the last commit being done 4 years ago. Your best bet may be forking the library and backporting jQuery UI 1.13.0 changes to it. For this specific issue, the most important is this bit of _mouseDrag: https://github.com/jquery/jquery-ui/blob/1.13.0/ui/widgets/sortable.js#L442-L445. I can't guarantee there won't be more changes to backport until it stops being broken, though.

I'm going to close this since the issue is not in UI but feel free to continue discussing the issue here.

@mgol mgol closed this as completed Nov 18, 2021
@mgol mgol removed this from the 1.13.1 milestone Nov 18, 2021
@jwcooper
Copy link

jwcooper commented Dec 6, 2021

@mgol Thanks for the tip on the issue and the fix, really appreciate it. It was indeed nested sortable that broke, and adding the dragDirection (and a few more things) worked with 1.13.

@codertushar
Copy link

This is fixed in ilikenwf/nestedSortable#138 (comment)
But the problem is it is not released yet(v2.1a). You can manually take the source code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment