浏览代码

Change behavior and rename model Document#getNearestSelectionPosition to getNearestSelectionRange. Added tests and required fixes to other methods using this.

Szymon Kupś 9 年之前
父节点
当前提交
52dfc74e82

+ 13 - 10
packages/ckeditor5-engine/src/controller/insertcontent.js

@@ -50,7 +50,12 @@ export default function insertContent( dataController, content, selection, batch
 		isLast: true
 	} );
 
-	selection.setRanges( insertion.getSelectionRanges() );
+	const newRange = insertion.getSelectionRange();
+
+	/* istanbul ignore else */
+	if ( newRange ) {
+		selection.setRanges( [ newRange ] );
+	}
 }
 
 /**
@@ -123,19 +128,17 @@ class Insertion {
 	}
 
 	/**
-	 * Returns a range to be selected after insertion.
+	 * Returns range to be selected after insertion.
+	 * Returns null if there is no valid range to select after insertion.
 	 *
-	 * @returns {module:engine/model/range~Range}
+	 * @returns {module:engine/model/range~Range|null}
 	 */
-	getSelectionRanges() {
+	getSelectionRange() {
 		if ( this.nodeToSelect ) {
-			return [ Range.createOn( this.nodeToSelect ) ];
-		} else {
-			const document = this.dataController.model;
-			const selectionPosition = document.getNearestSelectionPosition( this.position );
-
-			return [ new Range( selectionPosition ) ];
+			return Range.createOn( this.nodeToSelect );
 		}
+
+		return this.dataController.model.getNearestSelectionRange( this.position );
 	}
 
 	/**

+ 76 - 32
packages/ckeditor5-engine/src/model/document.js

@@ -280,51 +280,55 @@ export default class Document {
 	}
 
 	/**
-	 * Basing on given `position`, finds and returns a {@link module:engine/model/position~Position Position} instance that is nearest
-	 * to that `position` and is a correct position for selection. A position is correct for selection if
-	 * text node can be placed at that position.
+	 * Basing on given `position`, finds and returns a {@link module:engine/model/range~Range Range} instance that is
+	 * nearest to that `position` and is a correct range for selection.
 	 *
-	 * If no correct position is found, the first position in given `position` root is returned. This can happen if no node
-	 * has been added to the root or it may mean incorrect model document state.
+	 * Correct selection range might be collapsed - when it's located in position where text node can be placed.
+	 * Non-collapsed range is returned when selection can be placed around element marked as "object" in
+	 * {@link module:engine/model/schema~Schema schema}.
 	 *
-	 * @param {module:engine/model/position~Position} position Reference position where selection position should be looked for.
-	 * @returns {module:engine/model/position~Position|null} Nearest selection position.
+	 * Direction of searching for nearest correct selection range can be specified as:
+	 * * `both` - searching will be performed in both ways,
+	 * * `forward` - searching will be performed only forward,
+	 * * `backward` - searching will be performed only backward.
+	 *
+	 * When valid selection range cannot be found, `null` value is returned.
+	 *
+	 * @param {module:engine/model/position~Position} position Reference position where new selection range should be looked for.
+	 * @param {'both'|'forward'|'backward'} [direction='both'] Search direction.
+	 * @returns {module:engine/model/range~Range|null} Nearest selection range or `null` if one cannot be found.
 	 */
-	getNearestSelectionPosition( position ) {
+	getNearestSelectionRange( position, direction = 'both' ) {
+		// Return collapsed range if provided position is valid.
 		if ( this.schema.check( { name: '$text', inside: position } ) ) {
-			return Position.createFromPosition( position );
+			return new Range( position );
 		}
 
-		const backwardWalker = new TreeWalker( { startPosition: position, direction: 'backward' } );
-		const forwardWalker = new TreeWalker( { startPosition: position } );
+		let backwardWalker;
+		let forwardWalker;
 
-		let done = false;
-
-		while ( !done ) {
-			done = true;
+		if ( direction == 'both' || direction == 'backward' ) {
+			backwardWalker = new TreeWalker( { startPosition: position, direction: 'backward' } );
+		}
 
-			let step = backwardWalker.next();
+		if ( direction == 'both' || direction == 'forward' ) {
+			forwardWalker = new TreeWalker( { startPosition: position } );
+		}
 
-			if ( !step.done ) {
-				if ( this.schema.check( { name: '$text', inside: step.value.nextPosition } ) ) {
-					return step.value.nextPosition;
-				}
+		for ( let data of getWalkersData( backwardWalker, forwardWalker ) ) {
+			const type = ( data.walker == backwardWalker ? 'elementEnd' : 'elementStart' );
+			const value = data.value;
 
-				done = false;
+			if ( value.type == type && this.schema.objects.has( value.item.name ) ) {
+				return Range.createOn( value.item );
 			}
 
-			step = forwardWalker.next();
-
-			if ( !step.done ) {
-				if ( this.schema.check( { name: '$text', inside: step.value.nextPosition } ) ) {
-					return step.value.nextPosition;
-				}
-
-				done = false;
+			if ( this.schema.check( { name: '$text', inside: value.nextPosition } ) ) {
+				return new Range( value.nextPosition );
 			}
 		}
 
-		return new Position( position.root, [ 0 ] );
+		return null;
 	}
 
 	/**
@@ -370,9 +374,10 @@ export default class Document {
 
 		// Find the first position where the selection can be put.
 		const position = new Position( defaultRoot, [ 0 ] );
-		const selectionPosition = this.getNearestSelectionPosition( position );
+		const nearestRange = this.getNearestSelectionRange( position );
 
-		return new Range( selectionPosition );
+		// If valid selection range is not found - return range collapsed at the beginning of the root.
+		return nearestRange || new Range( position );
 	}
 
 	/**
@@ -447,3 +452,42 @@ function validateTextNodePosition( rangeBoundary ) {
 
 	return true;
 }
+
+// Generator function returning values from provided walkers, switching between them at each iteration. If only one walker
+// is provided it will return data only from that walker.
+//
+// @param {module:engine/module/treewalker~TreeWalker} [backward] Walker iterating in backward direction.
+// @param {module:engine/module/treewalker~TreeWalker} [forward] Walker iterating in forward direction.
+// @returns {Iterable.<Object>} Object returned at each iteration contains `value` and `walker` (informing which walker returned
+// given value) fields.
+function *getWalkersData( backward, forward ) {
+	let done = false;
+
+	while ( !done ) {
+		done = true;
+
+		if ( backward ) {
+			const step = backward.next();
+
+			if ( !step.done ) {
+				done = false;
+				yield {
+					walker: backward,
+					value: step.value
+				};
+			}
+		}
+
+		if ( forward ) {
+			const step = forward.next();
+
+			if ( !step.done ) {
+				done = false;
+				yield {
+					walker: forward,
+					value: step.value
+				};
+			}
+		}
+	}
+}

+ 7 - 2
packages/ckeditor5-engine/src/model/liveselection.js

@@ -629,9 +629,14 @@ export default class LiveSelection extends Selection {
 		newPath[ newPath.length - 1 ] -= gyPath[ 1 ];
 
 		const newPosition = new Position( oldRange.root, newPath );
-		const selectionPosition = this._document.getNearestSelectionPosition( newPosition );
-		const newRange = this._prepareRange( new Range( selectionPosition ) );
+		let selectionRange = this._document.getNearestSelectionRange( newPosition );
 
+		// If nearest valid selection range cannot be found - use one created at root beginning.
+		if ( !selectionRange ) {
+			selectionRange = new Range( new Position( newPosition.root, [ 0 ] ) );
+		}
+
+		const newRange = this._prepareRange( selectionRange );
 		const index = this._ranges.indexOf( gyRange );
 		this._ranges.splice( index, 1, newRange );
 

+ 148 - 38
packages/ckeditor5-engine/tests/model/document/document.js

@@ -8,14 +8,13 @@
 import Document from 'ckeditor5/engine/model/document.js';
 import Schema from 'ckeditor5/engine/model/schema.js';
 import RootElement from 'ckeditor5/engine/model/rootelement.js';
-import Element from 'ckeditor5/engine/model/element.js';
 import Batch from 'ckeditor5/engine/model/batch.js';
 import Delta from 'ckeditor5/engine/model/delta/delta.js';
-import Position from 'ckeditor5/engine/model/position.js';
 import Range from 'ckeditor5/engine/model/range.js';
 import CKEditorError from 'ckeditor5/utils/ckeditorerror.js';
 import count from 'ckeditor5/utils/count.js';
 import { jsonParseStringify } from 'tests/engine/model/_utils/utils.js';
+import { setData, getData } from 'ckeditor5/engine/dev-utils/model.js';
 
 describe( 'Document', () => {
 	let doc;
@@ -276,49 +275,160 @@ describe( 'Document', () => {
 		} );
 	} );
 
-	describe( 'getNearestSelectionPosition', () => {
+	describe( 'getNearestSelectionRange', () => {
 		let root;
+		let selection;
 
 		beforeEach( () => {
 			doc.schema.registerItem( 'paragraph', '$block' );
-			root = doc.createRoot();
-		} );
-
-		it( 'should return equal position if text node can be placed at that position', () => {
-			root.appendChildren( new Element( 'paragraph' ) );
-
-			const position = new Position( root, [ 0, 0 ] );
-			const selectionPosition = doc.getNearestSelectionPosition( position );
-
-			expect( position.isEqual( selectionPosition ) ).to.be.true;
-		} );
-
-		it( 'should return a position before if it is a closer position at which text node can be placed', () => {
-			root.appendChildren( [ new Element( 'paragraph' ), new Element( 'paragraph' ) ] );
-
-			const position = new Position( root, [ 1 ] );
-			const selectionPosition = doc.getNearestSelectionPosition( position );
-
-			expect( selectionPosition.path ).to.deep.equal( [ 0, 0 ] );
-		} );
-
-		it( 'should return a position after if it is a closer position at which text node can be placed', () => {
-			root.appendChildren( [ new Element( 'paragraph' ), new Element( 'image' ), new Element( 'paragraph' ) ] );
-
-			const position = new Position( root, [ 2 ] );
-			const selectionPosition = doc.getNearestSelectionPosition( position );
+			doc.schema.registerItem( 'image' );
+			doc.schema.allow( { name: 'image', inside: '$root' } );
+			doc.schema.registerItem( 'widget', '$block' );
+			doc.schema.allow( { name: 'widget', inside: '$root' } );
+			doc.schema.objects.add( 'widget' );
 
-			expect( selectionPosition.path ).to.deep.equal( [ 2, 0 ] );
+			root = doc.createRoot();
+			selection = doc.selection;
 		} );
 
-		it( 'should return first position in root if there is no position where text node can be placed', () => {
-			root.appendChildren( new Element( 'table' ) );
-
-			const position = new Position( root, [ 0, 0 ] );
-			const selectionPosition = doc.getNearestSelectionPosition( position );
-
-			expect( selectionPosition.path ).to.deep.equal( [ 0 ] );
-		} );
+		test(
+			'should return collapsed range if text node can be placed at that position - both',
+			'<paragraph>[]</paragraph>',
+			'both',
+			'<paragraph>[]</paragraph>'
+		);
+
+		test(
+			'should return collapsed range if text node can be placed at that position - forward',
+			'<paragraph>[]</paragraph>',
+			'forward',
+			'<paragraph>[]</paragraph>'
+		);
+
+		test(
+			'should return collapsed range if text node can be placed at that position - backward',
+			'<paragraph>[]</paragraph>',
+			'backward',
+			'<paragraph>[]</paragraph>'
+		);
+
+		test( 'should return null in empty document - both', '', 'both', null );
+
+		test( 'should return null in empty document - backward', '', 'backward', null );
+
+		test( 'should return null in empty document - forward', '', 'forward', null );
+
+		test(
+			'should find range before when searching both ways',
+			'<paragraph></paragraph>[]<paragraph></paragraph>',
+			'both',
+			'<paragraph>[]</paragraph><paragraph></paragraph>'
+		);
+
+		test(
+			'should find range before when searching backward',
+			'<paragraph></paragraph>[]<paragraph></paragraph>',
+			'backward',
+			'<paragraph>[]</paragraph><paragraph></paragraph>'
+		);
+
+		test(
+			'should find range after when searching forward',
+			'<paragraph></paragraph>[]<paragraph></paragraph>',
+			'forward',
+			'<paragraph></paragraph><paragraph>[]</paragraph>'
+		);
+
+		test(
+			'should find range after when searching both ways when it is closer',
+			'<paragraph></paragraph><image></image>[]<paragraph></paragraph>',
+			'both',
+			'<paragraph></paragraph><image></image><paragraph>[]</paragraph>'
+		);
+
+		test(
+			'should find range before when searching both ways when it is closer',
+			'<paragraph></paragraph><image></image>[]<image></image><image></image><paragraph></paragraph>',
+			'both',
+			'<paragraph>[]</paragraph><image></image><image></image><image></image><paragraph></paragraph>'
+		);
+
+		test(
+			'should return null if there is no valid range',
+			'<image></image>',
+			'both',
+			null
+		);
+
+		test(
+			'should return null if there is no valid range in given direction - backward',
+			'[]<paragraph></paragraph>',
+			'backward',
+			null
+		);
+
+		test(
+			'should return null if there is no valid range in given direction - forward',
+			'<paragraph></paragraph>[]',
+			'forward',
+			null
+		);
+
+		test(
+			'should select nearest object - both',
+			'<widget></widget>[]<widget></widget>',
+			'both',
+			'[<widget></widget>]<widget></widget>'
+		);
+
+		test(
+			'should select nearest object - forward',
+			'<paragraph></paragraph>[]<widget></widget>',
+			'forward',
+			'<paragraph></paragraph>[<widget></widget>]'
+		);
+
+		test(
+			'should select nearest object - forward',
+			'<paragraph></paragraph>[]<widget></widget>',
+			'forward',
+			'<paragraph></paragraph>[<widget></widget>]'
+		);
+
+		test(
+			'should select nearest object - backward',
+			'<widget></widget>[]<paragraph></paragraph>',
+			'backward',
+			'[<widget></widget>]<paragraph></paragraph>'
+		);
+
+		test(
+			'should move forward when placed at root start',
+			'[]<paragraph></paragraph><paragraph></paragraph>',
+			'both',
+			'<paragraph>[]</paragraph><paragraph></paragraph>'
+		);
+
+		test(
+			'should move backward when placed at root end',
+			'<paragraph></paragraph><paragraph></paragraph>[]',
+			'both',
+			'<paragraph></paragraph><paragraph>[]</paragraph>'
+		);
+
+		function test( testName, data, direction, expected ) {
+			it( testName, () => {
+				setData( doc, data );
+				const range = doc.getNearestSelectionRange( selection.anchor, direction );
+
+				if ( expected === null ) {
+					expect( range ).to.be.null;
+				} else {
+					selection.setRanges( [ range ] );
+					expect( getData( doc ) ).to.equal( expected );
+				}
+			} );
+		}
 	} );
 
 	describe( '_getDefaultRoot', () => {