Merge pull request #219 from ckeditor/t/ckeditor5/1955

Fix: Autoformat transformations in blocks containing inline elements. See ckeditor/ckeditor5#1955.

src/texttransformation.js

@@ -118,8 +118,7 @@ export default class TextTransformation extends Plugin {
 				const matches = from.exec( data.text );
 				const replaces = to( matches.slice( 1 ) );
 
-				// Used `focus` to be in line with `TextWatcher#_getText()`.
-				const selectionParent = editor.model.document.selection.focus.parent;
+				const matchedRange = data.range;
 
 				let changeIndex = matches.index;
 
@@ -134,7 +133,7 @@ export default class TextTransformation extends Plugin {
 							continue;
 						}
 
-						const replacePosition = model.createPositionAt( selectionParent, changeIndex );
+						const replacePosition = matchedRange.start.getShiftedBy( changeIndex );
 						const replaceRange = model.createRange( replacePosition, replacePosition.getShiftedBy( match.length ) );
 						const attributes = getTextAttributesAfterPosition( replacePosition );
 

src/textwatcher.js

@@ -9,6 +9,7 @@
 
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
+import getLastTextLine from './utils/getlasttextline';
 
 /**
  * The text watcher feature.
@@ -82,7 +83,13 @@ export default class TextWatcher {
 	 * @param {Object} data Data object for event.
 	 */
 	_evaluateTextBeforeSelection( suffix, data = {} ) {
-		const text = this._getText();
+		const model = this.model;
+		const document = model.document;
+		const selection = document.selection;
+
+		const rangeBeforeSelection = model.createRange( model.createPositionAt( selection.focus.parent, 0 ), selection.focus );
+
+		const { text, range } = getLastTextLine( rangeBeforeSelection, model );
 
 		const textHasMatch = this.testCallback( text );
 
@@ -98,7 +105,7 @@ export default class TextWatcher {
 		this.hasMatch = textHasMatch;
 
 		if ( textHasMatch ) {
-			const eventData = Object.assign( data, { text } );
+			const eventData = Object.assign( data, { text, range } );
 
 			/**
 			 * Fired whenever the text watcher found a match for data changes.
@@ -118,37 +125,6 @@ export default class TextWatcher {
 			this.fire( `matched:${ suffix }`, eventData );
 		}
 	}
-
-	/**
-	 * Returns the text before the caret from the current selection block.
-	 *
-	 * @returns {String|undefined} The text from the block or undefined if the selection is not collapsed.
-	 * @private
-	 */
-	_getText() {
-		const model = this.model;
-		const document = model.document;
-		const selection = document.selection;
-
-		const rangeBeforeSelection = model.createRange( model.createPositionAt( selection.focus.parent, 0 ), selection.focus );
-
-		return _getText( rangeBeforeSelection );
-	}
-}
-
-// Returns the whole text from a given range by adding all data from the text nodes together.
-//
-// @param {module:engine/model/range~Range} range
-// @returns {String}
-function _getText( range ) {
-	return Array.from( range.getItems() ).reduce( ( rangeText, node ) => {
-		if ( node.is( 'softBreak' ) ) {
-			// Trim text to a softBreak.
-			return '';
-		}
-
-		return rangeText + node.data;
-	}, '' );
 }
 
 mix( TextWatcher, EmitterMixin );

src/utils/getlasttextline.js

@@ -0,0 +1,60 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+/**
+ * @module typing/utils/getlasttextline
+ */
+
+/**
+ * Returns the last text line from the given range.
+ *
+ * "The last text line" is understood as text (from one or more text nodes) which is limited either by a parent block
+ * or by inline elements (e.g. `<softBreak>`).
+ *
+ *		const rangeToCheck = model.createRange(
+ *			model.createPositionAt( paragraph, 0 ),
+ *			model.createPositionAt( paragraph, 'end' )
+ *		);
+ *
+ *		const { text, range } = getLastTextLine( rangeToCheck, model );
+ *
+ * For model below, the returned `text` will be "Foo bar baz" and `range` will be set on whole `<paragraph>` content:
+ *
+ *		<paragraph>Foo bar baz<paragraph>
+ *
+ * However, in below case, `text` will be set to "baz" and `range` will be set only on "baz".
+ *
+ *		<paragraph>Foo<softBreak></softBreak>bar<softBreak></softBreak>baz<paragraph>
+ *
+ * @protected
+ * @param {module:engine/model/range~Range} range
+ * @param {module:engine/model/model~Model} model
+ * @returns {module:typing/utils/getlasttextline~LastTextLineData}
+ */
+export default function getLastTextLine( range, model ) {
+	let start = range.start;
+
+	const text = Array.from( range.getItems() ).reduce( ( rangeText, node ) => {
+		// Trim text to a last occurrence of an inline element and update range start.
+		if ( !( node.is( 'text' ) || node.is( 'textProxy' ) ) ) {
+			start = model.createPositionAfter( node );
+
+			return '';
+		}
+
+		return rangeText + node.data;
+	}, '' );
+
+	return { text, range: model.createRange( start, range.end ) };
+}
+
+/**
+ * The value returned by {@link module:typing/utils/getlasttextline~getLastTextLine}.
+ *
+ * @typedef {Object} module:typing/utils/getlasttextline~LastTextLineData
+ *
+ * @property {String} text The text from the text nodes in the last text line.
+ * @property {module:engine/model/range~Range} range The range set on the text nodes in the last text line.
+ */

tests/texttransformation.js

@@ -103,6 +103,7 @@ describe( 'Text transformation feature', () => {
 					testTransformation( '\' foo "bar"', '\' foo “bar”' );
 					testTransformation( 'Foo "Bar bar\'s it\'s a baz"', 'Foo “Bar bar\'s it\'s a baz”' );
 					testTransformation( ' ""', ' “”' );
+					testTransformation( ' "Bar baz"', ' “Bar baz”', '"A foo<softBreak></softBreak>' );
 				} );
 
 				describe( 'secondary', () => {
@@ -145,6 +146,15 @@ describe( 'Text transformation feature', () => {
 				.to.equal( '<paragraph>F<$text bold="true">oo “B</$text>ar”</paragraph>' );
 		} );
 
+		it( 'should work with soft breaks in parent', () => {
+			setData( model, '<paragraph>"Foo <softBreak></softBreak>"Bar[]</paragraph>' );
+
+			simulateTyping( '"' );
+
+			expect( getData( model, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>"Foo <softBreak></softBreak>“Bar”</paragraph>' );
+		} );
+
 		function testTransformation( transformFrom, transformTo, textInParagraph = 'A foo' ) {
 			it( `should transform "${ transformFrom }" to "${ transformTo }"`, () => {
 				setData( model, `<paragraph>${ textInParagraph }[]</paragraph>` );
@@ -352,6 +362,15 @@ describe( 'Text transformation feature', () => {
 
 				model = editor.model;
 				doc = model.document;
+
+				model.schema.register( 'softBreak', {
+					allowWhere: '$text',
+					isInline: true
+				} );
+				editor.conversion.elementToElement( {
+					model: 'softBreak',
+					view: 'br'
+				} );
 			} );
 	}
 

tests/utils/getlasttextline.js

@@ -0,0 +1,88 @@
+/**
+ * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
+ */
+
+import Model from '@ckeditor/ckeditor5-engine/src/model/model';
+import { setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import getText from '../../src/utils/getlasttextline';
+
+describe( 'utils', () => {
+	let model, doc, root;
+
+	beforeEach( () => {
+		model = new Model();
+		doc = model.document;
+		root = doc.createRoot();
+
+		model.schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+		model.schema.register( 'softBreak', { allowWhere: '$text', isInline: true } );
+		model.schema.extend( '$text', { allowAttributes: [ 'bold', 'italic' ] } );
+	} );
+
+	describe( 'getText()', () => {
+		it( 'should return all text from passed range', () => {
+			setModelData( model, '<paragraph>foobar[]baz</paragraph>' );
+
+			testOutput(
+				model.createRangeIn( root.getChild( 0 ) ),
+				'foobarbaz',
+				[ 0, 0 ],
+				[ 0, 9 ] );
+		} );
+
+		it( 'should limit the output text to the given range', () => {
+			setModelData( model, '<paragraph>foobar[]baz</paragraph>' );
+
+			const testRange = model.createRange(
+				model.createPositionAt( root.getChild( 0 ), 1 ),
+				model.document.selection.focus
+			);
+
+			testOutput(
+				testRange,
+				'oobar',
+				[ 0, 1 ],
+				[ 0, 6 ] );
+		} );
+
+		it( 'should limit the output to the last inline element text constrain in given range', () => {
+			setModelData( model, '<paragraph>foo<softBreak></softBreak>bar<softBreak></softBreak>baz[]</paragraph>' );
+
+			const testRange = model.createRange(
+				model.createPositionAt( root.getChild( 0 ), 0 ),
+				model.document.selection.focus
+			);
+
+			testOutput(
+				testRange,
+				'baz',
+				[ 0, 8 ],
+				[ 0, 11 ] );
+		} );
+
+		it( 'should return text from text nodes with attributes', () => {
+			setModelData( model,
+				'<paragraph>' +
+				'<$text bold="true">foo</$text>' +
+				'<$text bold="true" italic="true">bar</$text>' +
+				'<$text italic="true">baz</$text>[]' +
+				'</paragraph>'
+			);
+
+			testOutput(
+				model.createRangeIn( root.getChild( 0 ) ),
+				'foobarbaz',
+				[ 0, 0 ],
+				[ 0, 9 ] );
+		} );
+	} );
+
+	function testOutput( range1, expectedText, startPath, endPath ) {
+		const { text, range } = getText( range1, model );
+
+		expect( text ).to.equal( expectedText );
+		expect( range.start.path ).to.deep.equal( startPath );
+		expect( range.end.path ).to.deep.equal( endPath );
+	}
+} );