From 937607893c0a2261e1d05737b7c1f0b548b5282c Mon Sep 17 00:00:00 2001
From: Catrope <roan.kattouw@gmail.com>
Date: Wed, 17 Oct 2012 16:09:49 -0700
Subject: [PATCH] Fix selectNodes() bug reported by Inez

For <p>1<br/>2</p>, selectNodes([2,2]) correctly returned the end of the
first text node, but selectNodes([4,4]) returned index 2 in the
paragraph (i.e. between the break node and the second text node). The
correct behavior is to return the start of the second text node, i.e.
the mirror image of the behavior for [2,2].

Fixed this by applying the startBetween/endBetween logic only if the
relevant adjacent node is wrapped (or if it's missing). In the code,
this is expressed as !(adjacent node present && adjacent node wrapped).

Change-Id: Ie3b7fdf1de38ee253a798a7a73bc89734f4ca4fa
---
 modules/ve/test/ve.example.js | 28 ++++++++++++++++++++++++++++
 modules/ve/ve.Document.js     | 14 +++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/modules/ve/test/ve.example.js b/modules/ve/test/ve.example.js
index b9098b81dc..a2900487bd 100644
--- a/modules/ve/test/ve.example.js
+++ b/modules/ve/test/ve.example.js
@@ -413,6 +413,34 @@ ve.example.getSelectNodesCases = function ( doc ) {
 				}
 			],
 			'msg': 'range covering a list closing and a list opening'
+		},
+		{
+			'actual': doc.selectNodes( new ve.Range( 39, 39 ), 'leaves' ),
+			'expected': [
+				// preformatted/text
+				{
+					'node': lookup( documentNode, 2, 0 ),
+					'range': new ve.Range( 39, 39 ),
+					'index': 0,
+					'nodeRange': new ve.Range( 38, 39 ),
+					'nodeOuterRange': new ve.Range( 38, 39 )
+				}
+			],
+			'msg': 'zero-length range in text node before inline node'
+		},
+		{
+			'actual': doc.selectNodes( new ve.Range( 41, 41 ), 'leaves' ),
+			'expected': [
+				// preformatted/text
+				{
+					'node': lookup( documentNode, 2, 2 ),
+					'range': new ve.Range( 41, 41 ),
+					'index': 2,
+					'nodeRange': new ve.Range( 41, 42 ),
+					'nodeOuterRange': new ve.Range( 41, 42 )
+				}
+			],
+			'msg': 'zero-length range in text node after inline node'
 		}
 	];
 };
diff --git a/modules/ve/ve.Document.js b/modules/ve/ve.Document.js
index 0360fc676e..97e6432bdb 100644
--- a/modules/ve/ve.Document.js
+++ b/modules/ve/ve.Document.js
@@ -120,10 +120,14 @@ ve.Document.prototype.selectNodes = function ( range, mode ) {
 		endInside = end >= left && end <= right;
 		// Does the node have wrapping elements around it
 		isWrapped = node.isWrapped();
-		// Is the start between prevNode and node or between the parent's opening and node?
-		startBetween = isWrapped ? start === left - 1 : start === left;
-		// Is the end between node and nextNode or between node and the parent's closing?
-		endBetween = isWrapped ? end === right + 1 : end === right;
+		// Is there an unwrapped node right before this node?
+		isPrevUnwrapped = prevNode ? !prevNode.isWrapped() : false;
+		// Is there an unwrapped node right after this node?
+		isNextUnwrapped = nextNode ? !nextNode.isWrapped() : false;
+		// Is the start between prevNode's closing and node or between the parent's opening and node?
+		startBetween = ( isWrapped ? start === left - 1 : start === left ) && !isPrevUnwrapped;
+		// Is the end between node and nextNode's opening or between node and the parent's closing?
+		endBetween = ( isWrapped ? end === right + 1 : end === right ) && !isNextUnwrapped;
 
 		if ( isWrapped && end === left - 1 && currentFrame.index === 0 ) {
 			// The selection ends here with an empty range at the beginning of the node
@@ -148,7 +152,7 @@ ve.Document.prototype.selectNodes = function ( range, mode ) {
 			return retval;
 		}
 
-		if ( start === end && ( startBetween || endBetween ) && node.isWrapped() ) {
+		if ( start === end && ( startBetween || endBetween ) && isWrapped ) {
 			// Empty range in the parent, outside of any child
 			nodeRange = new ve.Range( currentFrame.startOffset,
 				currentFrame.startOffset + currentFrame.node.getLength()