Merge pull request #1037 from ckeditor/t/1036

Other: Introduced `options.includeSelf` to `getCommonAncestor()`. Closes #1036.

BREAKING CHANGE: The `includeNode` option of `Node#getAncestors()` methods (model and view) was renamed to `includeSelf`. See #1036.

Author: pomek

src/model/node.js

@@ -252,14 +252,14 @@ export default class Node {
 	 * Returns ancestors array of this node.
 	 *
 	 * @param {Object} options Options object.
-	 * @param {Boolean} [options.includeNode=false] When set to `true` this node will be also included in parent's array.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` this node will be also included in parent's array.
 	 * @param {Boolean} [options.parentFirst=false] When set to `true`, array will be sorted from node's parent to root element,
 	 * otherwise root element will be the first item in the array.
 	 * @returns {Array} Array with ancestors.
 	 */
-	getAncestors( options = { includeNode: false, parentFirst: false } ) {
+	getAncestors( options = { includeSelf: false, parentFirst: false } ) {
 		const ancestors = [];
-		let parent = options.includeNode ? this : this.parent;
+		let parent = options.includeSelf ? this : this.parent;
 
 		while ( parent ) {
 			ancestors[ options.parentFirst ? 'push' : 'unshift' ]( parent );
@@ -274,11 +274,14 @@ export default class Node {
 	 * which is a common ancestor of both nodes.
 	 *
 	 * @param {module:engine/model/node~Node} node The second node.
+	 * @param {Object} options Options object.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` both nodes will be considered "ancestors" too.
+	 * Which means that if e.g. node A is inside B, then their common ancestor will be B.
 	 * @returns {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment|null}
 	 */
-	getCommonAncestor( node ) {
-		const ancestorsA = this.getAncestors();
-		const ancestorsB = node.getAncestors();
+	getCommonAncestor( node, options = {} ) {
+		const ancestorsA = this.getAncestors( options );
+		const ancestorsB = node.getAncestors( options );
 
 		let i = 0;
 

src/model/position.js

@@ -294,7 +294,7 @@ export default class Position {
 		if ( this.parent.is( 'documentFragment' ) ) {
 			return [ this.parent ];
 		} else {
-			return this.parent.getAncestors( { includeNode: true } );
+			return this.parent.getAncestors( { includeSelf: true } );
 		}
 	}
 

src/model/selection.js

@@ -742,7 +742,7 @@ function isUnvisitedBlockContainer( element, visited ) {
 // Finds the lowest element in position's ancestors which is a block.
 // Marks all ancestors as already visited to not include any of them later on.
 function getParentBlock( position, visited ) {
-	const ancestors = position.parent.getAncestors( { parentFirst: true, includeNode: true } );
+	const ancestors = position.parent.getAncestors( { parentFirst: true, includeSelf: true } );
 	const block = ancestors.find( element => isUnvisitedBlockContainer( element, visited ) );
 
 	// Mark all ancestors of this position's parent, because find() might've stopped early and

src/model/textproxy.js

@@ -204,14 +204,14 @@ export default class TextProxy {
 	 * Returns ancestors array of this text proxy.
 	 *
 	 * @param {Object} options Options object.
-	 * @param {Boolean} [options.includeNode=false] When set to `true` this text proxy will be also included in parent's array.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` this text proxy will be also included in parent's array.
 	 * @param {Boolean} [options.parentFirst=false] When set to `true`, array will be sorted from text proxy parent to root element,
 	 * otherwise root element will be the first item in the array.
 	 * @returns {Array} Array with ancestors.
 	 */
-	getAncestors( options = { includeNode: false, parentFirst: false } ) {
+	getAncestors( options = { includeSelf: false, parentFirst: false } ) {
 		const ancestors = [];
-		let parent = options.includeNode ? this : this.parent;
+		let parent = options.includeSelf ? this : this.parent;
 
 		while ( parent ) {
 			ancestors[ options.parentFirst ? 'push' : 'unshift' ]( parent );

src/view/node.js

@@ -121,14 +121,14 @@ export default class Node {
 	 * Returns ancestors array of this node.
 	 *
 	 * @param {Object} options Options object.
-	 * @param {Boolean} [options.includeNode=false] When set to `true` this node will be also included in parent's array.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` this node will be also included in parent's array.
 	 * @param {Boolean} [options.parentFirst=false] When set to `true`, array will be sorted from node's parent to root element,
 	 * otherwise root element will be the first item in the array.
 	 * @returns {Array} Array with ancestors.
 	 */
-	getAncestors( options = { includeNode: false, parentFirst: false } ) {
+	getAncestors( options = { includeSelf: false, parentFirst: false } ) {
 		const ancestors = [];
-		let parent = options.includeNode ? this : this.parent;
+		let parent = options.includeSelf ? this : this.parent;
 
 		while ( parent ) {
 			ancestors[ options.parentFirst ? 'push' : 'unshift' ]( parent );
@@ -143,11 +143,14 @@ export default class Node {
 	 * which is a common ancestor of both nodes.
 	 *
 	 * @param {module:engine/view/node~Node} node The second node.
+	 * @param {Object} options Options object.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` both nodes will be considered "ancestors" too.
+	 * Which means that if e.g. node A is inside B, then their common ancestor will be B.
 	 * @returns {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment|null}
 	 */
-	getCommonAncestor( node ) {
-		const ancestorsA = this.getAncestors();
-		const ancestorsB = node.getAncestors();
+	getCommonAncestor( node, options = {} ) {
+		const ancestorsA = this.getAncestors( options );
+		const ancestorsB = node.getAncestors( options );
 
 		let i = 0;
 

src/view/position.js

@@ -171,7 +171,7 @@ export default class Position {
 		if ( this.parent.is( 'documentFragment' ) ) {
 			return [ this.parent ];
 		} else {
-			return this.parent.getAncestors( { includeNode: true } );
+			return this.parent.getAncestors( { includeSelf: true } );
 		}
 	}
 

src/view/textproxy.js

@@ -146,14 +146,14 @@ export default class TextProxy {
 	 * Returns ancestors array of this text proxy.
 	 *
 	 * @param {Object} options Options object.
-	 * @param {Boolean} [options.includeNode=false] When set to `true` {#textNode} will be also included in parent's array.
+	 * @param {Boolean} [options.includeSelf=false] When set to `true` {#textNode} will be also included in parent's array.
 	 * @param {Boolean} [options.parentFirst=false] When set to `true`, array will be sorted from text proxy parent to
 	 * root element, otherwise root element will be the first item in the array.
 	 * @returns {Array} Array with ancestors.
 	 */
-	getAncestors( options = { includeNode: false, parentFirst: false } ) {
+	getAncestors( options = { includeSelf: false, parentFirst: false } ) {
 		const ancestors = [];
-		let parent = options.includeNode ? this.textNode : this.parent;
+		let parent = options.includeSelf ? this.textNode : this.parent;
 
 		while ( parent !== null ) {
 			ancestors[ options.parentFirst ? 'push' : 'unshift' ]( parent );

tests/model/node.js

@@ -225,20 +225,20 @@ describe( 'Node', () => {
 			expect( textBA.getAncestors() ).to.deep.equal( [ root, two ] );
 		} );
 
-		it( 'should include itself if includeNode option is set to true', () => {
-			expect( root.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root ] );
-			expect( two.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root, two ] );
-			expect( textBA.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root, two, textBA ] );
-			expect( img.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root, two, img ] );
-			expect( textR.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root, two, textR ] );
+		it( 'should include itself if includeSelf option is set to true', () => {
+			expect( root.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root ] );
+			expect( two.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root, two ] );
+			expect( textBA.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root, two, textBA ] );
+			expect( img.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root, two, img ] );
+			expect( textR.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root, two, textR ] );
 		} );
 
 		it( 'should reverse order if parentFirst option is set to true', () => {
-			expect( root.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ root ] );
-			expect( two.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ two, root ] );
-			expect( textBA.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ textBA, two, root ] );
-			expect( img.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ img, two, root ] );
-			expect( textR.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ textR, two, root ] );
+			expect( root.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ root ] );
+			expect( two.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ two, root ] );
+			expect( textBA.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ textBA, two, root ] );
+			expect( img.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ img, two, root ] );
+			expect( textR.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ textR, two, root ] );
 		} );
 	} );
 
@@ -247,11 +247,18 @@ describe( 'Node', () => {
 			expect( img.getCommonAncestor( img ) ).to.equal( two );
 		} );
 
+		it( 'should return the given node for the same node if includeSelf is used', () => {
+			expect( img.getCommonAncestor( img, { includeSelf: true } ) ).to.equal( img );
+		} );
+
 		it( 'should return null for detached subtrees', () => {
 			const detached = new Element( 'foo' );
 
 			expect( img.getCommonAncestor( detached ) ).to.be.null;
 			expect( detached.getCommonAncestor( img ) ).to.be.null;
+
+			expect( img.getCommonAncestor( detached, { includeSelf: true } ) ).to.be.null;
+			expect( detached.getCommonAncestor( img, { includeSelf: true } ) ).to.be.null;
 		} );
 
 		it( 'should return null when one of the nodes is a tree root itself', () => {
@@ -260,9 +267,18 @@ describe( 'Node', () => {
 			expect( root.getCommonAncestor( root ) ).to.be.null;
 		} );
 
+		it( 'should return root when one of the nodes is a tree root itself and includeSelf is used', () => {
+			expect( root.getCommonAncestor( img, { includeSelf: true } ) ).to.equal( root );
+			expect( img.getCommonAncestor( root, { includeSelf: true } ) ).to.equal( root );
+			expect( root.getCommonAncestor( root, { includeSelf: true } ) ).to.equal( root );
+		} );
+
 		it( 'should return parent of the nodes at the same level', () => {
-			expect( img.getCommonAncestor( textBA ) ).to.equal( two );
-			expect( textR.getCommonAncestor( textBA ) ).to.equal( two );
+			expect( img.getCommonAncestor( textBA ), 1 ).to.equal( two );
+			expect( textR.getCommonAncestor( textBA ), 2 ).to.equal( two );
+
+			expect( img.getCommonAncestor( textBA, { includeSelf: true } ), 3 ).to.equal( two );
+			expect( textR.getCommonAncestor( textBA, { includeSelf: true } ), 4 ).to.equal( two );
 		} );
 
 		it( 'should return proper element for nodes in different branches and on different levels', () => {
@@ -282,6 +298,14 @@ describe( 'Node', () => {
 			expect( c.getCommonAncestor( b ), 3 ).to.equal( a );
 			expect( bom.getCommonAncestor( d ), 4 ).to.equal( a );
 			expect( b.getCommonAncestor( bom ), 5 ).to.equal( a );
+			expect( b.getCommonAncestor( bar ), 6 ).to.equal( a );
+
+			expect( bar.getCommonAncestor( foo, { includeSelf: true } ), 11 ).to.equal( c );
+			expect( foo.getCommonAncestor( d, { includeSelf: true } ), 12 ).to.equal( c );
+			expect( c.getCommonAncestor( b, { includeSelf: true } ), 13 ).to.equal( b );
+			expect( bom.getCommonAncestor( d, { includeSelf: true } ), 14 ).to.equal( a );
+			expect( b.getCommonAncestor( bom, { includeSelf: true } ), 15 ).to.equal( a );
+			expect( b.getCommonAncestor( bar, { includeSelf: true } ), 16 ).to.equal( b );
 		} );
 
 		it( 'should return document fragment', () => {

tests/model/textproxy.js

@@ -125,12 +125,12 @@ describe( 'TextProxy', () => {
 			expect( textProxy.getAncestors() ).to.deep.equal( [ root, element ] );
 		} );
 
-		it( 'should include itself if includeNode option is set to true', () => {
-			expect( textProxy.getAncestors( { includeNode: true } ) ).to.deep.equal( [ root, element, textProxy ] );
+		it( 'should include itself if includeSelf option is set to true', () => {
+			expect( textProxy.getAncestors( { includeSelf: true } ) ).to.deep.equal( [ root, element, textProxy ] );
 		} );
 
 		it( 'should reverse order if parentFirst option is set to true', () => {
-			expect( textProxy.getAncestors( { includeNode: true, parentFirst: true } ) ).to.deep.equal( [ textProxy, element, root ] );
+			expect( textProxy.getAncestors( { includeSelf: true, parentFirst: true } ) ).to.deep.equal( [ textProxy, element, root ] );
 		} );
 	} );
 

tests/view/node.js

@@ -65,7 +65,7 @@ describe( 'Node', () => {
 		} );
 
 		it( 'should return array including node itself if requested', () => {
-			const result = root.getAncestors( { includeNode: true } );
+			const result = root.getAncestors( { includeSelf: true } );
 			expect( result ).to.be.an( 'array' );
 			expect( result.length ).to.equal( 1 );
 			expect( result[ 0 ] ).to.equal( root );
@@ -77,7 +77,7 @@ describe( 'Node', () => {
 			expect( result[ 0 ] ).to.equal( root );
 			expect( result[ 1 ] ).to.equal( two );
 
-			const result2 = charR.getAncestors( { includeNode: true } );
+			const result2 = charR.getAncestors( { includeSelf: true } );
 			expect( result2.length ).to.equal( 3 );
 			expect( result2[ 0 ] ).to.equal( root );
 			expect( result2[ 1 ] ).to.equal( two );
@@ -90,7 +90,7 @@ describe( 'Node', () => {
 			expect( result[ 0 ] ).to.equal( two );
 			expect( result[ 1 ] ).to.equal( root );
 
-			const result2 = charR.getAncestors( { includeNode: true, parentFirst: true } );
+			const result2 = charR.getAncestors( { includeSelf: true, parentFirst: true } );
 			expect( result2.length ).to.equal( 3 );
 			expect( result2[ 2 ] ).to.equal( root );
 			expect( result2[ 1 ] ).to.equal( two );
@@ -114,11 +114,18 @@ describe( 'Node', () => {
 			expect( img.getCommonAncestor( img ) ).to.equal( two );
 		} );
 
+		it( 'should return the given node for the same node if includeSelf is used', () => {
+			expect( img.getCommonAncestor( img, { includeSelf: true } ) ).to.equal( img );
+		} );
+
 		it( 'should return null for detached subtrees', () => {
 			const detached = new Element( 'foo' );
 
 			expect( img.getCommonAncestor( detached ) ).to.be.null;
 			expect( detached.getCommonAncestor( img ) ).to.be.null;
+
+			expect( img.getCommonAncestor( detached, { includeSelf: true } ) ).to.be.null;
+			expect( detached.getCommonAncestor( img, { includeSelf: true } ) ).to.be.null;
 		} );
 
 		it( 'should return null when one of the nodes is a tree root itself', () => {
@@ -127,9 +134,18 @@ describe( 'Node', () => {
 			expect( root.getCommonAncestor( root ) ).to.be.null;
 		} );
 
+		it( 'should return root when one of the nodes is a tree root itself and includeSelf is used', () => {
+			expect( root.getCommonAncestor( img, { includeSelf: true } ) ).to.equal( root );
+			expect( img.getCommonAncestor( root, { includeSelf: true } ) ).to.equal( root );
+			expect( root.getCommonAncestor( root, { includeSelf: true } ) ).to.equal( root );
+		} );
+
 		it( 'should return parent of the nodes at the same level', () => {
-			expect( img.getCommonAncestor( charA ) ).to.equal( two );
-			expect( charB.getCommonAncestor( charA ) ).to.equal( two );
+			expect( img.getCommonAncestor( charA ), 1 ).to.equal( two );
+			expect( charB.getCommonAncestor( charA ), 2 ).to.equal( two );
+
+			expect( img.getCommonAncestor( charA, { includeSelf: true } ), 3 ).to.equal( two );
+			expect( charB.getCommonAncestor( charA, { includeSelf: true } ), 4 ).to.equal( two );
 		} );
 
 		it( 'should return proper element for nodes in different branches and on different levels', () => {
@@ -149,6 +165,14 @@ describe( 'Node', () => {
 			expect( c.getCommonAncestor( b ), 3 ).to.equal( a );
 			expect( bom.getCommonAncestor( d ), 4 ).to.equal( a );
 			expect( b.getCommonAncestor( bom ), 5 ).to.equal( a );
+			expect( b.getCommonAncestor( bar ), 6 ).to.equal( a );
+
+			expect( bar.getCommonAncestor( foo, { includeSelf: true } ), 11 ).to.equal( c );
+			expect( foo.getCommonAncestor( d, { includeSelf: true } ), 12 ).to.equal( c );
+			expect( c.getCommonAncestor( b, { includeSelf: true } ), 13 ).to.equal( b );
+			expect( bom.getCommonAncestor( d, { includeSelf: true } ), 14 ).to.equal( a );
+			expect( b.getCommonAncestor( bom, { includeSelf: true } ), 15 ).to.equal( a );
+			expect( b.getCommonAncestor( bar, { includeSelf: true } ), 16 ).to.equal( b );
 		} );
 
 		it( 'should return document fragment', () => {