Ver código fonte

Simplified "Position#getCommonAncestor()" (model) and fixed incorrect docs.

Kamil Piechaczek 8 anos atrás
pai
commit
2c6266851a

+ 7 - 10
packages/ckeditor5-engine/src/model/position.js

@@ -319,27 +319,24 @@ export default class Position {
 	}
 
 	/**
-	 * Returns a {@link module:engine/model/node~Node} which is a common ancestor for both positions. The {@link #root roots}
-	 * of these two positions must be identical.
+	 * Returns an {@link module:engine/model/element~Element} or {@link module:engine/model/documentfragment~DocumentFragment}
+	 * which is a common ancestor for both positions. The {@link #root roots} of these two positions must be identical.
 	 *
 	 * @param {module:engine/model/position~Position} position The second position.
-	 * @returns {module:engine/model/node~Node|null} Node that contains both positions.
+	 * @returns {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment|null}
 	 */
 	getCommonAncestor( position ) {
 		if ( this.root !== position.root ) {
 			return null;
 		}
 
-		const ancestorsA = this.getAncestors();
-		const ancestorsB = position.getAncestors();
+		const node = this.root.getNodeByPath( this.getCommonPath( position ) );
 
-		let i = 0;
-
-		while ( ancestorsA[ i ] == ancestorsB[ i ] && ancestorsA[ i ] ) {
-			i++;
+		if ( node.is( 'text' ) ) {
+			return node.parent;
 		}
 
-		return i === 0 ? null : ancestorsA[ i - 1 ];
+		return node;
 	}
 
 	/**

+ 3 - 2
packages/ckeditor5-engine/src/model/range.js

@@ -446,9 +446,10 @@ export default class Range {
 	}
 
 	/**
-	 * Returns a {@link module:engine/model/node~Node} which is a common ancestor for both positions.
+	 * Returns an {@link module:engine/model/element~Element} or {@link module:engine/model/documentfragment~DocumentFragment}
+	 * which is a common ancestor for both positions.
 	 *
-	 * @returns {module:engine/model/node~Node|null}
+	 * @returns {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment|null}
 	 */
 	getCommonAncestor() {
 		return this.start.getCommonAncestor( this.end );

+ 8 - 7
packages/ckeditor5-engine/tests/model/position.js

@@ -847,7 +847,7 @@ describe( 'Position', () => {
 
 	describe( 'getCommonAncestor()', () => {
 		it( 'returns null when roots of the position are not the same', () => {
-			const pos1 = new Position( root, [ 1, 1 ] );
+			const pos1 = new Position( root, [ 0 ] );
 			const pos2 = new Position( otherRoot, [ 1, 1 ] );
 
 			test( pos1, pos2, null );
@@ -855,11 +855,12 @@ describe( 'Position', () => {
 
 		it( 'for two the same positions returns the parent element', () => {
 			const fPosition = new Position( root, [ 1, 0, 0 ] );
+			const otherPosition = new Position( root, [ 1, 0, 0 ] );
 
-			test( fPosition, fPosition, li1 );
+			test( fPosition, otherPosition, li1 );
 		} );
 
-		it( 'for two positions in the same parent returns the parent element', () => {
+		it( 'for two positions in the same element returns the element', () => {
 			const fPosition = new Position( root, [ 1, 0, 0 ] );
 			const zPosition = new Position( root, [ 1, 0, 2 ] );
 
@@ -868,17 +869,17 @@ describe( 'Position', () => {
 
 		it( 'for two different positions returns first element which contains both positions', () => {
 			const zPosition = new Position( root, [ 1, 0, 2 ] );
-			const rPosition = new Position( root, [ 1, 1, 2 ] );
+			const liPosition = new Position( root, [ 1, 1 ] );
 
-			test( rPosition, zPosition, ul );
+			test( liPosition, zPosition, ul );
 		} );
 
 		it( 'works fine with positions hooked in `DocumentFragment`', () => {
 			const docFrag = new DocumentFragment( [ p, ul ] );
 			const zPosition = new Position( docFrag, [ 1, 0, 2 ] );
-			const rPosition = new Position( docFrag, [ 1, 1, 2 ] );
+			const afterLiPosition = new Position( docFrag, [ 1, 2 ] );
 
-			test( zPosition, rPosition, ul );
+			test( zPosition, afterLiPosition, ul );
 		} );
 
 		function test( positionA, positionB, lca ) {