Opened 14 years ago

Closed 13 years ago

Last modified 12 years ago

#483 closed enhancement (fixed)

Fixes to drag-and-drop (DnD) offset problems

Reported by: Martin J. Conlon <mconlon@…> Owned by: alex
Priority: high Milestone:
Component: General Version: 0.2
Keywords: drag drop offset Cc: mconlon@…
Blocked By: Blocking:

Description

There are multiple open tickets (#433, #452, poss. #293) which report drag-and-drop problems due to scrolling of parent divs, etc. Most of these problems seem due to inconsistent position calculations in the DnD source code. I have updated the source (diffs included below) to account for scrolling of parent elements. I hope this cures most of the problems.

There is one new utility function called dojo.html.getCursorPosition. Most changes to the source involve calls to getAbsoluteX and getAbsoluteY, which were most often not setting the includeScroll parameter to "true". Couple of spelling changes in the comments, references to dojo.html.toCoordinateArray changed to dojo.style.toCoordinateArray (which seems to be where the function lives now), and that's about it.

Martin

---8<---[begin dojo/src/html.js diff]---
--- trunk/src/html.js	Mon Feb 27 22:05:52 2006
+++ dojo/src/html.js	Wed Mar  1 19:36:33 2006
@@ -461,6 +461,24 @@
 }
 dojo.html.getElementsByClassName = dojo.html.getElementsByClass;
 
+dojo.html.getCursorPosition = function(e) {
+	e = e || window.event;
+	var cursor = {x:0, y:0};
+	if (e.pageX || e.pageY) {
+		cursor.x = e.pageX;
+		cursor.y = e.pageY;
+	}
+	else {
+		cursor.x = e.clientX +
+			(document.documentElement.scrollLeft || document.body.scrollLeft) -
+			document.documentElement.clientLeft;
+		cursor.y = e.clientY +
+			(document.documentElement.scrollTop || document.body.scrollTop) -
+			document.documentElement.clientTop;
+	}
+	return cursor;
+}
+
 /**
  * Calculates the mouse's direction of gravity relative to the centre
  * of the given node.
@@ -479,17 +497,16 @@
  */
 dojo.html.gravity = function(node, e){
 	node = dojo.byId(node);
-	var mousex = e.pageX || e.clientX + document.body.scrollLeft;
-	var mousey = e.pageY || e.clientY + document.body.scrollTop;
-	
+	var mouse = dojo.html.getCursorPosition(e);
+
 	with (dojo.html) {
-		var nodecenterx = getAbsoluteX(node) + (getInnerWidth(node) / 2);
-		var nodecentery = getAbsoluteY(node) + (getInnerHeight(node) / 2);
+		var nodecenterx = getAbsoluteX(node, true) + (getInnerWidth(node) / 2);
+		var nodecentery = getAbsoluteY(node, true) + (getInnerHeight(node) / 2);
 	}
 	
 	with (dojo.html.gravity) {
-		return ((mousex < nodecenterx ? WEST : EAST) |
-			(mousey < nodecentery ? NORTH : SOUTH));
+		return ((mouse.x < nodecenterx ? WEST : EAST) |
+			(mouse.y < nodecentery ? NORTH : SOUTH));
 	}
 }
 
@@ -500,18 +517,17 @@
 	
 dojo.html.overElement = function(element, e){
 	element = dojo.byId(element);
-	var mousex = e.pageX || e.clientX + document.body.scrollLeft;
-	var mousey = e.pageY || e.clientY + document.body.scrollTop;
-	
+	var mouse = dojo.html.getCursorPosition(e);
+
 	with(dojo.html){
-		var top = getAbsoluteY(element);
+		var top = getAbsoluteY(element, true);
 		var bottom = top + getInnerHeight(element);
-		var left = getAbsoluteX(element);
+		var left = getAbsoluteX(element, true);
 		var right = left + getInnerWidth(element);
 	}
 	
-	return (mousex >= left && mousex <= right &&
-		mousey >= top && mousey <= bottom);
+	return (mouse.x >= left && mouse.x <= right &&
+		mouse.y >= top && mouse.y <= bottom);
 }
 
 /**
@@ -936,7 +952,7 @@
 	size: function(node) {
 		if(!this.iframe) { return; }
 
-		coords = dojo.html.toCoordinateArray(node, true);
+		coords = dojo.style.toCoordinateArray(node, true);
 
 		var s = this.iframe.style;
 		s.width = coords.w + "px";
---8<---[end dojo/src/html.js diff]---



---8<---[begin dojo/src/style.js diff]---
--- trunk/src/style.js	Tue Feb 28 12:18:19 2006
+++ dojo/src/style.js	Tue Feb 28 12:33:44 2006
@@ -274,7 +274,7 @@
 	var offset = 0;
 	if(node["offsetParent"]){
 
-		// in Safari, if the node is an absolutly positioned child of the body
+		// in Safari, if the node is an absolutely positioned child of the body
 		// and the body has a margin the offset of the child and the body
 		// contain the body's margins, so we need to end at the body
 		if (dojo.render.html.safari
@@ -290,7 +290,7 @@
 			offset -= dojo.style.sumAncestorProperties(node, typeScroll);
 		}
 		// FIXME: this is known not to work sometimes on IE 5.x since nodes
-		// soemtimes need to be "tickled" before they will display their
+		// sometimes need to be "tickled" before they will display their
 		// offset correctly
 		do {
 			var n = node[typeStr];
@@ -301,6 +301,11 @@
 		var n = node[coord];
 		offset += isNaN(n) ? 0 : n;
 	}
+	
+	// Account for any document scrolling
+	//dojo.debug("Before: "+offset);
+	offset += (type=="top") ? dojo.html.getScrollTop() : dojo.html.getScrollLeft();
+	//dojo.debug("After: "+offset);
 	return offset;
 }
---8<---[end dojo/src/style.js diff]---



---8<---[begin dojo/src/dnd/HtmlDragAndDrop.js diff]---
--- trunk/src/dnd/HtmlDragAndDrop.js	Mon Feb 27 22:05:52 2006
+++ dojo/src/dnd/HtmlDragAndDrop.js	Thu Mar  2 12:20:36 2006
@@ -99,17 +99,19 @@
 	onDragStart: function(e){
 		dojo.html.clearSelection();
 
+		var mouse = dojo.html.getCursorPosition(e);
+
 		this.scrollOffset = {
-			top: dojo.html.getScrollTop(), // document.documentElement.scrollTop,
-			left: dojo.html.getScrollLeft() // document.documentElement.scrollLeft
+			top: dojo.html.getScrollTop(),
+			left: dojo.html.getScrollLeft()
 		};
 
-		this.dragStartPosition = {top: dojo.style.getAbsoluteY(this.domNode, true) + this.scrollOffset.top,
-			left: dojo.style.getAbsoluteX(this.domNode, true) + this.scrollOffset.left};
+		this.dragStartPosition = {top: dojo.style.getAbsoluteY(this.domNode, true),
+			left: dojo.style.getAbsoluteX(this.domNode, true)};
 
 
-		this.dragOffset = {top: this.dragStartPosition.top - e.clientY,
-			left: this.dragStartPosition.left - e.clientX};
+		this.dragOffset = {top: this.dragStartPosition.top - mouse.y,
+			left: this.dragStartPosition.left - mouse.x};
 
 		this.dragClone = this.createDragNode();
 
@@ -118,7 +120,7 @@
 			this.parentPosition = {top: 0, left: 0};
 		} else {
 			this.parentPosition = {top: dojo.style.getAbsoluteY(this.domNode.parentNode, true),
-				left: dojo.style.getAbsoluteX(this.domNode.parentNode,true)};
+				left: dojo.style.getAbsoluteX(this.domNode.parentNode, true)};
 		}
 
 		if (this.constrainToContainer) {
@@ -128,8 +130,8 @@
 		// set up for dragging
 		with(this.dragClone.style){
 			position = "absolute";
-			top = this.dragOffset.top + e.clientY + "px";
-			left = this.dragOffset.left + e.clientX + "px";
+			top = this.dragOffset.top + mouse.y + "px";
+			left = this.dragOffset.left + mouse.x + "px";
 		}
 
 		document.body.appendChild(this.dragClone);
@@ -158,8 +160,8 @@
 	},
 
 	updateDragOffset: function() {
-		var sTop = dojo.html.getScrollTop(); // document.documentElement.scrollTop;
-		var sLeft = dojo.html.getScrollLeft(); // document.documentElement.scrollLeft;
+		var sTop = dojo.html.getScrollTop();
+		var sLeft = dojo.html.getScrollLeft();
 		if(sTop != this.scrollOffset.top) {
 			var diff = sTop - this.scrollOffset.top;
 			this.dragOffset.top += diff;
@@ -169,9 +171,10 @@
 
 	/** Moves the node to follow the mouse */
 	onDragMove: function(e){
+		var mouse = dojo.html.getCursorPosition(e);
 		this.updateDragOffset();
-		var x = this.dragOffset.left + e.clientX - this.parentPosition.left;
-		var y = this.dragOffset.top + e.clientY - this.parentPosition.top;
+		var x = this.dragOffset.left + mouse.x;
+		var y = this.dragOffset.top + mouse.y;
 
 		if (this.constrainToContainer) {
 			if (x < this.constraints.minX) { x = this.constraints.minX; }
@@ -199,8 +202,8 @@
 				break;
 
 			case "dropFailure": // slide back to the start
-				var startCoords = [dojo.style.getAbsoluteX(this.dragClone),
-							dojo.style.getAbsoluteY(this.dragClone)];
+				var startCoords = [dojo.style.getAbsoluteX(this.dragClone, true),
+							dojo.style.getAbsoluteY(this.dragClone, true)];
 				// offset the end so the effect can be seen
 				var endCoords = [this.dragStartPosition.left + 1,
 					this.dragStartPosition.top + 1];
@@ -265,9 +268,9 @@
 		for (var i = 0, child; i < this.domNode.childNodes.length; i++) {
 			child = this.domNode.childNodes[i];
 			if (child.nodeType != dojo.dom.ELEMENT_NODE) { continue; }
-			var top = dojo.style.getAbsoluteY(child);
+			var top = dojo.style.getAbsoluteY(child, true);
 			var bottom = top + dojo.style.getInnerHeight(child);
-			var left = dojo.style.getAbsoluteX(child);
+			var left = dojo.style.getAbsoluteX(child, true);
 			var right = left + dojo.style.getInnerWidth(child);
 			this.childBoxes.push({top: top, bottom: bottom,
 				left: left, right: right, node: child});
@@ -279,14 +282,13 @@
 	},
 
 	_getNodeUnderMouse: function(e){
-		var mousex = e.pageX || e.clientX + document.body.scrollLeft;
-		var mousey = e.pageY || e.clientY + document.body.scrollTop;
+		var mouse = dojo.html.getCursorPosition(e);
 
 		// find the child
 		for (var i = 0, child; i < this.childBoxes.length; i++) {
 			with (this.childBoxes[i]) {
-				if (mousex >= left && mousex <= right &&
-					mousey >= top && mousey <= bottom) { return i; }
+				if (mouse.x >= left && mouse.x <= right &&
+					mouse.y >= top && mouse.y <= bottom) { return i; }
 			}
 		}
 
@@ -302,7 +304,7 @@
 			borderTopColor = "black";
 			borderTopStyle = "solid";
 			width = dojo.style.getInnerWidth(this.domNode) + "px";
-			left = dojo.style.getAbsoluteX(this.domNode) + "px";
+			left = dojo.style.getAbsoluteX(this.domNode, true) + "px";
 		}
 	},
 
@@ -337,7 +339,7 @@
 					top = (before ? this.childBoxes[0].top
 						: this.childBoxes[this.childBoxes.length - 1].bottom) + "px";
 				} else {
-					top = dojo.style.getAbsoluteY(this.domNode) + "px";
+					top = dojo.style.getAbsoluteY(this.domNode, true) + "px";
 				}
 			} else {
 				var child = this.childBoxes[boxIndex];
---8<---[end dojo/src/dnd/HtmlDragAndDrop.js diff]---



---8<---[begin dojo/src/dnd/HtmlDragManager.js diff]---
--- trunk/src/dnd/HtmlDragManager.js	Mon Feb 27 22:05:52 2006
+++ dojo/src/dnd/HtmlDragManager.js	Wed Mar  1 19:31:04 2006
@@ -146,8 +146,9 @@
 			return;
 		}
 
-		this.mouseDownX = e.clientX;
-		this.mouseDownY = e.clientY;
+		var mouse = dojo.html.getCursorPosition(e);
+		this.mouseDownX = mouse.x;
+		this.mouseDownY = mouse.y;
 
 		var target = e.target.nodeType == dojo.dom.TEXT_NODE ?
 			e.target.parentNode : e.target;
@@ -178,7 +179,6 @@
 		this.mouseDownY = null;
 		this._dragTriggered = false;
 		var _this = this;
-		e.preventDefault();
 		e.dragSource = this.dragSource;
 		if((!e.shiftKey)&&(!e.ctrlKey)){
 			if(_this.currentDropTarget) {
@@ -271,6 +271,7 @@
 
 	onMouseMove: function(e){
 		var _this = this;
+		var mouse = dojo.html.getCursorPosition(e);
 		// if we've got some sources, but no drag objects, we need to send
 		// onDragStart to all the right parties and get things lined up for
 		// drop target detection
@@ -279,10 +280,10 @@
 			var dx;
 			var dy;
 			if(!this._dragTriggered){
-				this._dragTriggered = (this._dragStartDistance(e.clientX, e.clientY) > this.threshold);
+				this._dragTriggered = (this._dragStartDistance(mouse.x, mouse.y) > this.threshold);
 				if(!this._dragTriggered){ return; }
-				dx = e.clientX-this.mouseDownX;
-				dy = e.clientY-this.mouseDownY;
+				dx = mouse.x - this.mouseDownX;
+				dy = mouse.y - this.mouseDownY;
 			}
 
 			if (this.selectedSources.length == 1) {
@@ -318,7 +319,7 @@
 		// it. If so, do all the actions that need doing.
 		if (this.currentDropTarget) {
 			//dojo.debug(dojo.dom.hasParent(this.currentDropTarget.domNode))
-			var c = dojo.html.toCoordinateArray(this.currentDropTarget.domNode);
+			var c = dojo.style.toCoordinateArray(this.currentDropTarget.domNode, true);
 			//		var dtp = this.currentDropTargetPoints;
 			var dtp = [
 				[c[0],c[1]], [c[0]+c[2], c[1]+c[3]]
@@ -381,10 +382,11 @@
 	},
 
 	isInsideBox: function(e, coords){
-		if(	(e.clientX > coords[0][0])&&
-			(e.clientX < coords[1][0])&&
-			(e.clientY > coords[0][1])&&
-			(e.clientY < coords[1][1]) ){
+		var mouse = dojo.html.getCursorPosition(e);
+		if(	(mouse.x > coords[0][0])&&
+			(mouse.x < coords[1][0])&&
+			(mouse.y > coords[0][1])&&
+			(mouse.y < coords[1][1]) ){
 			return true;
 		}
 		return false;
---8<---[end dojo/src/dnd/HtmlDragManager.js diff]---



---8<---[begin dojo/src/dnd/HtmlDragMove.js]---
--- trunk/src/dnd/HtmlDragMove.js	Mon Feb 27 22:05:51 2006
+++ dojo/src/dnd/HtmlDragMove.js	Wed Mar  1 19:30:48 2006
@@ -36,7 +36,9 @@
 	onDragStart: function(e){
 
 		dojo.html.clearSelection();
-		
+
+		var mouse = dojo.html.getCursorPosition(e);
+
 		this.dragClone = this.domNode;
 
 		this.scrollOffset = {
@@ -44,17 +46,17 @@
 			left: dojo.html.getScrollLeft() // document.documentElement.scrollLeft
 		};
 
-		this.dragStartPosition = {top: dojo.style.getAbsoluteY(this.domNode) ,
-			left: dojo.style.getAbsoluteX(this.domNode) };
+		this.dragStartPosition = {top: dojo.style.getAbsoluteY(this.domNode, true),
+			left: dojo.style.getAbsoluteX(this.domNode, true) };
 		
-		this.dragOffset = {top: this.dragStartPosition.top - e.clientY,
-			left: this.dragStartPosition.left - e.clientX};
+		this.dragOffset = {top: this.dragStartPosition.top - mouse.y,
+			left: this.dragStartPosition.left - mouse.x};
 
 		if (this.domNode.parentNode.nodeName.toLowerCase() == 'body') {
 			this.parentPosition = {top: 0, left: 0};
 		} else {
 			this.parentPosition = {top: dojo.style.getAbsoluteY(this.domNode.parentNode, true),
-				left: dojo.style.getAbsoluteX(this.domNode.parentNode,true)};
+				left: dojo.style.getAbsoluteX(this.domNode.parentNode, true)};
 		}
 
 		this.dragClone.style.position = "absolute";
---[end dojo/src/dnd/HtmlDragMove.js diff]---

That's all, folks. ;)

Attachments (2)

dnd.patch (13.3 KB) - added by Martin J. Conlon <mconlon@…> 14 years ago.
SVN patch
dnd.2.patch (13.3 KB) - added by mi-dojo@… 14 years ago.
Updated patch (truly) against 3239

Download all attachments as: .zip

Change History (7)

Changed 14 years ago by Martin J. Conlon <mconlon@…>

Attachment: dnd.patch added

SVN patch

Changed 14 years ago by mi-dojo@…

Attachment: dnd.2.patch added

Updated patch (truly) against 3239

comment:1 Changed 13 years ago by alex

Resolution: fixed
Status: newclosed

merged in [3432]

Thanks for the patch!

comment:2 Changed 13 years ago by bill

Cc: mconlon@… added
Resolution: fixed
Status: closedreopened

I think this patch might need a patch. I just tried test_FloatingPane.html. If you scroll down on the page and then drag one of the floating panes, the pane jumps towards the bottom on drag start.

Martin, care to take a look?

comment:3 Changed 13 years ago by bill

Resolution: fixed
Status: reopenedclosed

OK, I fixed the DnD problems in rev 3679, 3680.

comment:4 Changed 13 years ago by bill

PS: actually I should say that Martin's patch to DnD was OK; the problem was in dojo.style.getAbsolutePosition().

comment:5 Changed 12 years ago by (none)

Milestone: 0.3release

Milestone 0.3release deleted

Note: See TracTickets for help on using tickets.