Merge pull request #1256 from ckeditor/t/1213

Feature: Convert view to model using position. Closes #1213. Closes #1250.

BREAKING CHANGE: `DataController#parse`, `DataController#toModel`, `ViewConversionDispatcher#convert` gets `SchemaContextDefinition` as a contex instead of `String`.
BREAKING CHANGE: `ViewConversionApi#splitToAllowedParent` has been introduced.
BREAKING CHANGE: `ViewConversionApi#storage` has been introduced.
BREAKING CHANGE: `ViewConsumable` has been merged to `ViewConversionApi`.
BREAKING CHANGE: Format od data object passed across conversion callback has been changed.
Feature: `Schema#findAllowedParent` has been introduced.
Feature: `SchemaContext#concat` has been introduced.

Author: Piotr Jasiun

src/controller/datacontroller.js

@@ -209,11 +209,11 @@ export default class DataController {
 	 *
 	 * @see #set
 	 * @param {String} data Data to parse.
-	 * @param {String} [context='$root'] Base context in which the view will be converted to the model. See:
-	 * {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
+	 * @param {module:engine/model/schema~SchemaContextDefinition} [context=['$root']] Base context in which the view will
+	 * be converted to the model. See: {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
 	 * @returns {module:engine/model/documentfragment~DocumentFragment} Parsed data.
 	 */
-	parse( data, context = '$root' ) {
+	parse( data, context = [ '$root' ] ) {
 		// data -> view
 		const viewDocumentFragment = this.processor.toView( data );
 
@@ -231,12 +231,12 @@ export default class DataController {
 	 *
 	 * @param {module:engine/view/element~Element|module:engine/view/documentfragment~DocumentFragment} viewElementOrFragment
 	 * Element or document fragment whose content will be converted.
-	 * @param {String} [context='$root'] Base context in which the view will be converted to the model. See:
-	 * {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
+	 * @param {module:engine/model/schema~SchemaContextDefinition} [context=['$root']] Base context in which the view will
+	 * be converted to the model. See: {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#convert}.
 	 * @returns {module:engine/model/documentfragment~DocumentFragment} Output document fragment.
 	 */
-	toModel( viewElementOrFragment, context = '$root' ) {
-		return this.viewToModel.convert( viewElementOrFragment, { context: [ context ] } );
+	toModel( viewElementOrFragment, context = [ '$root' ] ) {
+		return this.viewToModel.convert( viewElementOrFragment, context );
 	}
 
 	/**

src/conversion/buildviewconverter.js

@@ -9,7 +9,8 @@
 
 import Matcher from '../view/matcher';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import isIterable from '@ckeditor/ckeditor5-utils/src/isiterable';
+import Position from '../model/position';
+import Range from '../model/range';
 
 /**
  * Provides chainable, high-level API to easily build basic view-to-model converters that are appended to given
@@ -269,12 +270,12 @@ class ViewConverterBuilder {
 	 */
 	toElement( element ) {
 		function eventCallbackGen( from ) {
-			return ( evt, data, consumable, conversionApi ) => {
+			return ( evt, data, conversionApi ) => {
 				const writer = conversionApi.writer;
 
 				// There is one callback for all patterns in the matcher.
 				// This will be usually just one pattern but we support matchers with many patterns too.
-				const matchAll = from.matcher.matchAll( data.input );
+				const matchAll = from.matcher.matchAll( data.viewItem );
 
 				// If there is no match, this callback should not do anything.
 				if ( !matchAll ) {
@@ -284,38 +285,60 @@ class ViewConverterBuilder {
 				// Now, for every match between matcher and actual element, we will try to consume the match.
 				for ( const match of matchAll ) {
 					// Create model element basing on creator function or element name.
-					const modelElement = element instanceof Function ? element( data.input, writer ) : writer.createElement( element );
+					const modelElement = element instanceof Function ? element( data.viewItem, writer ) : writer.createElement( element );
 
 					// Do not convert if element building function returned falsy value.
 					if ( !modelElement ) {
 						continue;
 					}
 
-					if ( !conversionApi.schema.checkChild( data.context, modelElement ) ) {
+					// When element was already consumed then skip it.
+					if ( !conversionApi.consumable.test( data.viewItem, from.consume || match.match ) ) {
 						continue;
 					}
 
-					// Try to consume appropriate values from consumable values list.
-					if ( !consumable.consume( data.input, from.consume || match.match ) ) {
+					// Find allowed parent for element that we are going to insert.
+					// If current parent does not allow to insert element but one of the ancestors does
+					// then split nodes to allowed parent.
+					const splitResult = conversionApi.splitToAllowedParent( modelElement, data.cursorPosition );
+
+					// When there is no split result it means that we can't insert element to model tree, so let's skip it.
+					if ( !splitResult ) {
 						continue;
 					}
 
-					// If everything is fine, we are ready to start the conversion.
-					// Add newly created `modelElement` to the parents stack.
-					data.context.push( modelElement );
+					// Insert element on allowed position.
+					conversionApi.writer.insert( modelElement, splitResult.position );
 
-					// Convert children of converted view element and append them to `modelElement`.
-					const modelChildren = conversionApi.convertChildren( data.input, consumable, data );
+					// Convert children and insert to element.
+					const childrenResult = conversionApi.convertChildren( data.viewItem, Position.createAt( modelElement ) );
 
-					for ( const child of Array.from( modelChildren ) ) {
-						writer.append( child, modelElement );
-					}
+					// Consume appropriate value from consumable values list.
+					conversionApi.consumable.consume( data.viewItem, from.consume || match.match );
+
+					// Set conversion result range.
+					data.modelRange = new Range(
+						// Range should start before inserted element
+						Position.createBefore( modelElement ),
+						// Should end after but we need to take into consideration that children could split our
+						// element, so we need to move range after parent of the last converted child.
+						// before: <allowed>[]</allowed>
+						// after: <allowed>[<converted><child></child></converted><child></child><converted>]</converted></allowed>
+						Position.createAfter( childrenResult.cursorPosition.parent )
+					);
 
-					// Remove created `modelElement` from the parents stack.
-					data.context.pop();
+					// Now we need to check where the cursorPosition should be.
+					// If we had to split parent to insert our element then we want to continue conversion inside split parent.
+					//
+					// before: <allowed><notAllowed>[]</notAllowed></allowed>
+					// after:  <allowed><notAllowed></notAllowed><converted></converted><notAllowed>[]</notAllowed></allowed>
+					if ( splitResult.cursorParent ) {
+						data.cursorPosition = Position.createAt( splitResult.cursorParent );
 
-					// Add `modelElement` as a result.
-					data.output = modelElement;
+					// Otherwise just continue after inserted element.
+					} else {
+						data.cursorPosition = data.modelRange.end;
+					}
 
 					// Prevent multiple conversion if there are other correct matches.
 					break;
@@ -345,10 +368,10 @@ class ViewConverterBuilder {
 	 */
 	toAttribute( keyOrCreator, value ) {
 		function eventCallbackGen( from ) {
-			return ( evt, data, consumable, conversionApi ) => {
+			return ( evt, data, conversionApi ) => {
 				// There is one callback for all patterns in the matcher.
 				// This will be usually just one pattern but we support matchers with many patterns too.
-				const matchAll = from.matcher.matchAll( data.input );
+				const matchAll = from.matcher.matchAll( data.viewItem );
 
 				// If there is no match, this callback should not do anything.
 				if ( !matchAll ) {
@@ -358,34 +381,39 @@ class ViewConverterBuilder {
 				// Now, for every match between matcher and actual element, we will try to consume the match.
 				for ( const match of matchAll ) {
 					// Try to consume appropriate values from consumable values list.
-					if ( !consumable.consume( data.input, from.consume || match.match ) ) {
+					if ( !conversionApi.consumable.consume( data.viewItem, from.consume || match.match ) ) {
 						continue;
 					}
 
-					// Since we are converting to attribute we need an output on which we will set the attribute.
-					// If the output is not created yet, we will create it.
-					if ( !data.output ) {
-						data.output = conversionApi.convertChildren( data.input, consumable, data );
+					// Since we are converting to attribute we need an range on which we will set the attribute.
+					// If the range is not created yet, we will create it.
+					if ( !data.modelRange ) {
+						// Convert children and set conversion result as a current data.
+						data = Object.assign( data, conversionApi.convertChildren( data.viewItem, data.cursorPosition ) );
 					}
 
 					// Use attribute creator function, if provided.
 					let attribute;
 
 					if ( keyOrCreator instanceof Function ) {
-						attribute = keyOrCreator( data.input );
+						attribute = keyOrCreator( data.viewItem );
 
 						if ( !attribute ) {
 							return;
 						}
 					} else {
 						attribute = {
 							key: keyOrCreator,
-							value: value ? value : data.input.getAttribute( from.attributeKey )
+							value: value ? value : data.viewItem.getAttribute( from.attributeKey )
 						};
 					}
 
-					// Set attribute on current `output`. `Schema` is checked inside this helper function.
-					setAttributeOn( data.output, attribute, data, conversionApi );
+					// Set attribute on each item in range according to Schema.
+					for ( const node of Array.from( data.modelRange.getItems() ) ) {
+						if ( conversionApi.schema.checkAttribute( node, attribute.key ) ) {
+							conversionApi.writer.setAttribute( attribute.key, attribute.value, node );
+						}
+					}
 
 					// Prevent multiple conversion if there are other correct matches.
 					break;
@@ -431,12 +459,12 @@ class ViewConverterBuilder {
 	 */
 	toMarker( creator ) {
 		function eventCallbackGen( from ) {
-			return ( evt, data, consumable, conversionApi ) => {
+			return ( evt, data, conversionApi ) => {
 				const writer = conversionApi.writer;
 
 				// There is one callback for all patterns in the matcher.
 				// This will be usually just one pattern but we support matchers with many patterns too.
-				const matchAll = from.matcher.matchAll( data.input );
+				const matchAll = from.matcher.matchAll( data.viewItem );
 
 				// If there is no match, this callback should not do anything.
 				if ( !matchAll ) {
@@ -447,10 +475,10 @@ class ViewConverterBuilder {
 
 				// When creator is provided then create model element basing on creator function.
 				if ( creator instanceof Function ) {
-					modelElement = creator( data.input );
+					modelElement = creator( data.viewItem );
 				// When there is no creator then create model element basing on data from view element.
 				} else {
-					modelElement = writer.createElement( '$marker', { 'data-name': data.input.getAttribute( 'data-name' ) } );
+					modelElement = writer.createElement( '$marker', { 'data-name': data.viewItem.getAttribute( 'data-name' ) } );
 				}
 
 				// Check if model element is correct (has proper name and property).
@@ -463,11 +491,19 @@ class ViewConverterBuilder {
 				// Now, for every match between matcher and actual element, we will try to consume the match.
 				for ( const match of matchAll ) {
 					// Try to consume appropriate values from consumable values list.
-					if ( !consumable.consume( data.input, from.consume || match.match ) ) {
+					if ( !conversionApi.consumable.consume( data.viewItem, from.consume || match.match ) ) {
 						continue;
 					}
 
-					data.output = modelElement;
+					// Tmp fix because multiple matchers are not properly matched and consumed.
+					// See https://github.com/ckeditor/ckeditor5-engine/issues/1257.
+					if ( data.modelRange ) {
+						continue;
+					}
+
+					writer.insert( modelElement, data.cursorPosition );
+					data.modelRange = Range.createOn( modelElement );
+					data.cursorPosition = data.modelRange.end;
 
 					// Prevent multiple conversion if there are other correct matches.
 					break;
@@ -504,22 +540,6 @@ class ViewConverterBuilder {
 	}
 }
 
-// Helper function that sets given attributes on given `module:engine/model/node~Node` or
-// `module:engine/model/documentfragment~DocumentFragment`.
-function setAttributeOn( toChange, attribute, data, conversionApi ) {
-	if ( isIterable( toChange ) ) {
-		for ( const node of toChange ) {
-			setAttributeOn( node, attribute, data, conversionApi );
-		}
-
-		return;
-	}
-
-	if ( conversionApi.schema.checkAttribute( toChange, attribute.key ) ) {
-		conversionApi.writer.setAttribute( attribute.key, attribute.value, toChange );
-	}
-}
-
 /**
  * Entry point for view-to-model converters builder. This chainable API makes it easy to create basic, most common
  * view-to-model converters and attach them to provided dispatchers. The method returns an instance of

src/conversion/view-to-model-converters.js

@@ -3,6 +3,8 @@
  * For licensing, see LICENSE.md.
  */
 
+import Range from '../model/range';
+
 /**
  * Contains {@link module:engine/view/view view} to {@link module:engine/model/model model} converters for
  * {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}.
@@ -26,10 +28,11 @@
  * {@link module:engine/model/documentfragment~DocumentFragment model fragment} with children of converted view item.
  */
 export function convertToModelFragment() {
-	return ( evt, data, consumable, conversionApi ) => {
+	return ( evt, data, conversionApi ) => {
 		// Second argument in `consumable.consume` is discarded for ViewDocumentFragment but is needed for ViewElement.
-		if ( !data.output && consumable.consume( data.input, { name: true } ) ) {
-			data.output = conversionApi.convertChildren( data.input, consumable, data );
+		if ( !data.modelRange && conversionApi.consumable.consume( data.viewItem, { name: true } ) ) {
+			data = Object.assign( data, conversionApi.convertChildren( data.viewItem, data.cursorPosition ) );
+			data.cursorPosition = data.modelRange.end;
 		}
 	};
 }
@@ -40,10 +43,15 @@ export function convertToModelFragment() {
  * @returns {Function} {@link module:engine/view/text~Text View text} converter.
  */
 export function convertText() {
-	return ( evt, data, consumable, conversionApi ) => {
-		if ( conversionApi.schema.checkChild( data.context, '$text' ) ) {
-			if ( consumable.consume( data.input ) ) {
-				data.output = conversionApi.writer.createText( data.input.data );
+	return ( evt, data, conversionApi ) => {
+		if ( conversionApi.schema.checkChild( data.cursorPosition, '$text' ) ) {
+			if ( conversionApi.consumable.consume( data.viewItem ) ) {
+				const text = conversionApi.writer.createText( data.viewItem.data );
+
+				conversionApi.writer.insert( text, data.cursorPosition );
+
+				data.modelRange = Range.createFromPositionAndShift( data.cursorPosition, text.offsetSize );
+				data.cursorPosition = data.modelRange.end;
 			}
 		}
 	};