8
0
Pārlūkot izejas kodu

Merge branch 'master' into t/922

Piotrek Koszuliński 8 gadi atpakaļ
vecāks
revīzija
c99398741c

+ 8 - 5
packages/ckeditor5-engine/src/controller/deletecontent.js

@@ -18,9 +18,12 @@ import Element from '../model/element';
  * @param {module:engine/model/selection~Selection} selection Selection of which the content should be deleted.
  * @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
  * @param {Object} [options]
- * @param {Boolean} [options.merge=false] Merge elements after removing the contents of the selection.
- * For example, `<h>x[x</h><p>y]y</p>` will become: `<h>x^y</h>` with the option enabled
- * and: `<h>x^</h><p>y</p>` without it.
+ * @param {Boolean} [options.leaveUnmerged=false] Whether to merge elements after removing the content of the selection.
+ *
+ * For example `<h>x[x</h><p>y]y</p>` will become:
+ * * `<h>x^y</h>` with the option disabled (`leaveUnmerged == false`)
+ * * `<h>x^</h><p>y</p>` with enabled (`leaveUnmerged == true`).
+ *
  * Note: {@link module:engine/model/schema~Schema#objects object} and {@link module:engine/model/schema~Schema#limits limit}
  * elements will not be merged.
  */
@@ -34,7 +37,7 @@ export default function deleteContent( selection, batch, options = {} ) {
 	const startPos = selRange.start;
 	const endPos = LivePosition.createFromPosition( selRange.end );
 
-	// 1. Remove the contents if there are any.
+	// 1. Remove the content if there is any.
 	if ( !selRange.start.isTouching( selRange.end ) ) {
 		batch.remove( selRange );
 	}
@@ -47,7 +50,7 @@ export default function deleteContent( selection, batch, options = {} ) {
 	// However, the algorithm supports also merging deeper structures (up to the depth of the shallower branch),
 	// as it's hard to imagine what should actually be the default behavior. Usually, specific features will
 	// want to override that behavior anyway.
-	if ( options.merge ) {
+	if ( !options.leaveUnmerged ) {
 		mergeBranches( batch, startPos, endPos );
 	}
 

+ 1 - 3
packages/ckeditor5-engine/src/controller/insertcontent.js

@@ -35,9 +35,7 @@ export default function insertContent( dataController, content, selection, batch
 	}
 
 	if ( !selection.isCollapsed ) {
-		dataController.deleteContent( selection, batch, {
-			merge: true
-		} );
+		dataController.deleteContent( selection, batch );
 	}
 
 	const insertion = new Insertion( dataController, batch, selection.anchor );

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

@@ -88,7 +88,7 @@ export default class LiveSelection extends Selection {
 	get isCollapsed() {
 		const length = this._ranges.length;
 
-		return length === 0 ? true : super.isCollapsed;
+		return length === 0 ? this._document._getDefaultRange().isCollapsed : super.isCollapsed;
 	}
 
 	/**
@@ -102,7 +102,7 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	get focus() {
-		return super.focus || this._document._getDefaultRange().start;
+		return super.focus || this._document._getDefaultRange().end;
 	}
 
 	/**

+ 10 - 2
packages/ckeditor5-engine/src/model/selection.js

@@ -576,7 +576,7 @@ export default class Selection {
 	 *
 	 * This method's result can be used for example to apply block styling to all blocks covered by this selection.
 	 *
-	 * *Note:* `getSelectedBlocks` always returns the deepest block.
+	 * **Note:** `getSelectedBlocks()` always returns the deepest block.
 	 *
 	 * In this case the function will return exactly all 3 paragraphs:
 	 *
@@ -590,6 +590,13 @@ export default class Selection {
 	 *
 	 *		<paragraph>[]a</paragraph>
 	 *
+	 * **Special case**: If a selection ends at the beginning of a block, that block is not returned as from user perspective
+	 * this block wasn't selected. See [#984](https://github.com/ckeditor/ckeditor5-engine/issues/984) for more details.
+	 *
+	 *		<paragraph>[a</paragraph>
+	 *		<paragraph>b</paragraph>
+	 *		<paragraph>]c</paragraph> // this block will not be returned
+	 *
 	 * @returns {Iterator.<module:engine/model/element~Element>}
 	 */
 	* getSelectedBlocks() {
@@ -610,7 +617,8 @@ export default class Selection {
 
 			const endBlock = getParentBlock( range.end, visited );
 
-			if ( endBlock ) {
+			// #984. Don't return the end block if the range ends right at its beginning.
+			if ( endBlock && !range.end.isTouching( Position.createAt( endBlock ) ) ) {
 				yield endBlock;
 			}
 		}

+ 1 - 5
packages/ckeditor5-engine/src/view/selection.js

@@ -439,7 +439,7 @@ export default class Selection {
 	 *
 	 * The location can be specified in the same form as {@link module:engine/view/position~Position.createAt} parameters.
 	 *
-	 * @fires change:range
+	 * @fires change
 	 * @param {module:engine/view/item~Item|module:engine/view/position~Position} itemOrPosition
 	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
 	 * first parameter is a {@link module:engine/view/item~Item view item}.
@@ -541,7 +541,3 @@ mix( Selection, EmitterMixin );
  *
  * @event change
  */
-
-/**
- * @event change:range
- */

+ 31 - 50
packages/ckeditor5-engine/tests/controller/deletecontent.js

@@ -74,8 +74,7 @@ describe( 'DataController', () => {
 			test(
 				'does not break things when option.merge passed',
 				'w[x<image></image>y]z',
-				'w[]z',
-				{ merge: true }
+				'w[]z'
 			);
 		} );
 
@@ -173,35 +172,32 @@ describe( 'DataController', () => {
 			test(
 				'do not merge when no need to',
 				'<paragraph>x</paragraph><paragraph>[foo]</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><paragraph>[]</paragraph><paragraph>y</paragraph>'
 			);
 
 			test(
 				'merges second element into the first one (same name)',
 				'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>y</paragraph>'
 			);
 
 			test(
 				'does not merge second element into the first one (same name, !option.merge)',
 				'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><paragraph>fo[]</paragraph><paragraph>ar</paragraph><paragraph>y</paragraph>'
+				'<paragraph>x</paragraph><paragraph>fo[]</paragraph><paragraph>ar</paragraph><paragraph>y</paragraph>',
+				{ leaveUnmerged: true }
 			);
 
 			test(
 				'merges second element into the first one (same name)',
 				'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>y</paragraph>'
 			);
 
 			test(
 				'merges second element into the first one (different name)',
 				'<paragraph>x</paragraph><heading1>fo[o</heading1><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><heading1>fo[]ar</heading1><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><heading1>fo[]ar</heading1><paragraph>y</paragraph>'
 			);
 
 			// Note: in all these cases we ignore the direction of merge.
@@ -214,7 +210,7 @@ describe( 'DataController', () => {
 					{ lastRangeBackward: true }
 				);
 
-				deleteContent( doc.selection, doc.batch(), { merge: true } );
+				deleteContent( doc.selection, doc.batch() );
 
 				expect( getData( doc ) ).to.equal( '<paragraph>x</paragraph><heading1>fo[]ar</heading1><paragraph>y</paragraph>' );
 			} );
@@ -222,29 +218,25 @@ describe( 'DataController', () => {
 			test(
 				'merges second element into the first one (different attrs)',
 				'<paragraph>x</paragraph><paragraph align="l">fo[o</paragraph><paragraph>b]ar</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><paragraph align="l">fo[]ar</paragraph><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><paragraph align="l">fo[]ar</paragraph><paragraph>y</paragraph>'
 			);
 
 			test(
 				'merges second element to an empty first element',
 				'<paragraph>x</paragraph><heading1>[</heading1><paragraph>fo]o</paragraph><paragraph>y</paragraph>',
-				'<paragraph>x</paragraph><heading1>[]o</heading1><paragraph>y</paragraph>',
-				{ merge: true }
+				'<paragraph>x</paragraph><heading1>[]o</heading1><paragraph>y</paragraph>'
 			);
 
 			test(
 				'merges empty element into the first element',
 				'<heading1>f[oo</heading1><paragraph>bar]</paragraph><paragraph>x</paragraph>',
-				'<heading1>f[]</heading1><paragraph>x</paragraph>',
-				{ merge: true }
+				'<heading1>f[]</heading1><paragraph>x</paragraph>'
 			);
 
 			test(
 				'leaves just one element when all selected',
 				'<heading1>[x</heading1><paragraph>foo</paragraph><paragraph>y]</paragraph>',
-				'<heading1>[]</heading1>',
-				{ merge: true }
+				'<heading1>[]</heading1>'
 			);
 
 			it( 'uses remove delta instead of merge delta if merged element is empty', () => {
@@ -254,7 +246,7 @@ describe( 'DataController', () => {
 				const spyMerge = sinon.spy( batch, 'merge' );
 				const spyRemove = sinon.spy( batch, 'remove' );
 
-				deleteContent( doc.selection, batch, { merge: true } );
+				deleteContent( doc.selection, batch );
 
 				expect( getData( doc ) ).to.equal( '<paragraph>ab[]</paragraph>' );
 
@@ -269,7 +261,7 @@ describe( 'DataController', () => {
 				const spyMerge = sinon.spy( batch, 'merge' );
 				const spyMove = sinon.spy( batch, 'move' );
 
-				deleteContent( doc.selection, batch, { merge: true } );
+				deleteContent( doc.selection, batch );
 
 				expect( getData( doc ) ).to.equal( '<paragraph>ab[]gh</paragraph>' );
 
@@ -284,8 +276,7 @@ describe( 'DataController', () => {
 				test(
 					'merges elements when deep nested',
 					'<paragraph>x<pchild>fo[o</pchild></paragraph><paragraph><pchild>b]ar</pchild>y</paragraph>',
-					'<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>',
-					{ merge: true }
+					'<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>'
 				);
 
 				it( 'merges elements when deep nested (3rd level)', () => {
@@ -322,7 +313,7 @@ describe( 'DataController', () => {
 
 					doc.selection.setRanges( [ range ] );
 
-					deleteContent( doc.selection, doc.batch(), { merge: true } );
+					deleteContent( doc.selection, doc.batch() );
 
 					expect( getData( doc ) )
 						.to.equal( '<pparent>x<paragraph>x<pchild>fo[]ar</pchild>y</paragraph>y</pparent>' );
@@ -331,15 +322,13 @@ describe( 'DataController', () => {
 				test(
 					'merges elements when left end deep nested',
 					'<paragraph>x<pchild>fo[o</pchild></paragraph><paragraph>b]ary</paragraph><paragraph>x</paragraph>',
-					'<paragraph>x<pchild>fo[]ary</pchild></paragraph><paragraph>x</paragraph>',
-					{ merge: true }
+					'<paragraph>x<pchild>fo[]ary</pchild></paragraph><paragraph>x</paragraph>'
 				);
 
 				test(
 					'merges elements when right end deep nested',
 					'<paragraph>x</paragraph><paragraph>fo[o</paragraph><paragraph><pchild>b]ar</pchild>x</paragraph>',
-					'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>x</paragraph>',
-					{ merge: true }
+					'<paragraph>x</paragraph><paragraph>fo[]ar</paragraph><paragraph>x</paragraph>'
 				);
 
 				it( 'merges elements when left end deep nested (3rd level)', () => {
@@ -369,7 +358,7 @@ describe( 'DataController', () => {
 
 					doc.selection.setRanges( [ range ] );
 
-					deleteContent( doc.selection, doc.batch(), { merge: true } );
+					deleteContent( doc.selection, doc.batch() );
 
 					expect( getData( doc ) )
 						.to.equal( '<pparent>x<paragraph>foo<pchild>ba[]om</pchild></paragraph></pparent>' );
@@ -378,15 +367,13 @@ describe( 'DataController', () => {
 				test(
 					'merges elements when right end deep nested (in an empty container)',
 					'<paragraph>fo[o</paragraph><paragraph><pchild>bar]</pchild></paragraph>',
-					'<paragraph>fo[]</paragraph>',
-					{ merge: true }
+					'<paragraph>fo[]</paragraph>'
 				);
 
 				test(
 					'merges elements when left end deep nested (in an empty container)',
 					'<paragraph><pchild>[foo</pchild></paragraph><paragraph>b]ar</paragraph><paragraph>x</paragraph>',
-					'<paragraph><pchild>[]ar</pchild></paragraph><paragraph>x</paragraph>',
-					{ merge: true }
+					'<paragraph><pchild>[]ar</pchild></paragraph><paragraph>x</paragraph>'
 				);
 
 				it( 'merges elements when left end deep nested (3rd level)', () => {
@@ -414,7 +401,7 @@ describe( 'DataController', () => {
 
 					doc.selection.setRanges( [ range ] );
 
-					deleteContent( doc.selection, doc.batch(), { merge: true } );
+					deleteContent( doc.selection, doc.batch() );
 
 					expect( getData( doc ) )
 						.to.equal( '<paragraph>fo[]</paragraph>' );
@@ -440,15 +427,13 @@ describe( 'DataController', () => {
 				test(
 					'does not merge an object element (if it is first)',
 					'<blockWidget><nestedEditable>fo[o</nestedEditable></blockWidget><paragraph>b]ar</paragraph>',
-					'<blockWidget><nestedEditable>fo[]</nestedEditable></blockWidget><paragraph>ar</paragraph>',
-					{ merge: true }
+					'<blockWidget><nestedEditable>fo[]</nestedEditable></blockWidget><paragraph>ar</paragraph>'
 				);
 
 				test(
 					'does not merge an object element (if it is second)',
 					'<paragraph>ba[r</paragraph><blockWidget><nestedEditable>f]oo</nestedEditable></blockWidget>',
-					'<paragraph>ba[]</paragraph><blockWidget><nestedEditable>oo</nestedEditable></blockWidget>',
-					{ merge: true }
+					'<paragraph>ba[]</paragraph><blockWidget><nestedEditable>oo</nestedEditable></blockWidget>'
 				);
 			} );
 		} );
@@ -611,22 +596,19 @@ describe( 'DataController', () => {
 			test(
 				'merge option should be ignored if both elements are limits',
 				'<inlineLimit>foo [bar</inlineLimit><inlineLimit>baz] qux</inlineLimit>',
-				'<inlineLimit>foo []</inlineLimit><inlineLimit> qux</inlineLimit>',
-				{ merge: true }
+				'<inlineLimit>foo []</inlineLimit><inlineLimit> qux</inlineLimit>'
 			);
 
 			test(
 				'merge option should be ignored if the first element is a limit',
 				'<inlineLimit>foo [bar</inlineLimit><x>baz] qux</x>',
-				'<inlineLimit>foo []</inlineLimit><x> qux</x>',
-				{ merge: true }
+				'<inlineLimit>foo []</inlineLimit><x> qux</x>'
 			);
 
 			test(
 				'merge option should be ignored if the second element is a limit',
 				'<x>baz [qux</x><inlineLimit>foo] bar</inlineLimit>',
-				'<x>baz []</x><inlineLimit> bar</inlineLimit>',
-				{ merge: true }
+				'<x>baz []</x><inlineLimit> bar</inlineLimit>'
 			);
 		} );
 
@@ -648,14 +630,14 @@ describe( 'DataController', () => {
 			test(
 				'should delete inside block limit element',
 				'<blockLimit><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph></blockLimit>',
-				'<blockLimit><paragraph>fo[]</paragraph><paragraph>ar</paragraph></blockLimit>'
+				'<blockLimit><paragraph>fo[]</paragraph><paragraph>ar</paragraph></blockLimit>',
+				{ leaveUnmerged: true }
 			);
 
 			test(
 				'should delete inside block limit element (with merge)',
 				'<blockLimit><paragraph>fo[o</paragraph><paragraph>b]ar</paragraph></blockLimit>',
-				'<blockLimit><paragraph>fo[]ar</paragraph></blockLimit>',
-				{ merge: true }
+				'<blockLimit><paragraph>fo[]ar</paragraph></blockLimit>'
 			);
 
 			test(
@@ -673,8 +655,7 @@ describe( 'DataController', () => {
 			test(
 				'merge option should be ignored if any of the elements is a limit',
 				'<blockLimit><paragraph>foo [bar</paragraph></blockLimit><blockLimit><paragraph>baz] qux</paragraph></blockLimit>',
-				'<blockLimit><paragraph>foo []</paragraph></blockLimit><blockLimit><paragraph> qux</paragraph></blockLimit>',
-				{ merge: true }
+				'<blockLimit><paragraph>foo []</paragraph></blockLimit><blockLimit><paragraph> qux</paragraph></blockLimit>'
 			);
 		} );
 

+ 102 - 29
packages/ckeditor5-engine/tests/model/document/document.js

@@ -280,12 +280,18 @@ describe( 'Document', () => {
 
 		beforeEach( () => {
 			doc.schema.registerItem( 'paragraph', '$block' );
+
 			doc.schema.registerItem( 'emptyBlock' );
 			doc.schema.allow( { name: 'emptyBlock', inside: '$root' } );
-			doc.schema.registerItem( 'widget', '$block' );
+
+			doc.schema.registerItem( 'widget' );
 			doc.schema.allow( { name: 'widget', inside: '$root' } );
 			doc.schema.objects.add( 'widget' );
 
+			doc.schema.registerItem( 'blockWidget', '$block' );
+			doc.schema.allow( { name: 'blockWidget', inside: '$root' } );
+			doc.schema.objects.add( 'blockWidget' );
+
 			doc.createRoot();
 			selection = doc.selection;
 		} );
@@ -374,34 +380,6 @@ describe( 'Document', () => {
 		);
 
 		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',
@@ -415,6 +393,101 @@ describe( 'Document', () => {
 			'<paragraph></paragraph><paragraph>[]</paragraph>'
 		);
 
+		describe( 'in case of objects which do not allow text inside', () => {
+			test(
+				'should select nearest object (o[]o) - both',
+				'<widget></widget>[]<widget></widget>',
+				'both',
+				'[<widget></widget>]<widget></widget>'
+			);
+
+			test(
+				'should select nearest object (o[]o) - forward',
+				'<widget></widget>[]<widget></widget>',
+				'forward',
+				'<widget></widget>[<widget></widget>]'
+			);
+
+			test(
+				'should select nearest object (o[]o) - backward',
+				'<widget></widget>[]<widget></widget>',
+				'both',
+				'[<widget></widget>]<widget></widget>'
+			);
+
+			test(
+				'should select nearest object (p[]o) - forward',
+				'<paragraph></paragraph>[]<widget></widget>',
+				'forward',
+				'<paragraph></paragraph>[<widget></widget>]'
+			);
+
+			test(
+				'should select nearest object (o[]p) - both',
+				'<widget></widget>[]<paragraph></paragraph>',
+				'both',
+				'[<widget></widget>]<paragraph></paragraph>'
+			);
+
+			test(
+				'should select nearest object (o[]p) - backward',
+				'<widget></widget>[]<paragraph></paragraph>',
+				'backward',
+				'[<widget></widget>]<paragraph></paragraph>'
+			);
+
+			test(
+				'should select nearest object ([]o) - both',
+				'[]<widget></widget><paragraph></paragraph>',
+				'both',
+				'[<widget></widget>]<paragraph></paragraph>'
+			);
+
+			test(
+				'should select nearest object ([]o) - forward',
+				'[]<widget></widget><paragraph></paragraph>',
+				'forward',
+				'[<widget></widget>]<paragraph></paragraph>'
+			);
+
+			test(
+				'should select nearest object (o[]) - both',
+				'<paragraph></paragraph><widget></widget>[]',
+				'both',
+				'<paragraph></paragraph>[<widget></widget>]'
+			);
+
+			test(
+				'should select nearest object (o[]) - backward',
+				'<paragraph></paragraph><widget></widget>[]',
+				'both',
+				'<paragraph></paragraph>[<widget></widget>]'
+			);
+		} );
+
+		describe( 'in case of objects which allow text inside', () => {
+			test(
+				'should select nearest object which allows text (o[]o) - both',
+				'<blockWidget></blockWidget>[]<blockWidget></blockWidget>',
+				'both',
+				'[<blockWidget></blockWidget>]<blockWidget></blockWidget>'
+			);
+
+			test(
+				'should select nearest object (o[]p) - both',
+				'<blockWidget></blockWidget>[]<paragraph></paragraph>',
+				'both',
+				'[<blockWidget></blockWidget>]<paragraph></paragraph>'
+			);
+
+			test(
+				'should select nearest object which allows text ([]o) - both',
+				'[]<blockWidget></blockWidget><paragraph></paragraph>',
+				'both',
+				'[<blockWidget></blockWidget>]<paragraph></paragraph>'
+			);
+		} );
+
 		function test( testName, data, direction, expected ) {
 			it( testName, () => {
 				setData( doc, data );

+ 70 - 10
packages/ckeditor5-engine/tests/model/liveselection.js

@@ -43,6 +43,9 @@ describe( 'LiveSelection', () => {
 		selection = doc.selection;
 		doc.schema.registerItem( 'p', '$block' );
 
+		doc.schema.registerItem( 'widget' );
+		doc.schema.objects.add( 'widget' );
+
 		liveRange = new LiveRange( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) );
 		range = new Range( new Position( root, [ 2 ] ), new Position( root, [ 2, 2 ] ) );
 	} );
@@ -99,9 +102,66 @@ describe( 'LiveSelection', () => {
 	} );
 
 	describe( 'isCollapsed', () => {
-		it( 'should return true for default range', () => {
+		it( 'should be true for the default range (in text position)', () => {
 			expect( selection.isCollapsed ).to.be.true;
 		} );
+
+		it( 'should be false for the default range (object selection) ', () => {
+			root.insertChildren( 0, new Element( 'widget' ) );
+
+			expect( selection.isCollapsed ).to.be.false;
+		} );
+
+		it( 'should back off to the default algorithm if selection has ranges', () => {
+			selection.addRange( range );
+
+			expect( selection.isCollapsed ).to.be.false;
+		} );
+	} );
+
+	describe( 'anchor', () => {
+		it( 'should equal the default range\'s start (in text position)', () => {
+			const expectedPos = new Position( root, [ 0, 0 ] );
+
+			expect( selection.anchor.isEqual( expectedPos ) ).to.be.true;
+		} );
+
+		it( 'should equal the default range\'s start (object selection)', () => {
+			root.insertChildren( 0, new Element( 'widget' ) );
+
+			const expectedPos = new Position( root, [ 0 ] );
+
+			expect( selection.anchor.isEqual( expectedPos ) ).to.be.true;
+		} );
+
+		it( 'should back off to the default algorithm if selection has ranges', () => {
+			selection.addRange( range );
+
+			expect( selection.anchor.isEqual( range.start ) ).to.be.true;
+		} );
+	} );
+
+	describe( 'focus', () => {
+		it( 'should equal the default range\'s end (in text position)', () => {
+			const expectedPos = new Position( root, [ 0, 0 ] );
+
+			expect( selection.focus.isEqual( expectedPos ) ).to.be.true;
+		} );
+
+		it( 'should equal the default range\'s end (object selection)', () => {
+			root.insertChildren( 0, new Element( 'widget' ) );
+
+			const expectedPos = new Position( root, [ 1 ] );
+
+			expect( selection.focus.isEqual( expectedPos ) ).to.be.true;
+			expect( selection.focus.isEqual( selection.getFirstRange().end ) ).to.be.true;
+		} );
+
+		it( 'should back off to the default algorithm if selection has ranges', () => {
+			selection.addRange( range );
+
+			expect( selection.focus.isEqual( range.end ) ).to.be.true;
+		} );
 	} );
 
 	describe( 'rangeCount', () => {
@@ -118,7 +178,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'addRange', () => {
+	describe( 'addRange()', () => {
 		it( 'should convert added Range to LiveRange', () => {
 			selection.addRange( range );
 
@@ -149,7 +209,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'collapse', () => {
+	describe( 'collapse()', () => {
 		it( 'detaches all existing ranges', () => {
 			selection.addRange( range );
 			selection.addRange( liveRange );
@@ -161,7 +221,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'destroy', () => {
+	describe( 'destroy()', () => {
 		it( 'should unbind all events', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
@@ -181,7 +241,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'setFocus', () => {
+	describe( 'setFocus()', () => {
 		it( 'modifies default range', () => {
 			const startPos = selection.getFirstPosition();
 			const endPos = Position.createAt( root, 'end' );
@@ -206,7 +266,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'removeAllRanges', () => {
+	describe( 'removeAllRanges()', () => {
 		let spy, ranges;
 
 		beforeEach( () => {
@@ -249,7 +309,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'setRanges', () => {
+	describe( 'setRanges()', () => {
 		it( 'should throw an error when range is invalid', () => {
 			expect( () => {
 				selection.setRanges( [ { invalid: 'range' } ] );
@@ -295,7 +355,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'getFirstRange', () => {
+	describe( 'getFirstRange()', () => {
 		it( 'should return default range if no ranges were added', () => {
 			const firstRange = selection.getFirstRange();
 
@@ -304,7 +364,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'getLastRange', () => {
+	describe( 'getLastRange()', () => {
 		it( 'should return default range if no ranges were added', () => {
 			const lastRange = selection.getLastRange();
 
@@ -313,7 +373,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( 'createFromSelection', () => {
+	describe( 'createFromSelection()', () => {
 		it( 'should return a LiveSelection instance', () => {
 			selection.addRange( range, true );
 

+ 31 - 23
packages/ckeditor5-engine/tests/model/position.js

@@ -15,7 +15,7 @@ import { jsonParseStringify } from '../../tests/model/_utils/utils';
 
 testUtils.createSinonSandbox();
 
-describe( 'position', () => {
+describe( 'Position', () => {
 	let doc, root, otherRoot, p, ul, li1, li2, f, o, z, b, a, r, foz, bar;
 
 	// root
@@ -118,7 +118,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'createFromParentAndOffset', () => {
+	describe( 'createFromParentAndOffset()', () => {
 		it( 'should create positions form node and offset', () => {
 			expect( Position.createFromParentAndOffset( root, 0 ) ).to.have.property( 'path' ).that.deep.equals( [ 0 ] );
 			expect( Position.createFromParentAndOffset( root, 1 ) ).to.have.property( 'path' ).that.deep.equals( [ 1 ] );
@@ -149,7 +149,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'createAt', () => {
+	describe( 'createAt()', () => {
 		it( 'should create positions from positions', () => {
 			const spy = testUtils.sinon.spy( Position, 'createFromPosition' );
 
@@ -177,7 +177,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'createBefore', () => {
+	describe( 'createBefore()', () => {
 		it( 'should create positions before elements', () => {
 			expect( Position.createBefore( p ) ).to.have.property( 'path' ).that.deep.equals( [ 0 ] );
 
@@ -203,7 +203,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'createAfter', () => {
+	describe( 'createAfter()', () => {
 		it( 'should create positions after elements', () => {
 			expect( Position.createAfter( p ) ).to.have.property( 'path' ).that.deep.equals( [ 1 ] );
 
@@ -229,7 +229,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'createFromPosition', () => {
+	describe( 'createFromPosition()', () => {
 		it( 'should create a copy of given position', () => {
 			const original = new Position( root, [ 1, 2, 3 ] );
 			const position = Position.createFromPosition( original );
@@ -361,7 +361,7 @@ describe( 'position', () => {
 		expect( position.getParentPath() ).to.deep.equal( [ 1, 2 ] );
 	} );
 
-	describe( 'isBefore', () => {
+	describe( 'isBefore()', () => {
 		it( 'should return true if given position has same root and is before this position', () => {
 			const position = new Position( root, [ 1, 1, 2 ] );
 			const beforePosition = new Position( root, [ 1, 0 ] );
@@ -384,7 +384,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'isEqual', () => {
+	describe( 'isEqual()', () => {
 		it( 'should return true if given position has same path and root', () => {
 			const position = new Position( root, [ 1, 1, 2 ] );
 			const samePosition = new Position( root, [ 1, 1, 2 ] );
@@ -407,7 +407,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'isAfter', () => {
+	describe( 'isAfter()', () => {
 		it( 'should return true if given position has same root and is after this position', () => {
 			const position = new Position( root, [ 1, 1, 2 ] );
 			const afterPosition = new Position( root, [ 1, 2 ] );
@@ -430,7 +430,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'isTouching', () => {
+	describe( 'isTouching()', () => {
 		it( 'should return true if positions are same', () => {
 			const position = new Position( root, [ 1, 1, 1 ] );
 			const result = position.isTouching( new Position( root, [ 1, 1, 1 ] ) );
@@ -470,6 +470,14 @@ describe( 'position', () => {
 			expect( positionB.isTouching( positionA ) ).to.be.false;
 		} );
 
+		it( 'should return false if there are whole nodes between positions (same depth)', () => {
+			const positionA = new Position( root, [ 1, 0 ] );
+			const positionB = new Position( root, [ 1, 1 ] );
+
+			expect( positionA.isTouching( positionB ) ).to.be.false;
+			expect( positionB.isTouching( positionA ) ).to.be.false;
+		} );
+
 		it( 'should return false if there are whole nodes between positions', () => {
 			const positionA = new Position( root, [ 1, 0, 3 ] );
 			const positionB = new Position( root, [ 1, 1, 1 ] );
@@ -487,21 +495,21 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'isAtStart', () => {
+	describe( 'isAtStart()', () => {
 		it( 'should return true if position is at the beginning of its parent', () => {
 			expect( new Position( root, [ 0 ] ).isAtStart ).to.be.true;
 			expect( new Position( root, [ 1 ] ).isAtStart ).to.be.false;
 		} );
 	} );
 
-	describe( 'isAtEnd', () => {
+	describe( 'isAtEnd()', () => {
 		it( 'should return true if position is at the end of its parent', () => {
 			expect( new Position( root, [ root.maxOffset ] ).isAtEnd ).to.be.true;
 			expect( new Position( root, [ 0 ] ).isAtEnd ).to.be.false;
 		} );
 	} );
 
-	describe( 'getAncestors', () => {
+	describe( 'getAncestors()', () => {
 		it( 'should return position parent element and it\'s ancestors', () => {
 			expect( new Position( root, [ 1, 1, 1 ] ).getAncestors() ).to.deep.equal( [ root, ul, li2 ] );
 		} );
@@ -513,7 +521,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'getCommonPath', () => {
+	describe( 'getCommonPath()', () => {
 		it( 'returns the common part', () => {
 			const pos1 = new Position( root, [ 1, 0, 0 ] );
 			const pos2 = new Position( root, [ 1, 0, 1 ] );
@@ -548,7 +556,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'compareWith', () => {
+	describe( 'compareWith()', () => {
 		it( 'should return same if positions are same', () => {
 			const position = new Position( root, [ 1, 2, 3 ] );
 			const compared = new Position( root, [ 1, 2, 3 ] );
@@ -578,7 +586,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'getLastMatchingPosition', () => {
+	describe( 'getLastMatchingPosition()', () => {
 		it( 'should skip forward', () => {
 			let position = new Position( root, [ 1, 0, 0 ] );
 
@@ -596,7 +604,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( '_getTransformedByInsertion', () => {
+	describe( '_getTransformedByInsertion()', () => {
 		it( 'should return a new Position instance', () => {
 			const position = new Position( root, [ 0 ] );
 			const transformed = position._getTransformedByInsertion( new Position( root, [ 2 ] ), 4, false );
@@ -656,7 +664,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( '_getTransformedByDeletion', () => {
+	describe( '_getTransformedByDeletion()', () => {
 		it( 'should return a new Position instance', () => {
 			const position = new Position( root, [ 0 ] );
 			const transformed = position._getTransformedByDeletion( new Position( root, [ 2 ] ), 4 );
@@ -716,7 +724,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( '_getTransformedByMove', () => {
+	describe( '_getTransformedByMove()', () => {
 		it( 'should increment offset if a range was moved to the same parent and closer offset', () => {
 			const position = new Position( root, [ 1, 2, 3 ] );
 			const transformed = position._getTransformedByMove( new Position( root, [ 2 ] ), new Position( root, [ 1, 2, 0 ] ), 3, false );
@@ -758,7 +766,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( '_getCombined', () => {
+	describe( '_getCombined()', () => {
 		it( 'should return correct combination of this and given positions', () => {
 			const position = new Position( root, [ 1, 3, 4, 2 ] );
 			const sourcePosition = new Position( root, [ 1, 1 ] );
@@ -770,7 +778,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'getShiftedBy', () => {
+	describe( 'getShiftedBy()', () => {
 		it( 'should return a new instance of Position with offset changed by shift value', () => {
 			const position = new Position( root, [ 1, 2, 3 ] );
 			const shifted = position.getShiftedBy( 2 );
@@ -795,7 +803,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'toJSON', () => {
+	describe( 'toJSON()', () => {
 		it( 'should serialize position', () => {
 			const position = new Position( root, [ 0 ] );
 
@@ -813,7 +821,7 @@ describe( 'position', () => {
 		} );
 	} );
 
-	describe( 'fromJSON', () => {
+	describe( 'fromJSON()', () => {
 		it( 'should create object with given document', () => {
 			const deserialized = Position.fromJSON( { root: 'main', path: [ 0, 1, 2 ] }, doc );
 

+ 67 - 6
packages/ckeditor5-engine/tests/model/selection.js

@@ -865,6 +865,7 @@ describe( 'Selection', () => {
 
 			doc.schema.registerItem( 'image' );
 			doc.schema.allow( { name: 'image', inside: '$root' } );
+			doc.schema.allow( { name: 'image', inside: '$block' } );
 			doc.schema.allow( { name: '$text', inside: 'image' } );
 
 			// Special block which can contain another blocks.
@@ -884,6 +885,15 @@ describe( 'Selection', () => {
 			expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'b' ] );
 		} );
 
+		it( 'returns block for a collapsed selection (empty block)', () => {
+			setData( doc, '<p>a</p><p>[]</p><p>c</p>' );
+
+			const blocks = Array.from( doc.selection.getSelectedBlocks() );
+
+			expect( blocks ).to.have.length( 1 );
+			expect( blocks[ 0 ].childCount ).to.equal( 0 );
+		} );
+
 		it( 'returns block for a non collapsed selection', () => {
 			setData( doc, '<p>a</p><p>[b]</p><p>c</p>' );
 
@@ -896,11 +906,8 @@ describe( 'Selection', () => {
 			expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'b', 'c' ] );
 		} );
 
-		// This isn't a behaviour that we like (https://github.com/ckeditor/ckeditor5-heading/issues/9) but I don't
-		// want to work on this issue now, when porting this method from the list/heading features.
-		// It has to be covered separately.
-		it( 'returns two blocks for a non collapsed selection if only end of selection is touching a block', () => {
-			setData( doc, '<p>a</p><h>b[</h><p>]c</p><p>d</p>' );
+		it( 'returns two blocks for a non collapsed selection (starts at block end)', () => {
+			setData( doc, '<p>a</p><h>b[</h><p>c]</p><p>d</p>' );
 
 			expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'b', 'c' ] );
 		} );
@@ -980,8 +987,62 @@ describe( 'Selection', () => {
 			expect( toText( doc.selection.getSelectedBlocks() ) ).to.be.empty;
 		} );
 
+		describe( '#984', () => {
+			it( 'does not return the last block if none of its content is selected', () => {
+				setData( doc, '<p>[a</p><p>b</p><p>]c</p>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a', 'b' ] );
+			} );
+
+			it( 'returns only the first block for a non collapsed selection if only end of selection is touching a block', () => {
+				setData( doc, '<p>a</p><h>b[</h><p>]c</p><p>d</p>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'b' ] );
+			} );
+
+			it( 'does not return the last block if none of its content is selected (nested case)', () => {
+				setData( doc, '<p>[a</p><nestedBlock><nestedBlock>]b</nestedBlock></nestedBlock>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a' ] );
+			} );
+
+			// Like a super edge case, we can live with this behavior as I don't even know what we could expect here
+			// since only the innermost block is considerd a block to return (so the <nB>b...</nB> needs to be ignored).
+			it( 'does not return the last block if none of its content is selected (nested case, wrapper with a content)', () => {
+				setData( doc, '<p>[a</p><nestedBlock>b<nestedBlock>]c</nestedBlock></nestedBlock>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a' ] );
+			} );
+
+			it( 'returns the last block if at least one of its child nodes is selected', () => {
+				setData( doc, '<p>[a</p><p>b</p><p><image></image>]c</p>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a', 'b', 'c' ] );
+			} );
+
+			// I needed these last 2 cases to justify the use of isTouching() instead of simple `offset == 0` check.
+			it( 'returns the last block if at least one of its child nodes is selected (end in an inline element)', () => {
+				setData( doc, '<p>[a</p><p>b</p><p><image>x]</image>c</p>' );
+
+				expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a', 'b', 'c' ] );
+			} );
+
+			it(
+				'does not return the last block if at least one of its child nodes is selected ' +
+				'(end in an inline element, no content selected)',
+				() => {
+					setData( doc, '<p>[a</p><p>b</p><p><image>]x</image>c</p>' );
+
+					expect( toText( doc.selection.getSelectedBlocks() ) ).to.deep.equal( [ 'a', 'b' ] );
+				}
+			);
+		} );
+
+		// Map all elements to data of its first child text node.
 		function toText( elements ) {
-			return Array.from( elements ).map( el => el.getChild( 0 ).data );
+			return Array.from( elements ).map( el => {
+				return Array.from( el.getChildren() ).find( child => child.data ).data;
+			} );
 		}
 	} );
 

+ 58 - 0
packages/ckeditor5-engine/tests/tickets/699.js

@@ -0,0 +1,58 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals document */
+
+import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+
+import buildViewConverter from '../../src/conversion/buildviewconverter';
+import buildModelConverter from '../../src/conversion/buildmodelconverter';
+
+import { getData as getModelData } from '../../src/dev-utils/model';
+import { getData as getViewData } from '../../src/dev-utils/view';
+
+describe( 'Bug ckeditor5-engine#699', () => {
+	let element;
+
+	beforeEach( () => {
+		element = document.createElement( 'div' );
+
+		document.body.appendChild( element );
+	} );
+
+	afterEach( () => {
+		element.remove();
+	} );
+
+	it( 'the engine sets the initial selection on the first widget', () => {
+		return ClassicTestEditor
+			.create( element, { plugins: [ Paragraph, WidgetPlugin ] } )
+			.then( editor => {
+				editor.setData( '<widget></widget><p>foo</p>' );
+
+				expect( getModelData( editor.document ) ).to.equal( '[<widget></widget>]<paragraph>foo</paragraph>' );
+				expect( getViewData( editor.editing.view ) ).to.equal( '[<widget></widget>]<p>foo</p>' );
+
+				return editor.destroy();
+			} );
+	} );
+} );
+
+function WidgetPlugin( editor ) {
+	const schema = editor.document.schema;
+
+	schema.registerItem( 'widget' );
+	schema.allow( { name: 'widget', inside: '$root' } );
+	schema.objects.add( 'widget' );
+
+	buildModelConverter().for( editor.data.modelToView, editor.editing.modelToView )
+		.fromElement( 'widget' )
+		.toElement( 'widget' );
+
+	buildViewConverter().for( editor.data.viewToModel )
+		.fromElement( 'widget' )
+		.toElement( 'widget' );
+}