Fixing #3608: Introducing CKEDITOR.dom.walker.bookmark to skip bookmark nodes and optimize range::enlage to avoid particial element selection.

git-svn-id: https://svn.ckeditor.com/CKEditor/trunk@3607 da63caf2-3823-0410-a1e8-d69ccee00dfb

Author: garry.yao

_source/core/dom/range.js

@@ -233,8 +233,13 @@ CKEDITOR.dom.range = function( document ) {
 	// Creates the appropriate node evaluator for the dom walker used inside
 	// check(Start|End)OfBlock.
 	function getCheckStartEndBlockEvalFunction( isStart ) {
-		var hadBr = false;
+		var hadBr = false,
+			bookmarkEvaluator = CKEDITOR.dom.walker.bookmark( true );
 		return function( node ) {
+			// First ignore bookmark nodes.
+			if ( bookmarkEvaluator( node ) )
+				return true;
+
 			if ( node.type == CKEDITOR.NODE_TEXT ) {
 				// If there's any visible text, then we're not at the start.
 				if ( CKEDITOR.tools.trim( node.getText() ).length )
@@ -975,34 +980,62 @@ CKEDITOR.dom.range = function( document ) {
 				case CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS:
 
 					// Enlarging the start boundary.
-					var walkerRange = new CKEDITOR.dom.range( this.document );
-					walkerRange.setStartAt( this.document.getBody(), CKEDITOR.POSITION_AFTER_START );
+					var walkerRange = new CKEDITOR.dom.range( this.document ),
+						body = this.document.getBody();
+					walkerRange.setStartAt( body, CKEDITOR.POSITION_AFTER_START );
 					walkerRange.setEnd( this.startContainer, this.startOffset );
 
 					var walker = new CKEDITOR.dom.walker( walkerRange ),
-
-						guard = CKEDITOR.dom.walker.blockBoundary(
+						blockBoundary, // The node on which the enlarging should stop.
+						tailBr, //
+						defaultGuard = CKEDITOR.dom.walker.blockBoundary(
 						( unit == CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS ) ? { br:1 } : null ),
-						tailBr,
-					listGuard = function( node ) {
-							var result = guard( node );
-							if ( !result && node.is && node.is( 'br' ) )
+						// Record the encountered 'blockBoundary' for later use.
+						boundaryGuard = function( node ) {
+							var retval = defaultGuard( node );
+							if ( !retval )
+								blockBoundary = node;
+							return retval;
+						},
+						// Record the encounted 'tailBr' for later use.
+						tailBrGuard = function( node ) {
+							var retval = boundaryGuard( node );
+							if ( !retval && node.is && node.is( 'br' ) )
 								tailBr = node;
-							return result;
+							return retval;
 						};
-					walker.guard = guard;
 
-					if ( ( enlargeable = walker.lastBackward() ) )
-						this.setStartAt( enlargeable, CKEDITOR.POSITION_BEFORE_START );
+					walker.guard = boundaryGuard;
+
+
+					if ( ( enlargeable = walker.lastBackward() ) ) {
+						// It's the body which stop the enlaring if no block boundary found.
+						blockBoundary = blockBoundary || body;
+
+						// Start the range at different position by comparing
+						// the document position of it with 'enlargeable' node.
+						this.setStartAt( blockBoundary, blockBoundary.contains( enlargeable ) ? CKEDITOR.POSITION_AFTER_START : CKEDITOR.POSITION_AFTER_END );
+					}
 
 					// Enlarging the end boundary.
 					walkerRange = this.clone();
 					walkerRange.collapse();
-					walkerRange.setEndAt( this.document.getBody(), CKEDITOR.POSITION_BEFORE_END );
+					walkerRange.setEndAt( body, CKEDITOR.POSITION_BEFORE_END );
 					walker = new CKEDITOR.dom.walker( walkerRange );
-					walker.guard = ( unit == CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS ) ? listGuard : guard;
-					if ( ( enlargeable = walker.lastForward() ) )
-						this.setEndAfter( enlargeable );
+
+					// tailBrGuard only used for on range end.
+					walker.guard = ( unit == CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS ) ? tailBrGuard : boundaryGuard;
+					blockBoundary = null;
+					// End the range right before the block boundary node.
+					;
+					if ( ( enlargeable = walker.lastForward() ) ) {
+						// It's the body which stop the enlaring if no block boundary found.
+						blockBoundary = blockBoundary || body;
+
+						// Start the range at different position by comparing
+						// the document position of it with 'enlargeable' node.
+						this.setEndAt( blockBoundary, blockBoundary.contains( enlargeable ) ? CKEDITOR.POSITION_BEFORE_END : CKEDITOR.POSITION_BEFORE_START );
+					}
 					// We must include the <br> at the end of range if there's
 					// one and we're expanding list item contents
 					if ( tailBr )

_source/core/dom/walker.js

@@ -293,5 +293,32 @@ For licensing, see LICENSE.html or http://ckeditor.com/license
 	CKEDITOR.dom.walker.listItemBoundary = function() {
 		return this.blockBoundary( { br:1 } );
 	};
+	/**
+	 * Whether the node is a bookmark node's inner text node.
+	 */
+	CKEDITOR.dom.walker.bookmarkContents = function( node ) {},
+
+	/**
+	 * Whether the to-be-evaluated node is a bookmark node OR bookmark node
+	 * inner contents.
+	 * @param {Boolean} contentOnly Whether only test againt the text content of
+	 * bookmark node instead of the element itself(default).
+	 * @param {Boolean} isReject Whether should return 'false' for the bookmark
+	 * node instead of 'true'(default).
+	 */
+	CKEDITOR.dom.walker.bookmark = function( contentOnly, isReject ) {
+		function isBookmarkNode( node ) {
+			return ( node && node.getName && node.getName() == 'span' && node.hasAttribute( '_fck_bookmark' ) );
+		}
+
+		return function( node ) {
+			var retval, parent;
+			// Is bookmark inner text node?
+			retval = ( node && !node.getName && ( parent = node.getParent() ) && isBookmarkNode( parent ) );
+			// Is bookmark node?
+			retval = contentOnly ? retval : retval || isBookmarkNode( node );
+			return isReject ? !retval : !!retval;
+		};
+	};
 
 })();

_source/plugins/domiterator/plugin.js

@@ -11,29 +11,6 @@ CKEDITOR.plugins.add( 'domiterator' );
 
 (function() {
 
-	function isBookmarkNode( node ) {
-		return ( node && node.getName && node.getName() == 'span' && node.hasAttribute( '_fck_bookmark' ) ) || ( node && !node.getName && isBookmarkNode( node.getParent() ) );
-	}
-
-	function ignoreBookmarkEvaluator( node ) {
-		return !isBookmarkNode( node );
-	}
-
-
-	/**
-	 * Find next source order node, ignore bookmark nodes and stop at the specified end node.
-	 * @param {Object} currentNode
-	 * @param {Object} endNode
-	 */
-	function getNextSourceNode( currentNode, endNode, startFromSibling ) {
-		var next = currentNode;
-		do {
-			next = next.getNextSourceNode( startFromSibling, null, endNode );
-		}
-		while ( isBookmarkNode( next ) )
-		return next;
-	}
-
 	var iterator = function( range ) {
 			if ( arguments.length < 1 )
 				return;
@@ -66,15 +43,16 @@ CKEDITOR.plugins.add( 'domiterator' );
 				range = this.range.clone();
 				range.enlarge( this.forceBrBreak ? CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS : CKEDITOR.ENLARGE_BLOCK_CONTENTS );
 
-				var walker = new CKEDITOR.dom.walker( range );
-				walker.evaluator = ignoreBookmarkEvaluator;
+				var walker = new CKEDITOR.dom.walker( range ),
+					ignoreBookmarkTextEvaluator = CKEDITOR.dom.walker.bookmark( true, true );
+				// Avoid anchor inside bookmark inner text.
+				walker.evaluator = ignoreBookmarkTextEvaluator;
 				this._.nextNode = walker.next();
-
 				// TODO: It's better to have walker.reset() used here.
 				walker = new CKEDITOR.dom.walker( range );
-				walker.evaluator = ignoreBookmarkEvaluator;
+				walker.evaluator = ignoreBookmarkTextEvaluator;
 				var lastNode = walker.previous();
-				this._.lastNode = getNextSourceNode( lastNode, null, true );
+				this._.lastNode = lastNode.getNextSourceNode( true );
 				// Probably the document end is reached, we need a marker node.
 				if ( !this._.lastNode ) {
 					this._.lastNode = range.document.createText( '' );
@@ -88,7 +66,6 @@ CKEDITOR.plugins.add( 'domiterator' );
 				lastNode = this._.lastNode;
 
 			this._.nextNode = null;
-
 			while ( currentNode ) {
 				// closeRange indicates that a paragraph boundary has been found,
 				// so the range can be closed.
@@ -198,7 +175,7 @@ CKEDITOR.plugins.add( 'domiterator' );
 				if ( isLast )
 					break;
 
-				currentNode = getNextSourceNode( currentNode, lastNode, continueFromSibling );
+				currentNode = currentNode.getNextSourceNode( continueFromSibling, null, lastNode );
 			}
 
 			// Now, based on the processed range, look for (or create) the block to be returned.
@@ -256,7 +233,7 @@ CKEDITOR.plugins.add( 'domiterator' );
 					// the current range, which could be an <li> child (nested
 					// lists) or the next sibling <li>.
 
-					this._.nextNode = ( block.equals( lastNode ) ? null : getNextSourceNode( range.getBoundaryNodes().endNode, lastNode, true ) );
+					this._.nextNode = ( block.equals( lastNode ) ? null : range.getBoundaryNodes().endNode.getNextSourceNode( true, null, lastNode ) );
 				}
 			}
 
@@ -283,7 +260,7 @@ CKEDITOR.plugins.add( 'domiterator' );
 			// above block can be removed or changed, so we can rely on it for the
 			// next interation.
 			if ( !this._.nextNode ) {
-				this._.nextNode = ( isLast || block.equals( lastNode ) ) ? null : getNextSourceNode( block, lastNode, true );
+				this._.nextNode = ( isLast || block.equals( lastNode ) ) ? null : block.getNextSourceNode( true, null, lastNode );
 			}
 
 			return block;

_source/plugins/list/plugin.js

@@ -326,7 +326,12 @@ For licensing, see LICENSE.html or http://ckeditor.com/license
 					var paragraph = doc.createElement( editor.config.enterMode == CKEDITOR.ENTER_P ? 'p' : ( editor.config.enterMode == CKEDITOR.ENTER_DIV ? 'div' : 'br' ) );
 					paragraph.appendTo( body );
 					ranges = [ new CKEDITOR.dom.range( doc ) ];
-					ranges[ 0 ].selectNodeContents( paragraph );
+					// IE exception on inserting anything when anchor inside <br>.
+					if ( paragraph.is( 'br' ) ) {
+						ranges[ 0 ].setStartBefore( paragraph );
+						ranges[ 0 ].setEndAfter( paragraph );
+					} else
+						ranges[ 0 ].selectNodeContents( paragraph );
 					selection.selectRanges( ranges );
 				}
 			}