Merge pull request #709 from ckeditor/t/708

Added objects handling to modifySelection method.
ckeditor · Dec 1, 2016 · 24c9fac · 24c9fac
2 parents 5a9f64b + 7844585
commit 24c9fac
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 25 deletions.
diff --git a/src/controller/datacontroller.js b/src/controller/datacontroller.js
@@ -119,7 +119,7 @@ export default class DataController {
 
 		this.on( 'insertContent', ( evt, data ) => insertContent( this, data.content, data.selection, data.batch ) );
 		this.on( 'deleteContent', ( evt, data ) => deleteContent( data.selection, data.batch, data.options ) );
-		this.on( 'modifySelection', ( evt, data ) => modifySelection( data.selection, data.options ) );
+		this.on( 'modifySelection', ( evt, data ) => modifySelection( this, data.selection, data.options ) );
 		this.on( 'getSelectedContent', ( evt, data ) => {
 			data.content = getSelectedContent( data.selection );
 		} );

diff --git a/src/controller/modifyselection.js b/src/controller/modifyselection.js
@@ -33,12 +33,15 @@ import { isInsideSurrogatePair, isInsideCombinedSymbol } from '../../utils/unico
  *
  * **Note:** if you extend a forward selection in a backward direction you will in fact shrink it.
  *
+ * @param {module:engine/controller/datacontroller~DataController} dataController The data controller in context of which
+ * the selection modification should be performed.
  * @param {module:engine/model/selection~Selection} selection The selection to modify.
  * @param {Object} [options]
  * @param {'forward'|'backward'} [options.direction='forward'] The direction in which the selection should be modified.
  * @param {'character'|'codePoint'} [options.unit='character'] The unit by which selection should be modified.
  */
-export default function modifySelection( selection, options = {} ) {
+export default function modifySelection( dataController, selection, options = {} ) {
+	const schema = dataController.model.schema;
 	const isForward = options.direction != 'backward';
 	options.unit = options.unit ? options.unit : 'character';
 
@@ -88,9 +91,14 @@ export default function modifySelection( selection, options = {} ) {
 	if ( value.type == 'text' ) {
 		selection.setFocus( value.previousPosition );
 	}
-	// 4.3. An element found after leaving previous element. Put focus inside that element, at it's beginning or end.
+	// 4.3. An element found after leaving previous element.
+	// When element is an object - put focus before or after that element, otherwise put it inside that element,
+	// at it's beginning or end.
 	else {
-		selection.setFocus( value.item, isForward ? 0 : 'end' );
+		const isObject = schema.objects.has( value.item.name );
+		const offset = isObject ? ( isForward ? 'after' : 'before' ) : ( isForward ? 0 : 'end' );
+
+		selection.setFocus( value.item, offset );
 	}
 }
 

diff --git a/tests/controller/modifyselection.js b/tests/controller/modifyselection.js
@@ -4,15 +4,17 @@
  */
 
 import Document from 'ckeditor5/engine/model/document.js';
+import DataController from 'ckeditor5/engine/controller/datacontroller.js';
 import Selection from 'ckeditor5/engine/model/selection.js';
 import modifySelection from 'ckeditor5/engine/controller/modifyselection.js';
 import { setData, stringify } from 'ckeditor5/engine/dev-utils/model.js';
 
 describe( 'DataController', () => {
-	let document;
+	let document, dataController;
 
 	beforeEach( () => {
 		document = new Document();
+		dataController = new DataController( document );
 		document.schema.registerItem( 'p', '$block' );
 		document.schema.registerItem( 'x', '$block' );
 		document.schema.registerItem( 'img', '$inline' );
@@ -64,7 +66,7 @@ describe( 'DataController', () => {
 				it( 'extends one character backward', () => {
 					setData( document, '<p>fo[]o</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>f[o]o</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -79,7 +81,7 @@ describe( 'DataController', () => {
 				it( 'extends one character backward (non-collapsed)', () => {
 					setData( document, '<p>foob[a]r</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>foo[ba]r</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -94,7 +96,7 @@ describe( 'DataController', () => {
 				it( 'extends to element boundary (backward)', () => {
 					setData( document, '<p>f[]oo</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>[f]oo</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -110,7 +112,7 @@ describe( 'DataController', () => {
 				it( 'shrinks backward selection (to collapsed)', () => {
 					setData( document, '<p>foo[b]ar</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection );
+					modifySelection( dataController, document.selection );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>foob[]ar</p>' );
 					expect( document.selection.isBackward ).to.false;
@@ -131,7 +133,7 @@ describe( 'DataController', () => {
 				it( 'extends one element backward', () => {
 					setData( document, '<p>fo<img></img>[]o</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>fo[<img></img>]o</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -146,7 +148,7 @@ describe( 'DataController', () => {
 				it( 'unicode support - combining mark backward', () => {
 					setData( document, '<p>foob̂[]ar</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>foo[b̂]ar</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -161,7 +163,7 @@ describe( 'DataController', () => {
 				it( 'unicode support - combining mark multiple backward', () => {
 					setData( document, '<p>foo̻̐ͩ[]bar</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>fo[o̻̐ͩ]bar</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -182,7 +184,7 @@ describe( 'DataController', () => {
 				it( 'unicode support - surrogate pairs backward', () => {
 					setData( document, '<p>\uD83D\uDCA9[]</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>[\uD83D\uDCA9]</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -199,7 +201,7 @@ describe( 'DataController', () => {
 				it( 'extends over boundary of empty elements (backward)', () => {
 					setData( document, '<p></p><p></p><p>[]</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p></p><p>[</p><p>]</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -214,7 +216,7 @@ describe( 'DataController', () => {
 				it( 'extends over boundary of non-empty elements (backward)', () => {
 					setData( document, '<p>a</p><p>[]bcd</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>a[</p><p>]bcd</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -229,7 +231,7 @@ describe( 'DataController', () => {
 				it( 'extends over character after boundary (backward)', () => {
 					setData( document, '<p>abc[</p><p>]d</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>ab[c</p><p>]d</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -256,7 +258,7 @@ describe( 'DataController', () => {
 				it( 'extends over element when next node is a text (backward)', () => {
 					setData( document, 'ab<p>[]c</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( 'ab[<p>]c</p>' );
 					expect( document.selection.isBackward ).to.true;
@@ -265,7 +267,7 @@ describe( 'DataController', () => {
 				it( 'shrinks over boundary of empty elements', () => {
 					setData( document, '<p>[</p><p>]</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection );
+					modifySelection( dataController, document.selection );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p></p><p>[]</p>' );
 					expect( document.selection.isBackward ).to.false;
@@ -274,7 +276,7 @@ describe( 'DataController', () => {
 				it( 'shrinks over boundary of empty elements (backward)', () => {
 					setData( document, '<p>[</p><p>]</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>[]</p><p></p>' );
 					expect( document.selection.isBackward ).to.false;
@@ -283,7 +285,7 @@ describe( 'DataController', () => {
 				it( 'shrinks over boundary of non-empty elements', () => {
 					setData( document, '<p>a[</p><p>]b</p>', { lastRangeBackward: true } );
 
-					modifySelection( document.selection );
+					modifySelection( dataController, document.selection );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>a</p><p>[]b</p>' );
 					expect( document.selection.isBackward ).to.false;
@@ -299,7 +301,7 @@ describe( 'DataController', () => {
 				it( 'updates selection attributes', () => {
 					setData( document, '<p><$text bold="true">foo</$text>[b]</p>' );
 
-					modifySelection( document.selection, { direction: 'backward' } );
+					modifySelection( dataController, document.selection, { direction: 'backward' } );
 
 					expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p><$text bold="true">foo[]</$text>b</p>' );
 					expect( document.selection.getAttribute( 'bold' ) ).to.equal( true );
@@ -332,7 +334,7 @@ describe( 'DataController', () => {
 			it( 'extends one user-perceived character backward - latin letters', () => {
 				setData( document, '<p>fo[]o</p>' );
 
-				modifySelection( document.selection, { unit: 'codePoint', direction: 'backward' } );
+				modifySelection( dataController, document.selection, { unit: 'codePoint', direction: 'backward' } );
 
 				expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>f[o]o</p>' );
 				expect( document.selection.isBackward ).to.true;
@@ -352,7 +354,7 @@ describe( 'DataController', () => {
 				// Document's selection will throw errors in some test cases (which are correct cases, but only for
 				// non-document selections).
 				const testSelection = Selection.createFromSelection( document.selection );
-				modifySelection( testSelection, { unit: 'codePoint', direction: 'backward' } );
+				modifySelection( dataController, testSelection, { unit: 'codePoint', direction: 'backward' } );
 
 				expect( stringify( document.getRoot(), testSelection ) ).to.equal( '<p>foob[̂]ar</p>' );
 				expect( testSelection.isBackward ).to.true;
@@ -375,12 +377,63 @@ describe( 'DataController', () => {
 			it( 'unicode support surrogate pairs backward', () => {
 				setData( document, '<p>\uD83D\uDCA9[]</p>' );
 
-				modifySelection( document.selection, { unit: 'codePoint', direction: 'backward' } );
+				modifySelection( dataController, document.selection, { unit: 'codePoint', direction: 'backward' } );
 
 				expect( stringify( document.getRoot(), document.selection ) ).to.equal( '<p>[\uD83D\uDCA9]</p>' );
 				expect( document.selection.isBackward ).to.true;
 			} );
 		} );
+
+		describe( 'objects handling', () => {
+			beforeEach( () => {
+				document.schema.registerItem( 'obj' );
+				document.schema.allow( { name: 'obj', inside: '$root' } );
+				document.schema.objects.add( 'obj' );
+				document.schema.registerItem( 'inlineObj', '$inline' );
+				document.schema.objects.add( 'inlineObj' );
+			} );
+
+			test(
+				'extends over next object element when at the end of an element',
+				'<p>foo[]</p><obj>bar</obj>',
+				'<p>foo[</p><obj>bar</obj>]'
+			);
+
+			test(
+				'extends over previous object element when at the beginning of an element ',
+				'<obj>bar</obj><p>[]foo</p>',
+				'[<obj>bar</obj><p>]foo</p>',
+				{ direction: 'backward' }
+			);
+
+			test(
+				'extends over object elements - forward',
+				'[<obj></obj>]<obj></obj>',
+				'[<obj></obj><obj></obj>]'
+			);
+
+			it( 'extends over object elements - backward', () => {
+				setData( document, '<obj></obj>[<obj></obj>]', { lastRangeBackward: true } );
+
+				modifySelection( dataController, document.selection, { direction: 'backward' } );
+
+				expect( stringify( document.getRoot(), document.selection ) ).to.equal( '[<obj></obj><obj></obj>]' );
+				expect( document.selection.isBackward ).to.true;
+			} );
+
+			test(
+				'extends over inline objects - forward',
+				'<p>foo[]<inlineObj>bar</inlineObj></p>',
+				'<p>foo[<inlineObj>bar</inlineObj>]</p>'
+			);
+
+			test(
+				'extends over inline objects - backward',
+				'<p><inlineObj>bar</inlineObj>[]foo</p>',
+				'<p>[<inlineObj>bar</inlineObj>]foo</p>',
+				{ direction: 'backward' }
+			);
+		} );
 	} );
 
 	function test( title, input, output, options ) {
@@ -394,7 +447,7 @@ describe( 'DataController', () => {
 			// Document's selection will throw errors in some test cases (which are correct cases, but only for
 			// non-document selections).
 			const testSelection = Selection.createFromSelection( document.selection );
-			modifySelection( testSelection, options );
+			modifySelection( dataController, testSelection, options );
 
 			expect( stringify( document.getRoot(), testSelection ) ).to.equal( output );
 		} );