Procházet zdrojové kódy

Merge pull request #616 from ckeditor/t/583b

T/583b Changes to the model required by List feature.
Piotrek Koszuliński před 9 roky
rodič
revize
7458d7fa53

+ 14 - 11
packages/ckeditor5-engine/src/conversion/mapper.js

@@ -176,7 +176,8 @@ export default class Mapper {
 	toModelPosition( viewPosition ) {
 		const data = {
 			viewPosition: viewPosition,
-			modelPosition: null
+			modelPosition: null,
+			mapper: this
 		};
 
 		this.fire( 'viewToModelPosition', data );
@@ -194,7 +195,8 @@ export default class Mapper {
 	toViewPosition( modelPosition ) {
 		const data = {
 			viewPosition: null,
-			modelPosition: modelPosition
+			modelPosition: modelPosition,
+			mapper: this
 		};
 
 		this.fire( 'modelToViewPosition', data );
@@ -273,7 +275,7 @@ export default class Mapper {
 		let modelOffset = 0;
 
 		for ( let i = 0; i < viewOffset; i++ ) {
-			modelOffset += this._getModelLength( viewParent.getChild( i ) );
+			modelOffset += this.getModelLength( viewParent.getChild( i ) );
 		}
 
 		return modelOffset;
@@ -307,11 +309,10 @@ export default class Mapper {
 	 *		<b>foo</b>              -> 3 // Length of an element which is not mapped is a length of its children.
 	 *		<div><p>x</p><p>y</p>   -> 2 // Assuming that <div> is not mapped and <p> are mapped.
 	 *
-	 * @private
 	 * @param {engine.view.Element} viewNode View node.
 	 * @returns {Number} Length of the node in the tree model.
 	 */
-	_getModelLength( viewNode ) {
+	getModelLength( viewNode ) {
 		if ( this._viewToModelMapping.has( viewNode ) ) {
 			const callback = this._viewToModelLengthCallbacks.get( viewNode.name );
 
@@ -322,7 +323,7 @@ export default class Mapper {
 			let len = 0;
 
 			for ( let child of viewNode.getChildren() ) {
-				len += this._getModelLength( child );
+				len += this.getModelLength( child );
 			}
 
 			return len;
@@ -373,7 +374,7 @@ export default class Mapper {
 		// If it is smaller we add the length.
 		while ( modelOffset < expectedOffset ) {
 			viewNode = viewParent.getChild( viewOffset );
-			lastLength = this._getModelLength( viewNode );
+			lastLength = this.getModelLength( viewNode );
 			modelOffset += lastLength;
 			viewOffset++;
 		}
@@ -429,7 +430,7 @@ mix( Mapper, EmitterMixin );
  *
  * 		// Assume that "captionedImage" model element is converted to <img> and following <span> elements in view,
  * 		// and the model element is bound to <img> element. Force mapping model positions inside "captionedImage" to that <span> element.
- *		mapper.on( 'modelToViewPosition', ( evt, mapper, modelPosition, data ) => {
+ *		mapper.on( 'modelToViewPosition', ( evt, data ) => {
  *			const positionParent = modelPosition.parent;
  *
  *			if ( positionParent.name == 'captionedImage' ) {
@@ -446,9 +447,10 @@ mix( Mapper, EmitterMixin );
  * the condition that checks if special case scenario happened should be as simple as possible.
  *
  * @event engine.conversion.Mapper.modelToViewPosition
- * @param {engine.model.Position} modelPosition Model position to be mapped.
  * @param {Object} data Data pipeline object that can store and pass data between callbacks. The callback should add
  * `viewPosition` value to that object with calculated {@link engine.view.Position view position}.
+ * @param {engine.model.Position} data.modelPosition Model position to be mapped.
+ * @param {engine.conversion.Mapper} data.mapper Mapper instance that fired the event.
  */
 
 /**
@@ -456,7 +458,7 @@ mix( Mapper, EmitterMixin );
  *
  * 		// See example in `modelToViewPosition` event description.
  * 		// This custom mapping will map positions from <span> element next to <img> to the "captionedImage" element.
- *		mapper.on( 'viewToModelPosition', ( evt, mapper, viewPosition, data ) => {
+ *		mapper.on( 'viewToModelPosition', ( evt, data ) => {
  *			const positionParent = viewPosition.parent;
  *
  *			if ( positionParent.hasClass( 'image-caption' ) ) {
@@ -469,7 +471,8 @@ mix( Mapper, EmitterMixin );
  *		} );
  *
  * @event engine.conversion.Mapper.viewToModelPosition
- * @param {engine.view.Position} viewPosition View position to be mapped.
  * @param {Object} data Data pipeline object that can store and pass data between callbacks. The callback should add
  * `modelPosition` value to that object with calculated {@link engine.model.Position model position}.
+ * @param {engine.view.Position} data.viewPosition View position to be mapped.
+ * @param {engine.conversion.Mapper} data.mapper Mapper instance that fired the event.
  */

+ 105 - 59
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -7,8 +7,11 @@ import ModelRange from '../model/range.js';
 import ModelPosition from '../model/position.js';
 import ModelElement from '../model/element.js';
 
+import ViewRange from '../view/range.js';
 import ViewElement from '../view/element.js';
 import ViewText from '../view/text.js';
+import ViewTextProxy from '../view/textproxy.js';
+import ViewTreeWalker from '../view/treewalker.js';
 import viewWriter from '../view/writer.js';
 
 /**
@@ -49,7 +52,9 @@ import viewWriter from '../view/writer.js';
  */
 export function insertElement( elementCreator ) {
 	return ( evt, data, consumable, conversionApi ) => {
-		consumable.consume( data.item, 'insert' );
+		if ( !consumable.consume( data.item, 'insert' ) ) {
+			return;
+		}
 
 		const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
 		const viewElement = ( elementCreator instanceof ViewElement ) ?
@@ -58,8 +63,6 @@ export function insertElement( elementCreator ) {
 
 		conversionApi.mapper.bindElements( data.item, viewElement );
 		viewWriter.insert( viewPosition, viewElement );
-
-		evt.stop();
 	};
 }
 
@@ -77,14 +80,14 @@ export function insertElement( elementCreator ) {
  */
 export function insertText() {
 	return ( evt, data, consumable, conversionApi ) => {
-		consumable.consume( data.item, 'insert' );
+		if ( !consumable.consume( data.item, 'insert' ) ) {
+			return;
+		}
 
 		const viewPosition = conversionApi.mapper.toViewPosition( data.range.start );
 		const viewText = new ViewText( data.item.data );
 
 		viewWriter.insert( viewPosition, viewText );
-
-		evt.stop();
 	};
 }
 
@@ -125,12 +128,13 @@ export function setAttribute( attributeCreator ) {
 	attributeCreator = attributeCreator || ( ( value, key ) => ( { value, key } ) );
 
 	return ( evt, data, consumable, conversionApi ) => {
+		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+			return;
+		}
+
 		const { key, value } = attributeCreator( data.attributeNewValue, data.attributeKey, data, consumable, conversionApi );
 
-		consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
 		conversionApi.mapper.toViewElement( data.item ).setAttribute( key, value );
-
-		evt.stop();
 	};
 }
 
@@ -172,12 +176,13 @@ export function removeAttribute( attributeCreator ) {
 	attributeCreator = attributeCreator || ( ( value, key ) => ( { key } ) );
 
 	return ( evt, data, consumable, conversionApi ) => {
+		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+			return;
+		}
+
 		const { key } = attributeCreator( data.attributeOldValue, data.attributeKey, data, consumable, conversionApi );
 
-		consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
 		conversionApi.mapper.toViewElement( data.item ).removeAttribute( key );
-
-		evt.stop();
 	};
 }
 
@@ -218,7 +223,9 @@ export function wrap( elementCreator ) {
 			elementCreator( data.attributeNewValue, data, consumable, conversionApi );
 
 		if ( viewElement ) {
-			consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
+			if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+				return;
+			}
 
 			// If this is a change event (because old value is not empty) and the creator is a function (so
 			// it may create different view elements basing on attribute value) we have to create
@@ -229,8 +236,6 @@ export function wrap( elementCreator ) {
 			}
 
 			viewWriter.wrap( viewRange, viewElement );
-
-			evt.stop();
 		}
 	};
 }
@@ -261,7 +266,9 @@ export function wrap( elementCreator ) {
  */
 export function unwrap( elementCreator ) {
 	return ( evt, data, consumable, conversionApi ) => {
-		consumable.consume( data.item, eventNameToConsumableType( evt.name ) );
+		if ( !consumable.consume( data.item, eventNameToConsumableType( evt.name ) ) ) {
+			return;
+		}
 
 		const viewRange = conversionApi.mapper.toViewRange( data.range );
 		const viewNode = ( elementCreator instanceof ViewElement ) ?
@@ -269,8 +276,6 @@ export function unwrap( elementCreator ) {
 			elementCreator( data.attributeOldValue, data, consumable, conversionApi );
 
 		viewWriter.unwrap( viewRange, viewNode );
-
-		evt.stop();
 	};
 }
 
@@ -285,14 +290,24 @@ export function unwrap( elementCreator ) {
  */
 export function move() {
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( consumable.consume( data.item, 'move' ) ) {
-			const sourceModelRange = ModelRange.createFromPositionAndShift( data.sourcePosition, data.item.offsetSize );
-			const sourceViewRange = conversionApi.mapper.toViewRange( sourceModelRange );
+		if ( !consumable.consume( data.item, 'move' ) ) {
+			return;
+		}
+
+		let sourceViewRange;
 
-			const targetViewPosition = conversionApi.mapper.toViewPosition( data.targetPosition );
+		if ( data.item instanceof ModelElement ) {
+			const viewElement = conversionApi.mapper.toViewElement( data.item );
+			sourceViewRange = ViewRange.createOn( viewElement );
+		} else {
+			const viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
 
-			viewWriter.move( sourceViewRange, targetViewPosition );
+			sourceViewRange = findViewTextRange( viewPosition, data.item.offsetSize );
 		}
+
+		const targetViewPosition = conversionApi.mapper.toViewPosition( data.targetPosition );
+
+		viewWriter.move( sourceViewRange, targetViewPosition );
 	};
 }
 
@@ -307,17 +322,46 @@ export function move() {
  */
 export function remove() {
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( consumable.consume( data.item, 'remove' ) ) {
-			const sourceModelRange = ModelRange.createFromPositionAndShift( data.sourcePosition, data.item.offsetSize );
-			const sourceViewRange = conversionApi.mapper.toViewRange( sourceModelRange );
+		if ( !consumable.consume( data.item, 'remove' ) ) {
+			return;
+		}
+
+		let viewRange = null;
 
-			viewWriter.remove( sourceViewRange );
+		if ( data.item instanceof ModelElement ) {
+			const viewElement = conversionApi.mapper.toViewElement( data.item );
+			viewRange = ViewRange.createOn( viewElement );
+		} else {
+			const viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
 
-			conversionApi.mapper.unbindModelElement( data.item );
+			viewRange = findViewTextRange( viewPosition, data.item.offsetSize );
 		}
+
+		viewWriter.remove( viewRange );
+		conversionApi.mapper.unbindModelElement( data.item );
 	};
 }
 
+// Helper function for `remove` and `move` converters. It returns a ViewRange that starts at ViewPosition `start` and
+// includes `size` characters.
+// This method is used to find a ViewRange basing on ModelPosition and ModelTextProxy item size in `move` and `remove`
+// converters where it is impossible to just map positions because those positions already are invalid in model
+// (because they got moved or removed).
+function findViewTextRange( start, size ) {
+	const walker = new ViewTreeWalker( { startPosition: start, singleCharacters: true, ignoreElementEnd: true } );
+	let offset = 0;
+
+	for ( let value of walker ) {
+		if ( value.item instanceof ViewTextProxy ) {
+			offset++;
+
+			if ( offset == size ) {
+				return new ViewRange( start, walker.position );
+			}
+		}
+	}
+}
+
 /**
  * Function factory, creates default model-to-view converter for elements which name has changed.
  *
@@ -334,38 +378,40 @@ export function remove() {
  */
 export function rename() {
 	return ( evt, data, consumable, conversionApi ) => {
-		if ( consumable.test( data.element, 'rename' ) ) {
-			// Create fake model element that will represent "old version" of renamed element.
-			const fakeElement = new ModelElement( data.oldName, data.element.getAttributes() );
-			// Append the fake element to model document to enable making range on it.
-			data.element.parent.insertChildren( data.element.index, fakeElement );
-
-			// Check what was bound to renamed element.
-			const oldViewElement = conversionApi.mapper.toViewElement( data.element );
-			// Unbind renamed element.
-			conversionApi.mapper.unbindModelElement( data.element );
-			// Bind view element to the fake element.
-			conversionApi.mapper.bindElements( fakeElement, oldViewElement );
-
-			// The range that includes only the renamed element. Will be used to insert an empty element in the view.
-			const insertRange = ModelRange.createFromParentsAndOffsets( data.element.parent, data.element.startOffset, data.element, 0 );
-
-			// Move source position and range of moved nodes. Will be used to move nodes from original view element to renamed one.
-			const moveSourcePosition = ModelPosition.createAt( fakeElement, 0 );
-			const moveRange = ModelRange.createIn( data.element );
-
-			// Remove range containing the fake element. Will be used to remove original view element from the view.
-			const removeRange = ModelRange.createOn( fakeElement );
-
-			// Start the conversion. Use already defined converters by firing insertion, move and remove conversion
-			// on correct ranges / positions.
-			conversionApi.dispatcher.convertInsertion( insertRange );
-			conversionApi.dispatcher.convertMove( moveSourcePosition, moveRange );
-			conversionApi.dispatcher.convertRemove( removeRange.start, removeRange );
-
-			// Cleanup.
-			fakeElement.remove();
+		if ( !consumable.consume( data.element, 'rename' ) ) {
+			return;
 		}
+
+		// Create fake model element that will represent "old version" of renamed element.
+		const fakeElement = new ModelElement( data.oldName, data.element.getAttributes() );
+		// Append the fake element to model document to enable making range on it.
+		data.element.parent.insertChildren( data.element.index, fakeElement );
+
+		// Check what was bound to renamed element.
+		const oldViewElement = conversionApi.mapper.toViewElement( data.element );
+		// Unbind renamed element.
+		conversionApi.mapper.unbindModelElement( data.element );
+		// Bind view element to the fake element.
+		conversionApi.mapper.bindElements( fakeElement, oldViewElement );
+
+		// The range that includes only the renamed element. Will be used to insert an empty element in the view.
+		const insertRange = ModelRange.createFromParentsAndOffsets( data.element.parent, data.element.startOffset, data.element, 0 );
+
+		// Move source position and range of moved nodes. Will be used to move nodes from original view element to renamed one.
+		const moveSourcePosition = ModelPosition.createAt( fakeElement, 0 );
+		const moveRange = ModelRange.createIn( data.element );
+
+		// Remove range containing the fake element. Will be used to remove original view element from the view.
+		const removeRange = ModelRange.createOn( fakeElement );
+
+		// Start the conversion. Use already defined converters by firing insertion, move and remove conversion
+		// on correct ranges / positions.
+		conversionApi.dispatcher.convertInsertion( insertRange );
+		conversionApi.dispatcher.convertMove( moveSourcePosition, moveRange );
+		conversionApi.dispatcher.convertRemove( removeRange.start, removeRange );
+
+		// Cleanup.
+		fakeElement.remove();
 	};
 }
 

+ 12 - 10
packages/ckeditor5-engine/src/conversion/modelconversiondispatcher.js

@@ -5,7 +5,6 @@
 
 import Consumable from './modelconsumable.js';
 import Range from '../model/range.js';
-import Position from '../model/position.js';
 import TextProxy from '../model/textproxy.js';
 import EmitterMixin from '../../utils/emittermixin.js';
 import mix from '../../utils/mix.js';
@@ -199,18 +198,24 @@ export default class ModelConversionDispatcher {
 	 * @param {engine.model.Range} range Moved range (after move).
 	 */
 	convertMove( sourcePosition, range ) {
+		// Keep in mind that move dispatcher expects flat range.
 		const consumable = this._createConsumableForRange( range, 'move' );
 
-		// Fire a separate event for each top-most node and text fragment contained in the range.
-		const items = Array.from( range.getItems( { shallow: true } ) ).reverse();
+		const items = Array.from( range.getItems( { shallow: true } ) );
+		const rangeSize = range.end.offset - range.start.offset;
+		const inSameParent = sourcePosition.parent == range.start.parent;
+
+		let offset = 0;
 
 		for ( let item of items ) {
 			const data = {
-				sourcePosition: sourcePosition.getShiftedBy( item.startOffset - range.start.offset ),
-				targetPosition: Position.createAt( range.start ),
+				sourcePosition: sourcePosition,
+				targetPosition: inSameParent ? range.start.getShiftedBy( rangeSize ) : range.start.getShiftedBy( offset ),
 				item: item
 			};
 
+			offset += data.item.offsetSize;
+
 			this._testAndFire( 'move', data, consumable );
 		}
 	}
@@ -225,12 +230,9 @@ export default class ModelConversionDispatcher {
 	convertRemove( sourcePosition, range ) {
 		const consumable = this._createConsumableForRange( range, 'remove' );
 
-		// Fire a separate event for each top-most node and text fragment contained in the range.
-		const items = Array.from( range.getItems( { shallow: true } ) ).reverse();
-
-		for ( let item of items ) {
+		for ( let item of range.getItems( { shallow: true } ) ) {
 			const data = {
-				sourcePosition: sourcePosition.getShiftedBy( item.startOffset - range.start.offset ),
+				sourcePosition: sourcePosition,
 				item: item
 			};
 

+ 2 - 2
packages/ckeditor5-engine/src/conversion/view-to-model-converters.js

@@ -32,8 +32,8 @@ import { normalizeNodes } from '../model/writer.js';
  */
 export function convertToModelFragment() {
 	return ( evt, data, consumable, conversionApi ) => {
-		// Second argument in `consumable.test` is discarded for ViewDocumentFragment but is needed for ViewElement.
-		if ( !data.output && consumable.test( data.input, { name: true } ) ) {
+		// Second argument in `consumable.consume` is discarded for ViewDocumentFragment but is needed for ViewElement.
+		if ( !data.output && consumable.consume( data.input, { name: true } ) ) {
 			const convertedChildren = conversionApi.convertChildren( data.input, consumable, data );
 
 			data.output = new ModelDocumentFragment( normalizeNodes( convertedChildren ) );

+ 1 - 1
packages/ckeditor5-engine/src/dataprocessor/xmldataprocessor.js

@@ -89,7 +89,7 @@ export default class XmlDataProcessor {
 		const domFragment = this._toDom( data );
 
 		// Convert DOM DocumentFragment to view DocumentFragment.
-		return this._domConverter.domToView( domFragment );
+		return this._domConverter.domToView( domFragment, { keepOriginalCase: true } );
 	}
 
 	/**

+ 1 - 0
packages/ckeditor5-engine/src/dev-utils/model.js

@@ -345,6 +345,7 @@ function convertToModelElement() {
 	return ( evt, data, consumable, conversionApi ) => {
 		const schemaQuery = {
 			name: data.input.name,
+			attributes: Array.from( data.input.getAttributeKeys() ),
 			inside: data.context
 		};
 

+ 5 - 3
packages/ckeditor5-engine/src/view/domconverter.js

@@ -252,8 +252,9 @@ export default class DomConverter {
 	 * @param {Node|DocumentFragment} domNode DOM node or document fragment to transform.
 	 * @param {Object} [options] Conversion options.
 	 * @param {Boolean} [options.bind=false] Determines whether new elements will be bound.
-	 * @param {Boolean} [options.withChildren=true] It true node's and document fragment's children will be converted too.
-	 * @returns {engine.view.Node|engine.view.DocumentFragment|null} Converted node or document fragment. Null
+	 * @param {Boolean} [options.withChildren=true] If `true`, node's and document fragment's children will be converted too.
+	 * @param {Boolean} [options.keepOriginalCase=false] If `false`, node's tag name will be converter to lower case.
+	 * @returns {engine.view.Node|engine.view.DocumentFragment|null} Converted node or document fragment or `null`
 	 * if DOM node is a {@link engine.view.filler filler}.
 	 */
 	domToView( domNode, options = {} ) {
@@ -283,7 +284,8 @@ export default class DomConverter {
 				}
 			} else {
 				// Create view element.
-				viewElement = new ViewElement( domNode.tagName.toLowerCase() );
+				const viewName = options.keepOriginalCase ? domNode.tagName : domNode.tagName.toLowerCase();
+				viewElement = new ViewElement( viewName );
 
 				if ( options.bind ) {
 					this.bindElements( domNode, viewElement );

+ 28 - 1
packages/ckeditor5-engine/src/view/writer.js

@@ -28,7 +28,8 @@ export default {
 	move,
 	wrap,
 	wrapPosition,
-	unwrap
+	unwrap,
+	rename
 };
 
 /**
@@ -355,6 +356,10 @@ export function remove( range ) {
  * {@link engine.view.Range#start start} and {@link engine.view.Range#end end} positions.
  */
 export function move( sourceRange, targetPosition ) {
+	if ( sourceRange.start.parent == targetPosition.parent ) {
+		targetPosition.offset -= sourceRange.end.offset - sourceRange.start.offset;
+	}
+
 	const nodes = remove( sourceRange );
 
 	return insert( targetPosition, nodes );
@@ -549,6 +554,28 @@ export function unwrap( range, attribute ) {
 	return new Range( start, end );
 }
 
+/**
+ * Renames element by creating a copy of renamed element but with changed name and then moving contents of the
+ * old element to the new one. Keep in mind that this will invalidate all {@link engine.view.Position positions} which
+ * has renamed element as {@link engine.view.Position#parent a parent}.
+ *
+ * New element has to be created because `Element#tagName` property in DOM is readonly.
+ *
+ * Since this function creates a new element and removes the given one, the new element is returned to keep reference.
+ *
+ * @param {engine.view.ContainerElement} viewElement Element to be renamed.
+ * @param {String} newName New name for element.
+ */
+export function rename( viewElement, newName ) {
+	const newElement = new ContainerElement( newName, viewElement.getAttributes() );
+
+	insert( Position.createAfter( viewElement ), newElement );
+	move( Range.createIn( viewElement ), Position.createAt( newElement ) );
+	remove( Range.createOn( viewElement ) );
+
+	return newElement;
+}
+
 // Returns first parent container of specified {@link engine.view.Position Position}.
 // Position's parent node is checked as first, then next parents are checked.
 // Note that {@link engine.view.DocumentFragment DocumentFragment} is treated like a container.

+ 22 - 7
packages/ckeditor5-engine/tests/conversion/buildviewconverter.js

@@ -341,21 +341,36 @@ describe( 'View converter builder', () => {
 		expect( modelToString( result ) ).to.equal( '<div class="myClass"><abcd>foo</abcd></div>' );
 	} );
 
-	it( 'should filter out structure that is wrong with schema', () => {
-		buildViewConverter().for( dispatcher ).fromElement( 'strong' ).toAttribute( 'bold', true );
+	it( 'should filter out structure that is wrong with schema - elements', () => {
 		buildViewConverter().for( dispatcher ).fromElement( 'div' ).toElement( 'div' );
 		buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' );
 
-		schema.disallow( { name: '$text', attributes: 'bold', inside: 'paragraph' } );
 		schema.disallow( { name: 'div', inside: '$root' } );
 
-		dispatcher.on( 'element', convertToModelFragment() );
+		dispatcher.on( 'element', convertToModelFragment(), { priority: 'lowest' } );
 
 		let viewElement = new ViewContainerElement( 'div', null,
 			new ViewContainerElement( 'p', null,
-				new ViewAttributeElement( 'strong', null,
-					new ViewText( 'foo' )
-				)
+				new ViewText( 'foo' )
+			)
+		);
+
+		let result = dispatcher.convert( viewElement, objWithContext );
+
+		expect( modelToString( result ) ).to.equal( '<paragraph>foo</paragraph>' );
+	} );
+
+	it( 'should filter out structure that is wrong with schema - attributes', () => {
+		buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' );
+		buildViewConverter().for( dispatcher ).fromElement( 'strong' ).toAttribute( 'bold', true );
+
+		schema.disallow( { name: '$text', attributes: 'bold', inside: 'paragraph' } );
+
+		dispatcher.on( 'element', convertToModelFragment(), { priority: 'lowest' } );
+
+		let viewElement = new ViewContainerElement( 'p', null,
+			new ViewAttributeElement( 'strong', null,
+				new ViewText( 'foo' )
 			)
 		);
 

+ 101 - 2
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -97,6 +97,18 @@ describe( 'insertText', () => {
 
 		expect( viewToString( viewRoot ) ).to.equal( '<div>நிலைக்கு</div>' );
 	} );
+
+	it( 'should be possible to override it', () => {
+		modelRoot.appendChildren( new ModelText( 'foobar' ) );
+		dispatcher.on( 'insert:$text', insertText() );
+		dispatcher.on( 'insert:$text', ( evt, data, consumable ) => {
+			consumable.consume( data.item, 'insert' );
+		}, { priority: 'high' } );
+
+		dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
+
+		expect( viewToString( viewRoot ) ).to.equal( '<div></div>' );
+	} );
 } );
 
 describe( 'insertElement', () => {
@@ -196,6 +208,48 @@ describe( 'setAttribute/removeAttribute', () => {
 
 		expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p><div class="nice"></div></div>' );
 	} );
+
+	it( 'should be possible to override setAttribute', () => {
+		const modelElement = new ModelElement( 'paragraph', { class: 'foo' }, new ModelText( 'foobar' ) );
+		const viewElement = new ViewContainerElement( 'p' );
+
+		modelRoot.appendChildren( modelElement );
+		dispatcher.on( 'insert:paragraph', insertElement( viewElement ) );
+		dispatcher.on( 'insert:$text', insertText() );
+		dispatcher.on( 'addAttribute:class', setAttribute() );
+		dispatcher.on( 'addAttribute:class', ( evt, data, consumable ) => {
+			consumable.consume( data.item, 'addAttribute:class' );
+		}, { priority: 'high' } );
+
+		dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
+
+		// No attribute set.
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
+	} );
+
+	it( 'should be possible to override removeAttribute', () => {
+		const modelElement = new ModelElement( 'paragraph', { class: 'foo' }, new ModelText( 'foobar' ) );
+		const viewElement = new ViewContainerElement( 'p' );
+
+		modelRoot.appendChildren( modelElement );
+		dispatcher.on( 'insert:paragraph', insertElement( viewElement ) );
+		dispatcher.on( 'insert:$text', insertText() );
+		dispatcher.on( 'addAttribute:class', setAttribute() );
+		dispatcher.on( 'removeAttribute:class', removeAttribute() );
+		dispatcher.on( 'removeAttribute:class', ( evt, data, consumable ) => {
+			consumable.consume( data.item, 'removeAttribute:class' );
+		}, { priority: 'high' } );
+
+		dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
+
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p class="foo">foobar</p></div>' );
+
+		modelElement.removeAttribute( 'class' );
+		dispatcher.convertAttribute( 'removeAttribute', createRangeOnElementOnly( modelElement ), 'class', 'bar', null );
+
+		// Nothing changed.
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p class="foo">foobar</p></div>' );
+	} );
 } );
 
 describe( 'wrap/unwrap', () => {
@@ -305,6 +359,50 @@ describe( 'wrap/unwrap', () => {
 
 		expect( viewToString( viewRoot ) ).to.equal( '<div><p>நிலைக்கு</p></div>' );
 	} );
+
+	it( 'should be possible to override wrap', () => {
+		const modelElement = new ModelElement( 'paragraph', null, new ModelText( 'foobar', { bold: true } ) );
+		const viewP = new ViewContainerElement( 'p' );
+		const viewB = new ViewAttributeElement( 'b' );
+
+		modelRoot.appendChildren( modelElement );
+		dispatcher.on( 'insert:paragraph', insertElement( viewP ) );
+		dispatcher.on( 'insert:$text', insertText() );
+		dispatcher.on( 'addAttribute:bold', wrap( viewB ) );
+		dispatcher.on( 'addAttribute:bold', ( evt, data, consumable ) => {
+			consumable.consume( data.item, 'addAttribute:bold' );
+		}, { priority: 'high' } );
+
+		dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
+
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p>foobar</p></div>' );
+	} );
+
+	it( 'should be possible to override unwrap', () => {
+		const modelElement = new ModelElement( 'paragraph', null, new ModelText( 'foobar', { bold: true } ) );
+		const viewP = new ViewContainerElement( 'p' );
+		const viewB = new ViewAttributeElement( 'b' );
+
+		modelRoot.appendChildren( modelElement );
+		dispatcher.on( 'insert:paragraph', insertElement( viewP ) );
+		dispatcher.on( 'insert:$text', insertText() );
+		dispatcher.on( 'addAttribute:bold', wrap( viewB ) );
+		dispatcher.on( 'removeAttribute:bold', unwrap( viewB ) );
+		dispatcher.on( 'removeAttribute:bold', ( evt, data, consumable ) => {
+			consumable.consume( data.item, 'removeAttribute:bold' );
+		}, { priority: 'high' } );
+
+		dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
+
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p><b>foobar</b></p></div>' );
+
+		modelWriter.removeAttribute( ModelRange.createIn( modelElement ), 'bold' );
+
+		dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelElement ), 'bold', true, null );
+
+		// Nothing changed.
+		expect( viewToString( viewRoot ) ).to.equal( '<div><p><b>foobar</b></p></div>' );
+	} );
 } );
 
 describe( 'move', () => {
@@ -505,11 +603,12 @@ describe( 'rename', () => {
 	} );
 
 	it( 'should enable default rename conversion, that uses already registered callbacks', () => {
+		const insertCallCount = converters.insert.callCount;
+
 		expect( viewRoot.getChild( 0 ).name ).to.equal( 'oldName' );
 		dispatcher.convertRename( element, oldName );
 
-		// Called twice, first time when renamed element was originally inserted to model and converted to view.
-		expect( converters.insert.calledTwice ).to.be.true;
+		expect( converters.insert.callCount - insertCallCount ).to.equal( 1 );
 		expect( converters.move.calledOnce ).to.be.true;
 		expect( converters.remove.calledOnce ).to.be.true;
 

+ 4 - 4
packages/ckeditor5-engine/tests/conversion/modelconversiondispatcher.js

@@ -287,7 +287,7 @@ describe( 'ModelConversionDispatcher', () => {
 	} );
 
 	describe( 'convertMove', () => {
-		it( 'should fire single event for moved range', () => {
+		it( 'should fire event for moved range', () => {
 			root.appendChildren( new ModelText( 'barfoo' ) );
 
 			const range = ModelRange.createFromParentsAndOffsets( root, 0, root, 3 );
@@ -298,14 +298,14 @@ describe( 'ModelConversionDispatcher', () => {
 				loggedEvents.push( log );
 			} );
 
-			dispatcher.convertMove( ModelPosition.createFromParentAndOffset( root , 3 ), range );
+			dispatcher.convertMove( ModelPosition.createFromParentAndOffset( root , 0 ), range );
 
-			expect( loggedEvents ).to.deep.equal( [ 'move:3:0:3' ] );
+			expect( loggedEvents ).to.deep.equal( [ 'move:0:3:3' ] );
 		} );
 	} );
 
 	describe( 'convertRemove', () => {
-		it( 'should fire single event for removed range', () => {
+		it( 'should fire event for removed range', () => {
 			root.appendChildren( new ModelText( 'foo' ) );
 			doc.graveyard.appendChildren( new ModelText( 'bar' ) );
 

+ 2 - 0
packages/ckeditor5-engine/tests/dev-utils/model.js

@@ -24,8 +24,10 @@ describe( 'model test utils', () => {
 
 		document.schema.registerItem( 'a', '$inline' );
 		document.schema.allow( { name: 'a', inside: '$root' } );
+		document.schema.allow( { name: 'a', inside: '$root', attributes: [ 'bar', 'car', 'foo' ] } );
 		document.schema.registerItem( 'b', '$inline' );
 		document.schema.allow( { name: 'b', inside: '$root' } );
+		document.schema.allow( { name: 'b', inside: '$root', attributes: [ 'barFoo', 'fooBar', 'x' ] } );
 		document.schema.registerItem( 'c', '$inline' );
 		document.schema.allow( { name: 'c', inside: '$root' } );
 		document.schema.registerItem( 'paragraph', '$block' );

+ 10 - 0
packages/ckeditor5-engine/tests/view/writer/move.js

@@ -6,6 +6,7 @@
 /* bender-tags: view, browser-only */
 
 import { move } from '/ckeditor5/engine/view/writer.js';
+import ViewPosition from '/ckeditor5/engine/view/position.js';
 import { stringify, parse } from '/ckeditor5/engine/dev-utils/view.js';
 
 describe( 'writer', () => {
@@ -102,5 +103,14 @@ describe( 'writer', () => {
 		it( 'should move part of the text node in document fragment', () => {
 			test( 'fo{ob}ar', 'fo{}ar', 'foar', 'fo{ob}ar' );
 		} );
+
+		it( 'should correctly move text nodes inside same parent', () => {
+			let { view, selection } = parse( '<container:p>[<attribute:b>a</attribute:b>]b<attribute:b>c</attribute:b></container:p>' );
+
+			const newRange = move( selection.getFirstRange(), ViewPosition.createAt( view, 2 ) );
+
+			const expectedView = '<container:p>b[<attribute:b>a}c</attribute:b></container:p>';
+			expect( stringify( view, newRange, { showType: true } ) ).to.equal( expectedView );
+		} );
 	} );
 } );

+ 40 - 0
packages/ckeditor5-engine/tests/view/writer/rename.js

@@ -0,0 +1,40 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* bender-tags: view, browser-only */
+
+import { rename } from '/ckeditor5/engine/view/writer.js';
+import { parse } from '/tests/engine/_utils/view.js';
+
+describe( 'writer', () => {
+	describe( 'rename', () => {
+		let root, foo;
+
+		beforeEach( () => {
+			root = parse( '<container:div><container:foo foo="1">xxx</container:foo></container:div>' );
+
+			foo = root.getChild( 0 );
+		} );
+
+		it( 'should rename given element by inserting a new element in the place of the old one', () => {
+			const text = foo.getChild( 0 );
+
+			rename( foo, 'bar' );
+
+			const bar = root.getChild( 0 );
+
+			expect( bar ).not.to.equal( foo );
+			expect( bar.name ).to.equal( 'bar' );
+			expect( bar.getAttribute( 'foo' ) ).to.equal( '1' );
+			expect( bar.getChild( 0 ) ).to.equal( text );
+		} );
+
+		it( 'should return a reference to the inserted element', () => {
+			const bar = rename( foo, 'bar' );
+
+			expect( bar ).to.equal( root.getChild( 0 ) );
+		} );
+	} );
+} );