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

[Martin J. Conlon <mconlon@…>]

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. ;)

[Martin J. Conlon <mconlon@…>]

SVN patch

[mi-dojo@…]

Updated patch (truly) against 3239

[alex]

merged in [3432]

Thanks for the patch!

[bill]

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?

[bill]

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

[bill]

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

[(none)]

Milestone 0.3release deleted