Search and Top Navigation
#4088 closed bug (fixed)
Opened February 07, 2009 03:57PM UTC
Closed May 25, 2011 08:12PM UTC
Last modified May 25, 2011 08:13PM UTC
Unable to remove() ui.draggable (sortable item) immediately after the drop callback.
| Reported by: | bohdan.ganicky | Owned by: | |
|---|---|---|---|
| Priority: | major | Milestone: | 1.8.14 |
| Component: | ui.droppable | Version: | 1.6rc6 |
| Keywords: | Cc: | ||
| Blocked by: | Blocking: |
Description
Originally reported by spiderling in this thread:
http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b
Expected behaviour:
// this should remove the dragged item after dropping it
drop: function(ev, ui) {
ui.draggable.remove();
}
Actual behavior:
the draggable isn't removed
Workaround:
// this works
drop: function(ev, ui) {
setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough
}
Testcase:
http://jquery-ui.googlecode.com/svn/trunk/tests/visual/droppable/droppable_ticket_4088.html
Could be related to:
Attachments (0)
Change History (13)
Changed February 07, 2009 04:03PM UTC by comment:1
| description: | Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/demos/sortable/with-droppable.html \ → Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/demos/sortable/with-droppable.html \ \ '''Could be related to:'''[[BR]] \ #4087 |
|---|
Changed February 07, 2009 05:38PM UTC by comment:2
| description: | Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/demos/sortable/with-droppable.html \ \ '''Could be related to:'''[[BR]] \ #4087 → Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/tests/visual/sortable/sortable_ticket_4088.html \ \ '''Could be related to:'''[[BR]] \ #4087 |
|---|
Changed February 07, 2009 05:41PM UTC by comment:3
| description: | Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/tests/visual/sortable/sortable_ticket_4088.html \ \ '''Could be related to:'''[[BR]] \ #4087 → Originally reported by spiderling in this thread:[[BR]] \ http://groups.google.com/group/jquery-ui-dev/browse_thread/thread/7537f702fa1ce21b \ \ '''Expected behaviour:''' \ {{{ \ // this should remove the dragged item after dropping it \ drop: function(ev, ui) { \ ui.draggable.remove(); \ } \ }}} \ \ '''Actual behavior:'''[[BR]] \ the draggable isn't removed \ \ '''Workaround:''' \ {{{ \ // this works \ drop: function(ev, ui) { \ setTimeout(function() { ui.draggable.remove(); }, 1); // yes, even 1ms is enough \ } \ }}} \ \ '''Testcase:'''[[BR]] \ http://jquery-ui.googlecode.com/svn/trunk/tests/visual/droppable/droppable_ticket_4088.html \ \ '''Could be related to:'''[[BR]] \ #4087 |
|---|
Changed February 10, 2009 10:07AM UTC by comment:4
| resolution: | → fixed |
|---|---|
| status: | new → closed |
Fixed in r2062. Thanks!
Changed August 27, 2010 02:19PM UTC by comment:5
| resolution: | fixed |
|---|---|
| status: | closed → reopened |
Unfortunately the fix hasn't worked for IE6/IE7/IE8 - I discovered this myself when using a mix of droppables and sortables and then found this bug report and also this thread where some other people have hit this problem: http://forum.jquery.com/topic/ie-won-t-remove-dropped-draggables
If I go to the test case linked from this ticket then the test works fine under Firefox but not under IE6, IE7 and IE8.
In my code if, in the drop event, I do the following:
ui.draggable.remove();
alert(ui.draggable[0].parentNode);
alert(ui.draggable.parent().length);
On firefox I get "null" and then "0" alerted.
On IE6 I get "[Object]" and then "0" alerted.
I believe under IE that when an element is removed from the page its parentNode doesn't become empty - looking at the jquery code regarding parents it looks like it maybe gets shoved into an orphaned document fragment, hence why the parent method in jquery does this check on the parent node type:
var parent = elem.parentNode;
return parent && parent.nodeType !== 11 ? parent : null;
Thus the code in the fix for #4088 is, I think, doing the wrong check when it does this:
if(!this._noFinalSort && this.currentItem[0].parentNode) this.placeholder.before(this.currentItem);
As I'm guessing currentItem is the item that's in ui.droppable in the drop event. That line of code should perhaps do something like:
if(!this._noFinalSort && this.currentItem.parent().length>0) this.placeholder.before(this.currentItem);
Or alternatively it could have the same check on nodeType as jquery's parent method has.
I'm not a jquery/jqueryui expert, so I'm not sure if my suggested fix is the right way to do this, but that's my best guess :).
Changed October 19, 2010 09:09AM UTC by comment:6
Should be a quick fix, but this has had no attention (perhaps because of the milestone being stuck on the original milestone it was deemed fixed on) - should I raise it as a new bug instead?
Changed October 19, 2010 03:50PM UTC by comment:7
| priority: | critical → major |
|---|
Changed January 13, 2011 01:34AM UTC by comment:8
| milestone: | 1.7 → 1.9 |
|---|
Changed May 16, 2011 09:25PM UTC by comment:9
I believe Skaffen was correct. I have tested the change to sortable to check the parent.length() before updating the position of a removed element and it does seem to correct the issue. It does not appear to be a problem with droppable because calling remove on any element outside of the sortable from droppable's drop method works without the setTimeout workaround. I have submitted this bug fix in https://github.com/jquery/jquery-ui/pull/294
Changed May 23, 2011 03:55PM UTC by comment:10
Fiddled vs. git builds: http://jsfiddle.net/gnarf/Amyez/
Changed May 25, 2011 08:12PM UTC by comment:11
| resolution: | → fixed |
|---|---|
| status: | reopened → closed |
Merge pull request #294 from kborchers/bug_4088
Sortable: Changed to check the parent's length so that the dom position of the removed element is not updated. Fixed #4088 - Unable to remove() ui.draggable (sortable item) immediately after the drop callback.
Changeset: fe4ae3045810421d2489175739098deb6852417b
Changed May 25, 2011 08:13PM UTC by comment:12
Sortable: Changed to check the parent's length so that the dom position of the removed element is not updated. Fixed #4088 - Unable to remove() ui.draggable (sortable item) immediately after the drop callback.
(cherry picked from commit 8e8a7b015f4f5a76c187dfca9af7519ae356bb16)
Changeset: ed65ce7a14882df12d2e029a4be84680a8b68240
Changed May 25, 2011 08:13PM UTC by comment:13
| milestone: | 1.9 → 1.8.14 |
|---|