Merge branch 'dmrewrite' of ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/VisualEditor into dmrewrite

modules/ve2/ve.Document.js

@@ -45,9 +45,13 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 		start = range.start,
 		end = range.end,
 		stack = [ {
+			// Node we are currently stepping through
+			// Note each iteration visits a child of node, not node itself
 			'node': doc,
+			// Index of the child in node we're visiting
 			'index': 0,
-			'startOffset': 0
+			// First offset inside node
+			'startOffset': 1
 		} ],
 		node,
 		prevNode,
@@ -80,7 +84,7 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			'node': doc,
 			'range': new ve.Range( start, end ),
 			'index': 0,
-			'parentRange': new ve.Range( 0, doc.getLength() )
+			'nodeRange': new ve.Range( 0, doc.getLength() )
 		} ];
 	}
 	// TODO maybe we could find the start more efficiently using the offset map
@@ -100,7 +104,7 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 		// Is the end between node and nextNode or between node and the parent's closing?
 		endBetween = node.isWrapped() ? end == right + 1 : end == right;
 
-		if ( start == end && ( startBetween || endBetween ) ) {
+		if ( start == end && ( startBetween || endBetween ) && node.isWrapped() ) {
 			// Empty range in the parent, outside of any child
 			parentFrame = stack[stack.length - 2];
 			return [ {
@@ -117,18 +121,18 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			
 			if ( mode == 'leaves' && node.children && node.children.length ) {
 				// Descend into node
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				startFound = true;
 				continue;
-			} else {
+			} else if ( !endInside ) {
 				// All of node is covered
 				retval.push( {
 					'node': node,
@@ -137,26 +141,34 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 					'nodeRange': new ve.Range( left, right )
 				} );
 				startFound = true;
+			} else {
+				// Part of node is covered
+				return [ {
+					'node': node,
+					'range': new ve.Range( start, end ),
+					'index': currentFrame.index,
+					'nodeRange': new ve.Range( left, right )
+				} ];
 			}
 		} else if ( startInside && endInside ) {
 			if ( node.children && node.children.length ) {
 				// Descend into node
+				// If the first child of node has an opening, skip over it
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				// If the first child of node has an opening, skip over it
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				continue;
 			} else {
 				// node is a leaf node and the range is entirely inside it
 				return [ {
 					'node': node,
-					'range': new ve.Range( left, right ),
+					'range': new ve.Range( start, end ),
 					'index': currentFrame.index,
 					'nodeRange': new ve.Range( left, right )
 				} ];
@@ -165,15 +177,15 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			if ( mode == 'leaves' && node.children && node.children.length ) {
 				// node is a branch node and the start is inside it
 				// Descend into it
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				continue;
 			} else {
 				// node is a leaf node and the start is inside it
@@ -194,15 +206,15 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			
 			if ( mode == 'leaves' && node.children && node.children.length ) {
 				// Descend into node
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				continue;
 			} else {
 				// All of node is covered
@@ -218,15 +230,15 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			if ( mode == 'leaves' && node.children && node.children.length ) {
 				// node is a branch node and the end is inside it
 				// Descend into it
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				continue;
 			} else {
 				// node is a leaf node and the end is inside it
@@ -247,15 +259,15 @@ ve.Document.prototype.selectNodes = function( range, mode ) {
 			
 			if ( mode == 'leaves' && node.children && node.children.length ) {
 				// Descend into node
+				if ( node.children[0].isWrapped() ) {
+					left++;
+				}
 				currentFrame = {
 					'node': node,
 					'index': 0,
 					'startOffset': left
 				};
 				stack.push( currentFrame );
-				if ( node.children[0].isWrapped() ) {
-					left++;
-				}
 				continue;
 			} else {
 				// All of node is covered

tests/ve2/ve.example.js

@@ -115,6 +115,45 @@ ve.example.getSelectNodesCases = function( doc ) {
 					'nodeRange': new ve.Range( 42, 52 )
 				}
 			]
+		},
+		// Zero-length range at the edge of a text node returns that text node rather than
+		// its parent
+		{
+			'actual': doc.selectNodes( new ve.Range( 1, 1 ), 'leaves' ),
+			'expected': [
+				// heading/text
+				{
+					'node': lookup( documentNode, 0, 0 ),
+					'range': new ve.Range( 1, 1 ),
+					'index': 0,
+					'nodeRange': new ve.Range( 1, 4 )
+				}
+			]
+		},
+		{
+			'actual': doc.selectNodes( new ve.Range( 4, 4 ), 'leaves' ),
+			'expected': [
+				// heading/text
+				{
+					'node': lookup( documentNode, 0, 0 ),
+					'range': new ve.Range( 4, 4 ),
+					'index': 0,
+					'nodeRange': new ve.Range( 1, 4 )
+				}
+			]
+		},
+		// Range entirely within one leaf node
+		{
+			'actual': doc.selectNodes( new ve.Range( 2, 3 ), 'leaves' ),
+			'expected': [
+				// heading/text
+				{
+					'node': lookup( documentNode, 0, 0 ),
+					'range': new ve.Range( 2, 3 ),
+					'index': 0,
+					'nodeRange': new ve.Range( 1, 4 )
+				}
+			]
 		}
 	];
 };
@@ -147,8 +186,9 @@ ve.example.nodeTreeEqual = function( a, b ) {
  * @method
  */
 ve.example.nodeSelectionEqual = function( a, b ) {
+	var minLength = a.length < b.length ? a.length : b.length;
 	equal( a.length, b.length, 'length match' );
-	for ( var i = 0; i < a.length; i++ ) {
+	for ( var i = 0; i < minLength; i++ ) {
 		ok( a[i].node === b[i].node, 'node match' );
 		if ( a[i].range && b[i].range ) {
 			deepEqual( a[i].range, b[i].range, 'range match' );