Browse Source

Refactored view->model conversion to use target position.

Oskar Wróbel 7 years ago
parent
commit
b490c60cfe

+ 59 - 38
packages/ckeditor5-engine/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
@@ -291,31 +292,62 @@ class ViewConverterBuilder {
 						continue;
 					}
 
-					if ( !conversionApi.schema.checkChild( data.context, modelElement ) ) {
+					// When element was already consumed then skip it.
+					if ( !consumable.test( data.input, 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.position, conversionApi );
+
+					// 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 );
-
-					// Convert children of converted view element and append them to `modelElement`.
-					const modelChildren = conversionApi.convertChildren( data.input, consumable, data );
-
-					for ( const child of Array.from( modelChildren ) ) {
-						writer.append( child, modelElement );
+					// Insert element on allowed position.
+					conversionApi.writer.insert( modelElement, splitResult.position );
+
+					// Convert children and insert to element.
+					const childrenOutput = conversionApi.convertChildren( data.input, consumable, Position.createAt( modelElement ) );
+
+					// Consume appropriate value from consumable values list.
+					consumable.consume( data.input, from.consume || match.match );
+
+					// Prepare conversion output range, we know that range will start before inserted element.
+					const output = new Range( Position.createBefore( modelElement ) );
+
+					// Now we need to check where the output should end.
+
+					// If we had to split parent to insert our element the we want to continue conversion
+					// inside split parent.
+					//
+					// before: <allowed><notAllowed>[]</notAllowed></allowed>
+					// after:  <allowed><notAllowed></notAllowed>[<converted></converted><notAllowed>]</notAllowed></allowed>
+					if ( splitResult.endElement ) {
+						output.end = Position.createAt( splitResult.endElement );
+
+					// When element is converted on target position (without splitting) we need to move range
+					// after this element 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.
+					//
+					// after: <allowed>[]</allowed>
+					// before: <allowed>[<converted><child></child></converted><child></child><converted>]</converted></allowed>
+					} else if ( childrenOutput.end.parent !== childrenOutput.end.root ) {
+						output.end = Position.createAfter( childrenOutput.end.parent );
+
+					// Finally when we are sure that element and its parent was not split we need to put selection
+					// after element.
+					//
+					// after: <allowed>[]</allowed>
+					// before: <allowed>[<converted></converted>]</allowed>
+					} else {
+						output.end = Position.createAfter( modelElement );
 					}
 
-					// Remove created `modelElement` from the parents stack.
-					data.context.pop();
-
-					// Add `modelElement` as a result.
-					data.output = modelElement;
+					data.output = output;
 
 					// Prevent multiple conversion if there are other correct matches.
 					break;
@@ -365,7 +397,7 @@ class ViewConverterBuilder {
 					// 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 );
+						data.output = conversionApi.convertChildren( data.input, consumable, data.position );
 					}
 
 					// Use attribute creator function, if provided.
@@ -384,8 +416,12 @@ class ViewConverterBuilder {
 						};
 					}
 
-					// 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.output.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;
@@ -467,7 +503,8 @@ class ViewConverterBuilder {
 						continue;
 					}
 
-					data.output = modelElement;
+					writer.insert( modelElement, data.position );
+					data.output = Range.createOn( modelElement );
 
 					// Prevent multiple conversion if there are other correct matches.
 					break;
@@ -504,22 +541,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

+ 9 - 3
packages/ckeditor5-engine/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}.
@@ -29,7 +31,7 @@ export function convertToModelFragment() {
 	return ( evt, data, consumable, 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 );
+			data.output = conversionApi.convertChildren( data.input, consumable, data.position );
 		}
 	};
 }
@@ -41,9 +43,13 @@ export function convertToModelFragment() {
  */
 export function convertText() {
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( conversionApi.schema.checkChild( data.context, '$text' ) ) {
+		if ( conversionApi.schema.checkChild( data.position, '$text' ) ) {
 			if ( consumable.consume( data.input ) ) {
-				data.output = conversionApi.writer.createText( data.input.data );
+				const text = conversionApi.writer.createText( data.input.data );
+
+				conversionApi.writer.insert( text, data.position );
+
+				data.output = Range.createFromPositionAndShift( data.position, text.offsetSize );
 			}
 		}
 	};

+ 134 - 44
packages/ckeditor5-engine/src/conversion/viewconversiondispatcher.js

@@ -10,9 +10,7 @@
 import ViewConsumable from './viewconsumable';
 import ModelRange from '../model/range';
 import ModelPosition from '../model/position';
-import ModelTreeWalker from '../model/treewalker';
-import ModelNode from '../model/node';
-import ModelDocumentFragment from '../model/documentfragment';
+import { SchemaContext } from '../model/schema';
 
 import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -127,6 +125,7 @@ export default class ViewConversionDispatcher {
 		// set on `conversionApi`. This way only a part of `ViewConversionDispatcher` API is exposed.
 		this.conversionApi.convertItem = this._convertItem.bind( this );
 		this.conversionApi.convertChildren = this._convertChildren.bind( this );
+		this.conversionApi.splitToAllowedParent = this._splitToAllowedParent.bind( this );
 	}
 
 	/**
@@ -137,43 +136,62 @@ export default class ViewConversionDispatcher {
 	 * @fires documentFragment
 	 * @param {module:engine/view/documentfragment~DocumentFragment|module:engine/view/element~Element} viewItem
 	 * Part of the view to be converted.
-	 * @param {Object} additionalData Additional data to be passed in `data` argument when firing `ViewConversionDispatcher`
-	 * events. See also {@link ~ViewConversionDispatcher#event:element element event}.
+	 * @param {module:engine/model/schema~SchemaContextDefinition} [context=['$root']] Elements will be converted according to this context.
 	 * @returns {module:engine/model/documentfragment~DocumentFragment} Model data that is a result of the conversion process
 	 * wrapped in `DocumentFragment`. Converted marker elements will be set as that document fragment's
 	 * {@link module:engine/model/documentfragment~DocumentFragment#markers static markers map}.
 	 */
-	convert( viewItem, additionalData ) {
+	convert( viewItem, context = [ '$root' ] ) {
 		return this._model.change( writer => {
+			this.fire( 'viewCleanup', viewItem );
+
+			const consumable = ViewConsumable.createFrom( viewItem );
+
+			// Create model tree according to given context. Elements will be converted to the top element of this tree.
+			// Thanks to this schema will be able check items precisely.
+			// Top element of context tree is marked by a `isContextTree` attribute.
+			const position = contextToPosition( context, writer );
+
 			// Store writer in current conversion as a conversion API.
 			this.conversionApi.writer = writer;
 
-			this.fire( 'viewCleanup', viewItem );
+			// Create set for split elements. We need to remember this elements, because at the end of conversion
+			// we want to remove all empty elements that was created as a result of the split.
+			this.conversionApi.splitElements = new Set();
 
-			const consumable = ViewConsumable.createFrom( viewItem );
-			let conversionResult = this._convertItem( viewItem, consumable, additionalData );
+			// Additional date available between conversions.
+			// Needed when one converter needs to leave some data for oder converters.
+			this.conversionApi.data = {};
 
-			// Remove writer from conversion API after conversion.
-			this.conversionApi.writer = null;
+			// Do the conversion.
+			const range = this._convertItem( viewItem, consumable, position );
 
-			// We can get a null here if conversion failed (see _convertItem())
-			// or simply if an item could not be converted (e.g. due to the schema).
-			if ( !conversionResult ) {
-				return writer.createDocumentFragment();
-			}
+			// Conversion result is always a document fragment so let's create this fragment.
+			const documentFragment = writer.createDocumentFragment();
+
+			// When there is a conversion result.
+			if ( range ) {
+				// Remove each empty element that was created as a result of split.
+				for ( const item of this.conversionApi.splitElements ) {
+					removeEmptySplitResult( item, this.conversionApi );
+				}
 
-			// When conversion result is not a document fragment we need to wrap it in document fragment.
-			if ( !conversionResult.is( 'documentFragment' ) ) {
-				const docFrag = writer.createDocumentFragment();
+				// Move all items that was converted to context tree to document fragment.
+				for ( const item of Array.from( position.parent.getChildren() ) ) {
+					writer.append( item, documentFragment );
+				}
 
-				writer.append( conversionResult, docFrag );
-				conversionResult = docFrag;
+				// Extract temporary markers elements from model and set as static markers collection.
+				documentFragment.markers = extractMarkersFromModelFragment( documentFragment, writer );
 			}
 
-			// Extract temporary markers elements from model and set as static markers collection.
-			conversionResult.markers = extractMarkersFromModelFragment( conversionResult, writer );
+			// Clear conversion API.
+			this.conversionApi.writer = null;
+			this.conversionApi.splitElements = null;
+			this.conversionApi.data = null;
 
-			return conversionResult;
+			// Return fragment as conversion result.
+			return documentFragment;
 		} );
 	}
 
@@ -181,11 +199,8 @@ export default class ViewConversionDispatcher {
 	 * @private
 	 * @see module:engine/conversion/viewconversiondispatcher~ViewConversionApi#convertItem
 	 */
-	_convertItem( input, consumable, additionalData = {} ) {
-		const data = Object.assign( {}, additionalData, {
-			input,
-			output: null
-		} );
+	_convertItem( input, consumable, position ) {
+		const data = Object.assign( { input, position, output: null } );
 
 		if ( input.is( 'element' ) ) {
 			this.fire( 'element:' + input.name, data, consumable, this.conversionApi );
@@ -196,7 +211,7 @@ export default class ViewConversionDispatcher {
 		}
 
 		// Handle incorrect `data.output`.
-		if ( data.output && !( data.output instanceof ModelNode || data.output instanceof ModelDocumentFragment ) ) {
+		if ( data.output && !( data.output instanceof ModelRange ) ) {
 			/**
 			 * Incorrect conversion result was dropped.
 			 *
@@ -217,19 +232,52 @@ export default class ViewConversionDispatcher {
 	 * @private
 	 * @see module:engine/conversion/viewconversiondispatcher~ViewConversionApi#convertChildren
 	 */
-	_convertChildren( input, consumable, additionalData ) {
-		const writer = this.conversionApi.writer;
-		const documentFragment = writer.createDocumentFragment();
+	_convertChildren( input, consumable, position ) {
+		const output = new ModelRange( position );
 
 		for ( const viewChild of Array.from( input.getChildren() ) ) {
-			const modelChild = this._convertItem( viewChild, consumable, additionalData );
+			const range = this._convertItem( viewChild, consumable, output.end );
 
-			if ( modelChild instanceof ModelNode || modelChild instanceof ModelDocumentFragment ) {
-				writer.append( modelChild, documentFragment );
+			if ( range instanceof ModelRange ) {
+				output.end = range.end;
 			}
 		}
 
-		return documentFragment;
+		return output;
+	}
+
+	/**
+	 * TODO
+	 *
+	 * @private
+	 */
+	_splitToAllowedParent( element, position, conversionApi ) {
+		function checkLimit( node ) {
+			return node.hasAttribute( 'isContextTree' );
+		}
+
+		const allowedParent = conversionApi.schema.findAllowedParent( element, position, checkLimit );
+
+		if ( !allowedParent ) {
+			return;
+		}
+
+		if ( allowedParent === position.parent ) {
+			return { position };
+		}
+
+		const data = conversionApi.writer.split( position, allowedParent );
+
+		for ( const pos of data.range.getPositions() ) {
+			if ( !pos.isEqual( data.position ) ) {
+				conversionApi.splitElements.add( pos.parent );
+			}
+		}
+
+		return {
+			position: data.position,
+			endElement: data.range.end.parent
+		};
 	}
 
 	/**
@@ -292,16 +340,13 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
 	const markers = new Map();
 
 	// Create ModelTreeWalker.
-	const walker = new ModelTreeWalker( {
-		startPosition: ModelPosition.createAt( modelItem, 0 ),
-		ignoreElementEnd: true
-	} );
+	const range = ModelRange.createIn( modelItem ).getItems();
 
 	// Walk through DocumentFragment and collect marker elements.
-	for ( const value of walker ) {
+	for ( const item of range ) {
 		// Check if current element is a marker.
-		if ( value.item.name == '$marker' ) {
-			markerElements.add( value.item );
+		if ( item.name == '$marker' ) {
+			markerElements.add( item );
 		}
 	}
 
@@ -325,6 +370,43 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
 	return markers;
 }
 
+function contextToPosition( contextDefinition, writer ) {
+	let position;
+
+	for ( const item of new SchemaContext( contextDefinition ) ) {
+		const attributes = {};
+
+		for ( const key of item.getAttributeKeys() ) {
+			attributes[ key ] = item.getAttribute( key );
+		}
+
+		const current = writer.createElement( item.name, attributes );
+
+		if ( position ) {
+			writer.append( current, position );
+		}
+
+		position = ModelPosition.createAt( current );
+	}
+
+	position.parent.setAttribute( 'isContextTree', true );
+
+	return position;
+}
+
+function removeEmptySplitResult( item, conversionApi ) {
+	if ( item.isEmpty ) {
+		const parent = item.parent;
+
+		conversionApi.writer.remove( item );
+		conversionApi.splitElements.delete( item );
+
+		if ( conversionApi.splitElements.has( parent ) ) {
+			removeEmptySplitResult( parent, conversionApi );
+		}
+	}
+}
+
 /**
  * Conversion interface that is registered for given {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher}
  * and is passed as one of parameters when {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher dispatcher}
@@ -351,6 +433,7 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
  * @param {module:engine/view/documentfragment~DocumentFragment|module:engine/view/element~Element|module:engine/view/text~Text}
  * input Item to convert.
  * @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable Values to consume.
+ * @param {module:engine/model/position~Position} position Position of conversion.
  * @param {Object} [additionalData] Additional data to be passed in `data` argument when firing `ViewConversionDispatcher`
  * events. See also {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#event:element element event}.
  * @returns {module:engine/model/node~Node|module:engine/model/documentfragment~DocumentFragment|null} The result of item conversion,
@@ -367,8 +450,15 @@ function extractMarkersFromModelFragment( modelItem, writer ) {
  * @param {module:engine/view/documentfragment~DocumentFragment|module:engine/view/element~Element}
  * input Item which children will be converted.
  * @param {module:engine/conversion/viewconsumable~ViewConsumable} consumable Values to consume.
+ * @param {module:engine/model/position~Position} position Position of conversion.
  * @param {Object} [additionalData] Additional data to be passed in `data` argument when firing `ViewConversionDispatcher`
  * events. See also {@link module:engine/conversion/viewconversiondispatcher~ViewConversionDispatcher#event:element element event}.
  * @returns {module:engine/model/documentfragment~DocumentFragment} Model document fragment containing results of conversion
  * of all children of given item.
  */
+
+/**
+ * Custom data stored by converter for conversion process.
+ *
+ * @param {Object} #data
+ */