From e2a693ba78be5a8d9bbe0b55e48d82860a1bbdc3 Mon Sep 17 00:00:00 2001 From: awgy Date: Wed, 3 Nov 2010 02:45:47 -0500 Subject: [PATCH] Mouse: tie the preventClickEvent property to the event target, not the container. Fixes #4752 - link event firing on sortable with connect list --- tests/jquery.simulate.js | 1 + tests/unit/sortable/sortable_tickets.js | 44 +++++++++++++++++++++++++ ui/jquery.ui.mouse.js | 10 ++++-- 3 files changed, 52 insertions(+), 3 deletions(-) diff --git a/tests/jquery.simulate.js b/tests/jquery.simulate.js index 3e7ca1090d5..81d7f62bf83 100644 --- a/tests/jquery.simulate.js +++ b/tests/jquery.simulate.js @@ -120,6 +120,7 @@ $.extend($.simulate.prototype, { this.simulateEvent(document, "mousemove", coord); this.simulateEvent(document, "mousemove", coord); this.simulateEvent(target, "mouseup", coord); + this.simulateEvent(target, "click", coord); }, findCenter: function(el) { var el = $(this.target), o = el.offset(); diff --git a/tests/unit/sortable/sortable_tickets.js b/tests/unit/sortable/sortable_tickets.js index e265c2f3a64..3edc8c04cfb 100644 --- a/tests/unit/sortable/sortable_tickets.js +++ b/tests/unit/sortable/sortable_tickets.js @@ -36,4 +36,48 @@ test("#3019: Stop fires too early", function() { }); +test('#4752: link event firing on sortable with connect list', function () { + var fired = {}, + hasFired = function (type) { return (type in fired) && (true === fired[type]); }; + + $('#sortable').clone().attr('id', 'sortable2').insertAfter('#sortable'); + + $('#main ul').sortable({ + connectWith: '#main ul', + change: function (e, ui) { + fired.change = true; + }, + receive: function (e, ui) { + fired.receive = true; + }, + remove: function (e, ui) { + fired.remove = true; + } + }); + + $('#main ul li').live('click.ui-sortable-test', function () { + fired.click = true; + }); + + $('#sortable li:eq(0)').simulate('click'); + ok(!hasFired('change'), 'Click only, change event should not have fired'); + ok(hasFired('click'), 'Click event should have fired'); + + // Drag an item within the first list + fired = {}; + $('#sortable li:eq(0)').simulate('drag', { dx: 0, dy: 40 }); + ok(hasFired('change'), '40px drag, change event should have fired'); + ok(!hasFired('receive'), 'Receive event should not have fired'); + ok(!hasFired('remove'), 'Remove event should not have fired'); + ok(!hasFired('click'), 'Click event should not have fired'); + + // Drag an item from the first list to the second, connected list + fired = {}; + $('#sortable li:eq(0)').simulate('drag', { dx: 0, dy: 150 }); + ok(hasFired('change'), '150px drag, change event should have fired'); + ok(hasFired('receive'), 'Receive event should have fired'); + ok(hasFired('remove'), 'Remove event should have fired'); + ok(!hasFired('click'), 'Click event should not have fired'); +}); + })(jQuery); diff --git a/ui/jquery.ui.mouse.js b/ui/jquery.ui.mouse.js index d0c82f83f2d..bfe8640a2a3 100644 --- a/ui/jquery.ui.mouse.js +++ b/ui/jquery.ui.mouse.js @@ -26,8 +26,8 @@ $.widget("ui.mouse", { return self._mouseDown(event); }) .bind('click.'+this.widgetName, function(event) { - if(self._preventClickEvent) { - self._preventClickEvent = false; + if (true === $.data(event.target, self.widgetName + '.preventClickEvent')) { + $.removeData(event.target, self.widgetName + '.preventClickEvent'); event.stopImmediatePropagation(); return false; } @@ -118,7 +118,11 @@ $.widget("ui.mouse", { if (this._mouseStarted) { this._mouseStarted = false; - this._preventClickEvent = (event.target == this._mouseDownEvent.target); + + if (event.target == this._mouseDownEvent.target) { + $.data(event.target, this.widgetName + '.preventClickEvent', true); + } + this._mouseStop(event); }