Merge branch 't/12515'

CHANGES.md

@@ -16,6 +16,7 @@ Fixed Issues:
 * [#12546](http://dev.ckeditor.com/ticket/12546): Fixed: Preview tab in docprops dialog is always disabled.
 * [#12300](http://dev.ckeditor.com/ticket/12300): Fixed: The [`editor.change`](http://docs.ckeditor.com/#!/api/CKEDITOR.editor-event-change) event fired on first navigation key press after typing.
 * [#12141](http://dev.ckeditor.com/ticket/12141): Fixed: List items are lost when indenting a list item with a content wrapped with a block element.
+* [#12515](http://dev.ckeditor.com/ticket/12515): Fixed: Cursor is in the wrong position when executing undo command after adding image and typing some text.
 
 Other Changes:
 * [#12550](http://dev.ckeditor.com/ticket/12550): Added CKEDITOR.dtd.main.

core/dom/node.js

@@ -265,18 +265,24 @@ CKEDITOR.tools.extend( CKEDITOR.dom.node.prototype, {
 
 	/**
 	 * Gets the index of a node in an array of its `parent.childNodes`.
+	 * Returns `-1` if node does not have a parent or when `normalized` argument is set to `true`
+	 * and the text node is empty and will be removed while normalization.
 	 *
 	 * Let us assume having the following `childNodes` array:
 	 *
-	 *		[ emptyText, element1, text, text, element2 ]
-	 *		element1.getIndex();		// 1
-	 *		element1.getIndex( true );	// 0
-	 *		element2.getIndex();		// 4
-	 *		element2.getIndex( true );	// 2
+	 *		[ emptyText, element1, text, text, element2, emptyText2 ]
 	 *
-	 * @param {Boolean} normalized When `true`, empty text nodes and one followed
-	 * by another one text node are not counted in.
-	 * @returns {Number} Index of a node.
+	 *		emptyText.getIndex()			// 0
+	 *		emptyText.getIndex( true )		// -1
+	 *		element1.getIndex();			// 1
+	 *		element1.getIndex( true );		// 0
+	 *		element2.getIndex();			// 4
+	 *		element2.getIndex( true );		// 2
+	 *		emptyText2.getIndex();			// 5
+	 *		emptyText2.getIndex( true );	// -1
+	 *
+	 * @param {Boolean} normalized When `true`, adjacent text nodes are merged and empty text nodes are removed.
+	 * @returns {Number} Index of a node or `-1` if node does not have a parent or is removed while normalization.
 	 */
 	getIndex: function( normalized ) {
 		// Attention: getAddress depends on this.$
@@ -287,7 +293,17 @@ CKEDITOR.tools.extend( CKEDITOR.dom.node.prototype, {
 			isNormalizing;
 
 		if ( !this.$.parentNode )
-			return index;
+			return -1;
+
+		// The idea is - all empty text nodes will be virtually merged into their adjacent text nodes.
+		// If an empty text node does not have an adjacent non-empty text node we can return -1 straight away,
+		// because it and all its sibling text nodes will be merged into an empty text node and then totally ignored.
+		if ( normalized && current.nodeType == CKEDITOR.NODE_TEXT && !current.nodeValue ) {
+			var adjacent = getAdjacentNonEmptyTextNode( current ) || getAdjacentNonEmptyTextNode( current, true );
+
+			if ( !adjacent )
+				return -1;
+		}
 
 		do {
 			// Bypass blank node and adjacent text nodes.
@@ -300,6 +316,18 @@ CKEDITOR.tools.extend( CKEDITOR.dom.node.prototype, {
 		while ( ( current = current.previousSibling ) );
 
 		return index;
+
+		function getAdjacentNonEmptyTextNode( node, lookForward ) {
+			var sibling = lookForward ? node.nextSibling : node.previousSibling;
+
+			if ( !sibling || sibling.nodeType != CKEDITOR.NODE_TEXT ) {
+				return null;
+			}
+
+			// If found a non-empty text node, then return it.
+			// If not, then continue search.
+			return sibling.nodeValue ? sibling : getAdjacentNonEmptyTextNode( sibling, lookForward );
+		}
 	},
 
 	/**

core/dom/range.js

@@ -624,6 +624,8 @@ CKEDITOR.dom.range = function( root ) {
 		 * @returns {Boolean} return.is2 This is "bookmark2".
 		 */
 		createBookmark2: ( function() {
+			var isNotText = CKEDITOR.dom.walker.nodeType( CKEDITOR.NODE_TEXT, true );
+
 			// Returns true for limit anchored in element and placed between text nodes.
 			//
 			//               v
@@ -672,8 +674,43 @@ CKEDITOR.dom.range = function( root ) {
 
 				// The last step - fix the offset inside text node by adding
 				// lengths of preceding text nodes which will be merged with container.
-				if ( container.type == CKEDITOR.NODE_TEXT )
-					offset += getLengthOfPrecedingTextNodes( container );
+				if ( container.type == CKEDITOR.NODE_TEXT ) {
+					var precedingLength = getLengthOfPrecedingTextNodes( container );
+
+					// Normal case - text node is not empty.
+					if ( container.getText() ) {
+						offset += precedingLength;
+
+					// Awful case - the text node is empty and thus will be totally lost.
+					// In this case we are trying to normalize the limit to the left:
+					// * either to the preceding text node,
+					// * or to the "gap" after the preceding element.
+					} else {
+						// Find the closest non-text sibling.
+						var precedingContainer = container.getPrevious( isNotText );
+
+						// If there are any characters on the left, that means that we can anchor
+						// there, because this text node will not be lost.
+						if ( precedingLength ) {
+							offset = precedingLength;
+
+							if ( precedingContainer ) {
+								// The text node is the first node after the closest non-text sibling.
+								container = precedingContainer.getNext();
+							} else {
+								// But if there was no non-text sibling, then the text node is the first child.
+								container = container.getParent().getFirst();
+							}
+
+						// If there are no characters on the left, then anchor after the previous non-text node.
+						// E.g. (see tests for a legend :D):
+						// <b>x</b>(foo)({}bar) -> <b>x</b>[](foo)(bar)
+						} else {
+							container = container.getParent();
+							offset = precedingContainer ? ( precedingContainer.getIndex( true ) + 1 ) : 0;
+						}
+					}
+				}
 
 				limit.container = container;
 				limit.offset = offset;