Procházet zdrojové kódy

Merge pull request #1196 from ckeditor/t/858

Other: Refactoring: make writer a protected operations util.
Piotr Jasiun před 8 roky
rodič
revize
be480b46e3

+ 3 - 2
packages/ckeditor5-engine/src/controller/datacontroller.js

@@ -295,10 +295,11 @@ export default class DataController {
 	 *
 	 * @fires module:engine/controller/datacontroller~DataController#getSelectedContent
 	 * @param {module:engine/model/selection~Selection} selection The selection of which content will be retrieved.
+	 * @param {module:engine/model/batch~Batch} batch Batch to which deltas will be added.
 	 * @returns {module:engine/model/documentfragment~DocumentFragment} Document fragment holding the clone of the selected content.
 	 */
-	getSelectedContent( selection ) {
-		return getSelectedContent( selection );
+	getSelectedContent( selection, batch ) {
+		return getSelectedContent( selection, batch );
 	}
 
 	/**

+ 10 - 12
packages/ckeditor5-engine/src/controller/getselectedcontent.js

@@ -7,11 +7,8 @@
  * @module engine/controller/getselectedcontent
  */
 
-import DocumentFragment from '../model/documentfragment';
 import Range from '../model/range';
 import Position from '../model/position';
-import Text from '../model/text';
-import { remove } from '../model/writer';
 
 /**
  * Gets a clone of the selected content.
@@ -25,10 +22,11 @@ import { remove } from '../model/writer';
  *		<quote><h>st</h></quote><p>se</p>
  *
  * @param {module:engine/model/selection~Selection} selection The selection of which content will be returned.
+ * @param {module:engine/model/batch~Batch} batch Batch to which deltas will be added.
  * @returns {module:engine/model/documentfragment~DocumentFragment}
  */
-export default function getSelectedContent( selection ) {
-	const frag = new DocumentFragment();
+export default function getSelectedContent( selection, batch ) {
+	const frag = batch.createDocumentFragment();
 	const range = selection.getFirstRange();
 
 	if ( !range || range.isCollapsed ) {
@@ -69,9 +67,9 @@ export default function getSelectedContent( selection ) {
 	// Clone the whole contents.
 	for ( const item of flatSubtreeRange.getItems( { shallow: true } ) ) {
 		if ( item.is( 'textProxy' ) ) {
-			frag.appendChildren( new Text( item.data, item.getAttributes() ) );
+			batch.appendText( item.data, item.getAttributes(), frag );
 		} else {
-			frag.appendChildren( item.clone( true ) );
+			batch.append( item.clone( true ), frag );
 		}
 	}
 
@@ -97,8 +95,8 @@ export default function getSelectedContent( selection ) {
 		const leftExcessRange = new Range( Position.createAt( frag ), newRange.start );
 		const rightExcessRange = new Range( newRange.end, Position.createAt( frag, 'end' ) );
 
-		removeRangeContent( rightExcessRange );
-		removeRangeContent( leftExcessRange );
+		removeRangeContent( rightExcessRange, batch );
+		removeRangeContent( leftExcessRange, batch );
 	}
 
 	return frag;
@@ -106,7 +104,7 @@ export default function getSelectedContent( selection ) {
 
 // After https://github.com/ckeditor/ckeditor5-engine/issues/690 is fixed,
 // this function will, most likely, be able to rewritten using getMinimalFlatRanges().
-function removeRangeContent( range ) {
+function removeRangeContent( range, batch ) {
 	const parentsToCheck = [];
 
 	Array.from( range.getItems( { direction: 'backward' } ) )
@@ -128,7 +126,7 @@ function removeRangeContent( range ) {
 		.forEach( itemRange => {
 			parentsToCheck.push( itemRange.start.parent );
 
-			remove( itemRange );
+			batch.remove( itemRange );
 		} );
 
 	// Remove ancestors of the removed items if they turned to be empty now
@@ -141,7 +139,7 @@ function removeRangeContent( range ) {
 
 			parent = parent.parent;
 
-			remove( removeRange );
+			batch.remove( removeRange );
 		}
 	} );
 }

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

@@ -20,7 +20,6 @@ import ModelSelection from '../model/selection';
 import ModelDocumentFragment from '../model/documentfragment';
 import ModelElement from '../model/element';
 import ModelText from '../model/text';
-import modelWriter from '../model/writer';
 
 import ViewConversionDispatcher from '../conversion/viewconversiondispatcher';
 import ViewSelection from '../view/selection';
@@ -324,9 +323,7 @@ export function parse( data, schema, batch, options = {} ) {
 
 function convertToModelFragment() {
 	return ( evt, data, consumable, conversionApi ) => {
-		const convertedChildren = conversionApi.convertChildren( data.input, consumable, data );
-
-		data.output = new ModelDocumentFragment( modelWriter.normalizeNodes( convertedChildren ) );
+		data.output = conversionApi.convertChildren( data.input, consumable, data );
 		conversionApi.mapper.bindElements( data.output, data.input );
 
 		evt.stop();

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/attributeoperation.js

@@ -10,7 +10,7 @@
 import Operation from './operation';
 import Range from '../range';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import writer from '../writer';
+import { _setAttribute } from './utils';
 import isEqual from '@ckeditor/ckeditor5-utils/src/lib/lodash/isEqual';
 
 /**
@@ -151,7 +151,7 @@ export default class AttributeOperation extends Operation {
 		// If value to set is same as old value, don't do anything.
 		if ( !isEqual( this.oldValue, this.newValue ) ) {
 			// Execution.
-			writer.setAttribute( this.range, this.key, this.newValue );
+			_setAttribute( this.range, this.key, this.newValue );
 		}
 
 		return { range: this.range, key: this.key, oldValue: this.oldValue, newValue: this.newValue };

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/detachoperation.js

@@ -10,7 +10,7 @@
 import Operation from './operation';
 import Position from '../position';
 import Range from '../range';
-import { remove } from '../writer';
+import { _remove } from './utils';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
@@ -73,7 +73,7 @@ export default class DetachOperation extends Operation {
 			throw new CKEditorError( 'detach-operation-on-document-node: Cannot detach document node.' );
 		}
 
-		const nodes = remove( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ) );
+		const nodes = _remove( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ) );
 
 		return { nodes };
 	}

+ 3 - 3
packages/ckeditor5-engine/src/model/operation/insertoperation.js

@@ -11,7 +11,7 @@ import Operation from './operation';
 import Position from '../position';
 import NodeList from '../nodelist';
 import RemoveOperation from './removeoperation';
-import { insert, normalizeNodes } from '../writer';
+import { _insert, _normalizeNodes } from './utils';
 import Text from '../text';
 import Element from '../element';
 
@@ -45,7 +45,7 @@ export default class InsertOperation extends Operation {
 		 * @readonly
 		 * @member {module:engine/model/nodelist~NodeList} module:engine/model/operation/insertoperation~InsertOperation#nodeList
 		 */
-		this.nodes = new NodeList( normalizeNodes( nodes ) );
+		this.nodes = new NodeList( _normalizeNodes( nodes ) );
 
 		/**
 		 * @inheritDoc
@@ -94,7 +94,7 @@ export default class InsertOperation extends Operation {
 		const originalNodes = this.nodes;
 		this.nodes = new NodeList( [ ...originalNodes ].map( node => node.clone( true ) ) );
 
-		const range = insert( this.position, originalNodes );
+		const range = _insert( this.position, originalNodes );
 
 		return { range };
 	}

+ 2 - 2
packages/ckeditor5-engine/src/model/operation/moveoperation.js

@@ -12,7 +12,7 @@ import Position from '../position';
 import Range from '../range';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
-import writer from './../writer';
+import { _move } from './utils';
 
 /**
  * Operation to move a range of {@link module:engine/model/item~Item model items}
@@ -184,7 +184,7 @@ export default class MoveOperation extends Operation {
 			}
 		}
 
-		const range = writer.move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
+		const range = _move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition );
 
 		return {
 			sourcePosition: this.sourcePosition,

+ 33 - 54
packages/ckeditor5-engine/src/model/writer.js → packages/ckeditor5-engine/src/model/operation/utils.js

@@ -4,54 +4,36 @@
  */
 
 /**
- * @module engine/model/writer
+ * @module engine/model/operation/utils
  */
 
-import Node from './node';
-import Text from './text';
-import TextProxy from './textproxy';
-import Range from './range';
-import DocumentFragment from './documentfragment';
-import NodeList from './nodelist';
+import Node from '../node';
+import Text from '../text';
+import TextProxy from '../textproxy';
+import Range from '../range';
+import DocumentFragment from '../documentfragment';
+import NodeList from '../nodelist';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
- * Contains functions used for composing model tree, grouped together under "model writer" name. Those functions
- * are built on top of {@link module:engine/model/node~Node node}, and it's child classes', APIs.
+ * Contains functions used for composing model tree by {@link module:engine/model/operation~Operation operations}.
+ * Those functions are built on top of {@link module:engine/model/node~Node node}, and it's child classes', APIs.
  *
- * Model writer API has multiple advantages and it is highly recommended to use it when changing model tree and nodes:
- * * model writer API {@link module:engine/model/writer~writer.normalizeNodes normalizes inserted nodes}, which means that you can insert
- * not only {@link module:engine/model/node~Node nodes}, but also `String`s, {@link module:engine/model/textproxy~TextProxy text proxies}
- * and
- * {@link module:engine/model/documentfragment~DocumentFragment document fragments},
- * * model writer API operates on {@link module:engine/model/position~Position positions}, which means that you have
- * better control over manipulating model tree as positions operate on offsets rather than indexes,
- * * model writer API automatically merges {@link module:engine/model/text~Text text nodes} with same attributes, which means
- * lower memory usage and better efficiency.
- *
- * @namespace writer
+ * @protected
+ * @namespace utils
  */
-const writer = {
-	insert,
-	remove,
-	move,
-	setAttribute,
-	removeAttribute,
-	normalizeNodes
-};
-
-export default writer;
 
 /**
  * Inserts given nodes at given position.
  *
- * @function module:engine/model/writer~writer.insert
+ * @protected
+ * @function module:engine/model/operation/utils~utils.insert
  * @param {module:engine/model/position~Position} position Position at which nodes should be inserted.
  * @param {module:engine/model/node~NodeSet} nodes Nodes to insert.
  * @returns {module:engine/model/range~Range} Range spanning over inserted elements.
  */
-export function insert( position, nodes ) {
-	nodes = normalizeNodes( nodes );
+export function _insert( position, nodes ) {
+	nodes = _normalizeNodes( nodes );
 
 	// We have to count offset before inserting nodes because they can get merged and we would get wrong offsets.
 	const offset = nodes.reduce( ( sum, node ) => sum + node.offsetSize, 0 );
@@ -75,18 +57,19 @@ export function insert( position, nodes ) {
 /**
  * Removed nodes in given range. Only {@link module:engine/model/range~Range#isFlat flat} ranges are accepted.
  *
- * @function module:engine/model/writer~writer.remove
+ * @protected
+ * @function module:engine/model/operation/utils~utils.remove
  * @param {module:engine/model/range~Range} range Range containing nodes to remove.
  * @returns {Array.<module:engine/model/node~Node>}
  */
-export function remove( range ) {
+export function _remove( range ) {
 	if ( !range.isFlat ) {
 		/**
 		 * Trying to remove a range which starts and ends in different element.
 		 *
-		 * @error model-writer-remove-range-not-flat
+		 * @error operation-utils-remove-range-not-flat
 		 */
-		throw new CKEditorError( 'model-writer-remove-range-not-flat: ' +
+		throw new CKEditorError( 'operation-utils-remove-range-not-flat: ' +
 			'Trying to remove a range which starts and ends in different element.' );
 	}
 
@@ -109,38 +92,42 @@ export function remove( range ) {
 /**
  * Moves nodes in given range to given target position. Only {@link module:engine/model/range~Range#isFlat flat} ranges are accepted.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.move
  * @param {module:engine/model/range~Range} sourceRange Range containing nodes to move.
  * @param {module:engine/model/position~Position} targetPosition Position to which nodes should be moved.
  * @returns {module:engine/model/range~Range} Range containing moved nodes.
  */
-export function move( sourceRange, targetPosition ) {
+export function _move( sourceRange, targetPosition ) {
 	if ( !sourceRange.isFlat ) {
 		/**
 		 * Trying to move a range which starts and ends in different element.
 		 *
-		 * @error model-writer-move-range-not-flat
+		 * @error operation-utils-move-range-not-flat
 		 */
-		throw new CKEditorError( 'model-writer-move-range-not-flat: ' +
+		throw new CKEditorError( 'operation-utils-move-range-not-flat: ' +
 			'Trying to move a range which starts and ends in different element.' );
 	}
 
-	const nodes = this.remove( sourceRange );
+	const nodes = _remove( sourceRange );
 
 	// We have to fix `targetPosition` because model changed after nodes from `sourceRange` got removed and
 	// that change might have an impact on `targetPosition`.
 	targetPosition = targetPosition._getTransformedByDeletion( sourceRange.start, sourceRange.end.offset - sourceRange.start.offset );
 
-	return this.insert( targetPosition, nodes );
+	return _insert( targetPosition, nodes );
 }
 
 /**
  * Sets given attribute on nodes in given range.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.setAttribute
  * @param {module:engine/model/range~Range} range Range containing nodes that should have the attribute set.
  * @param {String} key Key of attribute to set.
  * @param {*} value Attribute value.
  */
-export function setAttribute( range, key, value ) {
+export function _setAttribute( range, key, value ) {
 	// Range might start or end in text nodes, so we have to split them.
 	_splitNodeAtPosition( range.start );
 	_splitNodeAtPosition( range.end );
@@ -166,24 +153,16 @@ export function setAttribute( range, key, value ) {
 	_mergeNodesAtIndex( range.end.parent, range.end.index );
 }
 
-/**
- * Removes given attribute from nodes in given range.
- *
- * @param {module:engine/model/range~Range} range Range containing nodes that should have the attribute removed.
- * @param {String} key Key of attribute to remove.
- */
-export function removeAttribute( range, key ) {
-	this.setAttribute( range, key, null );
-}
-
 /**
  * Normalizes given object or an array of objects to an array of {@link module:engine/model/node~Node nodes}. See
  * {@link module:engine/model/node~NodeSet NodeSet} for details on how normalization is performed.
  *
+ * @protected
+ * @function module:engine/model/operation/utils~utils.normalizeNodes
  * @param {module:engine/model/node~NodeSet} nodes Objects to normalize.
  * @returns {Array.<module:engine/model/node~Node>} Normalized nodes.
  */
-export function normalizeNodes( nodes ) {
+export function _normalizeNodes( nodes ) {
 	const normalized = [];
 
 	if ( !( nodes instanceof Array ) ) {

+ 2 - 2
packages/ckeditor5-engine/tests/controller/datacontroller.js

@@ -433,7 +433,7 @@ describe( 'DataController', () => {
 
 			data.on( 'getSelectedContent', spy );
 
-			data.getSelectedContent( sel );
+			data.getSelectedContent( sel, modelDocument.batch() );
 
 			expect( spy.calledOnce ).to.be.true;
 		} );
@@ -443,7 +443,7 @@ describe( 'DataController', () => {
 
 			setData( modelDocument, '<paragraph>fo[ob]ar</paragraph>' );
 
-			const content = data.getSelectedContent( modelDocument.selection );
+			const content = data.getSelectedContent( modelDocument.selection, modelDocument.batch() );
 
 			expect( stringify( content ) ).to.equal( 'ob' );
 		} );

+ 34 - 34
packages/ckeditor5-engine/tests/controller/getselectedcontent.js

@@ -30,7 +30,7 @@ describe( 'Delete utils', () => {
 			it( 'returns empty fragment for no selection', () => {
 				setData( doc, 'abc' );
 
-				const frag = getSelectedContent( doc.selection );
+				const frag = getSelectedContent( doc.selection, doc.batch() );
 
 				expect( frag ).instanceOf( DocumentFragment );
 				expect( frag.isEmpty ).to.be.true;
@@ -39,7 +39,7 @@ describe( 'Delete utils', () => {
 			it( 'returns empty fragment for empty selection', () => {
 				setData( doc, 'a[]bc' );
 
-				const frag = getSelectedContent( doc.selection );
+				const frag = getSelectedContent( doc.selection, doc.batch() );
 
 				expect( frag ).instanceOf( DocumentFragment );
 				expect( frag.isEmpty ).to.be.true;
@@ -48,7 +48,7 @@ describe( 'Delete utils', () => {
 			it( 'gets one character', () => {
 				setData( doc, 'a[b]c' );
 
-				const frag = getSelectedContent( doc.selection );
+				const frag = getSelectedContent( doc.selection, doc.batch() );
 				const content = stringify( frag );
 
 				expect( frag ).instanceOf( DocumentFragment );
@@ -58,49 +58,49 @@ describe( 'Delete utils', () => {
 			it( 'gets full text', () => {
 				setData( doc, '[abc]' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( 'abc' );
 			} );
 
 			it( 'gets text with an attribute', () => {
 				setData( doc, 'xxx<$text bold="true">a[b]c</$text>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<$text bold="true">b</$text>' );
 			} );
 
 			it( 'gets text with attributes', () => {
 				setData( doc, 'x<$text bold="true">a[b</$text><$text italic="true">c]d</$text>x' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<$text bold="true">b</$text><$text italic="true">c</$text>' );
 			} );
 
 			it( 'gets text with and without attribute', () => {
 				setData( doc, '<$text bold="true">a[b</$text>c]d' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<$text bold="true">b</$text>c' );
 			} );
 
 			it( 'gets text and element', () => {
 				setData( doc, '[ab<image></image>c]' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( 'ab<image></image>c' );
 			} );
 
 			it( 'gets one element', () => {
 				setData( doc, 'a[<image></image>]b' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<image></image>' );
 			} );
 
 			it( 'gets multiple elements', () => {
 				setData( doc, '[<image></image><image></image>]' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<image></image><image></image>' );
 			} );
 		} );
@@ -128,63 +128,63 @@ describe( 'Delete utils', () => {
 			it( 'gets one character', () => {
 				setData( doc, '<paragraph>a[b]c</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( 'b' );
 			} );
 
 			it( 'gets entire paragraph content', () => {
 				setData( doc, '<paragraph>[a<image></image>b]</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( 'a<image></image>b' );
 			} );
 
 			it( 'gets two blocks - partial, partial', () => {
 				setData( doc, '<heading1>a[bc</heading1><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>bc</heading1><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets two blocks - full, partial', () => {
 				setData( doc, '<heading1>[abc</heading1><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>abc</heading1><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets two blocks - full, partial 2', () => {
 				setData( doc, '<heading1>[abc</heading1><paragraph>de<image></image>]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>abc</heading1><paragraph>de<image></image></paragraph>' );
 			} );
 
 			it( 'gets two blocks - full, partial 3', () => {
 				setData( doc, '<heading1>x</heading1><heading1>[abc</heading1><paragraph><image></image>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>abc</heading1><paragraph><image></image>de</paragraph>' );
 			} );
 
 			it( 'gets two blocks - full, partial 4', () => {
 				setData( doc, '<heading1>[abc</heading1><paragraph>de]f<image></image></paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>abc</heading1><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets two blocks - partial, full', () => {
 				setData( doc, '<heading1>a[bc</heading1><paragraph>def]</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>bc</heading1><paragraph>def</paragraph>' );
 			} );
 
 			it( 'gets two blocks - partial, full 2', () => {
 				setData( doc, '<heading1>a[<image></image>bc</heading1><paragraph>def]</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1><image></image>bc</heading1><paragraph>def</paragraph>' );
 			} );
 
@@ -192,7 +192,7 @@ describe( 'Delete utils', () => {
 			it( 'gets two blocks - empty, full', () => {
 				setData( doc, '<heading1>abc[</heading1><paragraph>def]</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<paragraph>def</paragraph>' );
 			} );
 
@@ -200,28 +200,28 @@ describe( 'Delete utils', () => {
 			it( 'gets two blocks - partial, empty', () => {
 				setData( doc, '<heading1>a[bc</heading1><paragraph>]def</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>bc</heading1>' );
 			} );
 
 			it( 'gets three blocks', () => {
 				setData( doc, '<heading1>a[bc</heading1><paragraph>x</paragraph><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<heading1>bc</heading1><paragraph>x</paragraph><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets block image', () => {
 				setData( doc, '<paragraph>a</paragraph>[<blockImage><caption>Foo</caption></blockImage>]<paragraph>b</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<blockImage><caption>Foo</caption></blockImage>' );
 			} );
 
 			it( 'gets two blocks', () => {
 				setData( doc, '<paragraph>a</paragraph>[<blockImage></blockImage><blockImage></blockImage>]<paragraph>b</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<blockImage></blockImage><blockImage></blockImage>' );
 			} );
 
@@ -229,7 +229,7 @@ describe( 'Delete utils', () => {
 			it( 'gets content when multiple text items needs to be removed from the right excess', () => {
 				setData( doc, '<paragraph>a[b</paragraph><paragraph>c]d<$text bold="true">e</$text>f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content )
 					.to.equal( '<paragraph>b</paragraph><paragraph>c</paragraph>' );
 			} );
@@ -238,7 +238,7 @@ describe( 'Delete utils', () => {
 			it( 'gets content when multiple text items needs to be removed from the left excess', () => {
 				setData( doc, '<paragraph>a<$text bold="true">b</$text>c[d</paragraph><paragraph>e]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content )
 					.to.equal( '<paragraph>d</paragraph><paragraph>e</paragraph>' );
 			} );
@@ -262,28 +262,28 @@ describe( 'Delete utils', () => {
 			it( 'gets content when ends are equally deeply nested', () => {
 				setData( doc, '<heading1>x</heading1><quote><paragraph>a[bc</paragraph><paragraph>de]f</paragraph></quote>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<paragraph>bc</paragraph><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets content when left end nested deeper', () => {
 				setData( doc, '<quote><paragraph>a[bc</paragraph></quote><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<quote><paragraph>bc</paragraph></quote><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets content when left end nested deeper 2', () => {
 				setData( doc, '<quote><paragraph>a[bc</paragraph><heading1>x</heading1></quote><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<quote><paragraph>bc</paragraph><heading1>x</heading1></quote><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets content when left end nested deeper 3', () => {
 				setData( doc, '<quote><heading1>x</heading1><paragraph>a[bc</paragraph></quote><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<quote><paragraph>bc</paragraph></quote><paragraph>de</paragraph>' );
 			} );
 
@@ -291,21 +291,21 @@ describe( 'Delete utils', () => {
 			it( 'gets content when left end nested deeper 4', () => {
 				setData( doc, '<quote><heading1>x[</heading1><paragraph>abc</paragraph></quote><paragraph>de]f</paragraph>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<quote><paragraph>abc</paragraph></quote><paragraph>de</paragraph>' );
 			} );
 
 			it( 'gets content when right end nested deeper', () => {
 				setData( doc, '<paragraph>a[bc</paragraph><quote><paragraph>de]f</paragraph></quote>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content ).to.equal( '<paragraph>bc</paragraph><quote><paragraph>de</paragraph></quote>' );
 			} );
 
 			it( 'gets content when both ends nested deeper than the middle element', () => {
 				setData( doc, '<quote><heading1>a[bc</heading1></quote><heading1>x</heading1><quote><heading1>de]f</heading1></quote>' );
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content )
 					.to.equal( '<quote><heading1>bc</heading1></quote><heading1>x</heading1><quote><heading1>de</heading1></quote>' );
 			} );
@@ -325,7 +325,7 @@ describe( 'Delete utils', () => {
 					'</quote>'
 				);
 
-				const content = stringify( getSelectedContent( doc.selection ) );
+				const content = stringify( getSelectedContent( doc.selection, doc.batch() ) );
 				expect( content )
 					.to.equal( '<paragraph>ar</paragraph>bo' );
 			} );

+ 28 - 38
packages/ckeditor5-engine/tests/conversion/advanced-converters.js

@@ -4,14 +4,12 @@
  */
 
 import ModelDocument from '../../src/model/document';
-import ModelDocumentFragment from '../../src/model/documentfragment';
 import ModelElement from '../../src/model/element';
 import ModelText from '../../src/model/text';
 import ModelTextProxy from '../../src/model/textproxy';
 import ModelRange from '../../src/model/range';
 import ModelPosition from '../../src/model/position';
 import ModelWalker from '../../src/model/treewalker';
-import modelWriter from '../../src/model/writer';
 
 import ViewElement from '../../src/view/element';
 import ViewContainerElement from '../../src/view/containerelement';
@@ -208,8 +206,8 @@ describe( 'advanced-converters', () => {
 
 			const viewFigureConverter = function( evt, data, consumable, conversionApi ) {
 				if ( consumable.consume( data.input, { name: true } ) ) {
-					const modelImage = conversionApi.convertItem( data.input.getChild( 0 ), consumable, batch );
-					const modelCaption = conversionApi.convertItem( data.input.getChild( 1 ), consumable, batch );
+					const modelImage = conversionApi.convertItem( data.input.getChild( 0 ), consumable );
+					const modelCaption = conversionApi.convertItem( data.input.getChild( 1 ), consumable );
 
 					modelImage.appendChildren( modelCaption );
 
@@ -217,24 +215,18 @@ describe( 'advanced-converters', () => {
 				}
 			};
 
-			const viewImageConverter = function( evt, data, consumable ) {
+			const viewImageConverter = function( evt, data, consumable, conversionApi ) {
 				if ( consumable.consume( data.input, { name: true } ) ) {
-					const modelImage = new ModelElement( 'image' );
-
-					for ( const attributeKey of data.input.getAttributeKeys() ) {
-						modelImage.setAttribute( attributeKey, data.input.getAttribute( attributeKey ) );
-					}
-
-					data.output = modelImage;
+					data.output = conversionApi.batch.createElement( 'image', data.input.getAttributes() );
 				}
 			};
 
 			const viewFigcaptionConverter = function( evt, data, consumable, conversionApi ) {
 				if ( consumable.consume( data.input, { name: true } ) ) {
-					const modelCaption = new ModelElement( 'caption' );
-					const children = conversionApi.convertChildren( data.input, consumable, batch );
+					const modelCaption = conversionApi.batch.createElement( 'caption' );
+					const children = conversionApi.convertChildren( data.input, consumable );
 
-					modelCaption.appendChildren( children );
+					conversionApi.batch.append( children, modelCaption );
 
 					data.output = modelCaption;
 				}
@@ -250,14 +242,14 @@ describe( 'advanced-converters', () => {
 		} );
 
 		it( 'should convert model images changes without caption to view', () => {
-			const modelElement = new ModelElement( 'image', { src: 'bar.jpg', title: 'bar' } );
-			modelRoot.appendChildren( modelElement );
+			const modelElement = batch.createElement( 'image', { src: 'bar.jpg', title: 'bar' } );
+			batch.append( modelElement, modelRoot );
 			modelDispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><img src="bar.jpg" title="bar"></img></div>' );
 
-			modelElement.setAttribute( 'src', 'new.jpg' );
-			modelElement.removeAttribute( 'title' );
+			batch.setAttribute( 'src', 'new.jpg', modelElement );
+			batch.removeAttribute( 'title', modelElement );
 			modelDispatcher.convertAttribute( 'changeAttribute', createRangeOnElementOnly( modelElement ), 'src', 'bar.jpg', 'new.jpg' );
 			modelDispatcher.convertAttribute( 'removeAttribute', createRangeOnElementOnly( modelElement ), 'title', 'bar', null );
 
@@ -372,7 +364,7 @@ describe( 'advanced-converters', () => {
 			viewDispatcher.on( 'element:a', ( evt, data, consumable, conversionApi ) => {
 				if ( consumable.consume( data.input, { name: true, attribute: 'href' } ) ) {
 					if ( !data.output ) {
-						data.output = conversionApi.convertChildren( data.input, consumable, batch );
+						data.output = conversionApi.convertChildren( data.input, consumable );
 					}
 
 					for ( const child of data.output ) {
@@ -384,7 +376,7 @@ describe( 'advanced-converters', () => {
 			viewDispatcher.on( 'element:a', ( evt, data, consumable, conversionApi ) => {
 				if ( consumable.consume( data.input, { attribute: 'title' } ) ) {
 					if ( !data.output ) {
-						data.output = conversionApi.convertChildren( data.input, consumable, batch );
+						data.output = conversionApi.convertChildren( data.input, consumable );
 					}
 
 					for ( const child of data.output ) {
@@ -469,7 +461,7 @@ describe( 'advanced-converters', () => {
 						}
 					}
 
-					const children = conversionApi.convertChildren( data.input, consumable, batch );
+					const children = conversionApi.convertChildren( data.input, consumable );
 					data.output.appendChildren( children );
 				}
 			} );
@@ -486,15 +478,16 @@ describe( 'advanced-converters', () => {
 			expect( viewToString( viewRoot ) ).to.equal( '<div><a href="foo.html" title="Foo title">foo</a></div>' );
 
 			// Let's change link's attributes.
-			modelWriter.setAttribute( range, 'linkHref', 'bar.html' );
-			modelWriter.setAttribute( range, 'linkTitle', 'Bar title' );
+			batch.setAttributes( {
+				linkHref: 'bar.html',
+				linkTitle: 'Bar title'
+			}, range );
 			modelDispatcher.convertAttribute( 'changeAttribute', range, 'linkHref', 'foo.html', 'bar.html' );
 			modelDispatcher.convertAttribute( 'changeAttribute', range, 'linkTitle', 'Foo title', 'Bar title' );
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><a href="bar.html" title="Bar title">foo</a></div>' );
 
-			const removed = modelWriter.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ) );
-			modelDoc.graveyard.appendChildren( removed );
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ) );
 			modelDispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 0 ),
 				ModelRange.createIn( modelDoc.graveyard )
@@ -505,13 +498,13 @@ describe( 'advanced-converters', () => {
 			range = ModelRange.createIn( modelRoot );
 
 			// Let's remove just one attribute.
-			modelWriter.removeAttribute( range, 'linkTitle' );
+			batch.removeAttribute( 'linkTitle', range );
 			modelDispatcher.convertAttribute( 'removeAttribute', range, 'linkTitle', 'Bar title', null );
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><a href="bar.html">oo</a></div>' );
 
 			// Let's remove the other attribute.
-			modelWriter.removeAttribute( range, 'linkHref' );
+			batch.removeAttribute( 'linkHref', range );
 			modelDispatcher.convertAttribute( 'removeAttribute', range, 'linkHref', 'bar.html', null );
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div>oo</div>' );
@@ -603,7 +596,7 @@ describe( 'advanced-converters', () => {
 		viewDispatcher.on( 'element:a', ( evt, data, consumable, conversionApi ) => {
 			if ( consumable.consume( data.input, { name: true, attribute: 'href' } ) ) {
 				if ( !data.output ) {
-					data.output = conversionApi.convertChildren( data.input, consumable, batch );
+					data.output = conversionApi.convertChildren( data.input, consumable );
 				}
 
 				for ( const child of data.output ) {
@@ -616,7 +609,7 @@ describe( 'advanced-converters', () => {
 			if ( consumable.consume( data.input, { name: true } ) ) {
 				data.output = new ModelElement( 'paragraph' );
 
-				const children = conversionApi.convertChildren( data.input, consumable, batch );
+				const children = conversionApi.convertChildren( data.input, consumable );
 
 				for ( let i = 1; i < children.childCount; i++ ) {
 					const child = children.getChild( i );
@@ -633,13 +626,13 @@ describe( 'advanced-converters', () => {
 
 		viewDispatcher.on( 'element:table', ( evt, data, consumable, conversionApi ) => {
 			if ( consumable.consume( data.input, { name: true } ) ) {
-				data.output = conversionApi.convertChildren( data.input, consumable, batch );
+				data.output = conversionApi.convertChildren( data.input, consumable );
 			}
 		} );
 
 		viewDispatcher.on( 'element:td', ( evt, data, consumable, conversionApi ) => {
 			if ( consumable.consume( data.input, { name: true } ) ) {
-				data.output = conversionApi.convertChildren( data.input, consumable, batch );
+				data.output = conversionApi.convertChildren( data.input, consumable );
 			}
 		} );
 
@@ -654,10 +647,7 @@ describe( 'advanced-converters', () => {
 			] )
 		] );
 
-		const model = viewDispatcher.convert( viewTable, batch );
-		const modelFragment = new ModelDocumentFragment( model );
-
-		expect( modelToString( modelFragment ) )
+		expect( modelToString( viewDispatcher.convert( viewTable, batch ) ) )
 			.to.equal( '<paragraph>foo <$text linkHref="bar.html">bar</$text></paragraph><paragraph>abc</paragraph>' );
 	} );
 
@@ -681,7 +671,7 @@ describe( 'advanced-converters', () => {
 						}
 					}
 
-					data.output.appendChildren( conversionApi.convertChildren( data.input, consumable, batch ) );
+					data.output.appendChildren( conversionApi.convertChildren( data.input, consumable ) );
 				}
 			}, { priority: 'lowest' } );
 
@@ -705,7 +695,7 @@ describe( 'advanced-converters', () => {
 			viewDispatcher.on( 'element:strong', ( evt, data, consumable, conversionApi ) => {
 				if ( consumable.consume( data.input, { name: true } ) ) {
 					if ( !data.output ) {
-						data.output = conversionApi.convertChildren( data.input, consumable, batch );
+						data.output = conversionApi.convertChildren( data.input, consumable );
 					}
 
 					for ( const child of data.output ) {

+ 7 - 6
packages/ckeditor5-engine/tests/conversion/buildmodelconverter.js

@@ -10,7 +10,6 @@ import ModelElement from '../../src/model/element';
 import ModelText from '../../src/model/text';
 import ModelRange from '../../src/model/range';
 import ModelPosition from '../../src/model/position';
-import modelWriter from '../../src/model/writer';
 
 import ViewDocument from '../../src/view/document';
 import ViewElement from '../../src/view/element';
@@ -70,12 +69,14 @@ function viewToString( item ) {
 }
 
 describe( 'Model converter builder', () => {
-	let dispatcher, mapper, modelDoc, modelRoot, viewDoc, viewRoot, viewSelection;
+	let dispatcher, mapper, modelDoc, modelRoot, viewDoc, viewRoot, viewSelection, batch;
 
 	beforeEach( () => {
 		modelDoc = new ModelDocument();
 		modelRoot = modelDoc.createRoot( 'root', 'root' );
 
+		batch = modelDoc.batch();
+
 		viewDoc = new ViewDocument();
 		viewRoot = viewDoc.createRoot( 'div' );
 		viewSelection = viewDoc.selection;
@@ -145,7 +146,7 @@ describe( 'Model converter builder', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><strong>foo</strong></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelRoot ), 'bold' );
+			batch.removeAttribute( 'bold', modelRoot );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelRoot ), 'bold', true, null );
 
@@ -162,7 +163,7 @@ describe( 'Model converter builder', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><strong>foo</strong></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelRoot ), 'bold' );
+			batch.removeAttribute( 'bold', modelRoot );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelRoot ), 'bold', true, null );
 
@@ -179,13 +180,13 @@ describe( 'Model converter builder', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><em>foo</em></div>' );
 
-			modelWriter.setAttribute( ModelRange.createIn( modelRoot ), 'italic', 'i' );
+			batch.setAttribute( 'italic', 'i', modelRoot );
 
 			dispatcher.convertAttribute( 'changeAttribute', ModelRange.createIn( modelRoot ), 'italic', 'em', 'i' );
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><i>foo</i></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelRoot ), 'italic' );
+			batch.removeAttribute( 'italic', modelRoot );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelRoot ), 'italic', 'i', null );
 

+ 16 - 33
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -8,7 +8,6 @@ import ModelElement from '../../src/model/element';
 import ModelText from '../../src/model/text';
 import ModelRange from '../../src/model/range';
 import ModelPosition from '../../src/model/position';
-import modelWriter from '../../src/model/writer';
 
 import ViewElement from '../../src/view/element';
 import ViewContainerElement from '../../src/view/containerelement';
@@ -36,13 +35,15 @@ import {
 import { createRangeOnElementOnly } from '../../tests/model/_utils/utils';
 
 describe( 'model-to-view-converters', () => {
-	let dispatcher, modelDoc, modelRoot, mapper, viewRoot;
+	let dispatcher, modelDoc, modelRoot, mapper, viewRoot, batch;
 
 	beforeEach( () => {
 		modelDoc = new ModelDocument();
 		modelRoot = modelDoc.createRoot();
 		viewRoot = new ViewContainerElement( 'div' );
 
+		batch = modelDoc.batch();
+
 		mapper = new Mapper();
 		mapper.bindElements( modelRoot, viewRoot );
 
@@ -537,7 +538,7 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p><b>foobar</b></p></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelElement ), 'bold' );
+			batch.removeAttribute( 'bold', modelElement );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelElement ), 'bold', true, null );
 
@@ -564,7 +565,7 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p><b>foobar</b></p></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelElement ), 'style' );
+			batch.removeAttribute( 'style', modelElement );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelElement ), 'style', 'bold', null );
 
@@ -594,7 +595,7 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p>x<a href="http://foo.com">foo</a>x</p></div>' );
 
-			modelWriter.setAttribute( ModelRange.createIn( modelElement ), 'link', 'http://foobar.com' );
+			batch.setAttribute( 'link', 'http://foobar.com', modelElement );
 
 			dispatcher.convertAttribute(
 				'changeAttribute',
@@ -622,7 +623,7 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p>நி<b>லைக்</b>கு</p></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelElement ), 'bold' );
+			batch.removeAttribute( 'bold', modelElement );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelElement ), 'bold', true, null );
 
@@ -665,7 +666,7 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><p><b>foobar</b></p></div>' );
 
-			modelWriter.removeAttribute( ModelRange.createIn( modelElement ), 'bold' );
+			batch.removeAttribute( 'bold', modelElement );
 
 			dispatcher.convertAttribute( 'removeAttribute', ModelRange.createIn( modelElement ), 'bold', true, null );
 
@@ -1011,10 +1012,7 @@ describe( 'model-to-view-converters', () => {
 
 			dispatcher.convertInsertion( ModelRange.createIn( modelRoot ) );
 
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelDiv, 0, modelDiv, 6 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelDiv, 0, modelDiv, 6 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelDiv, 0 ),
@@ -1035,10 +1033,7 @@ describe( 'model-to-view-converters', () => {
 			dispatcher.on( 'remove', remove() );
 
 			// Remove 'b'.
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 3 ),
@@ -1059,10 +1054,7 @@ describe( 'model-to-view-converters', () => {
 			dispatcher.on( 'remove', remove() );
 
 			// Remove 'o<span></span>b'.
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelRoot, 2, modelRoot, 4 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 2, modelRoot, 4 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 2 ),
@@ -1084,7 +1076,7 @@ describe( 'model-to-view-converters', () => {
 			dispatcher.on( 'remove', remove() );
 
 			// Move <a></a> after "b". Can be e.g. a part of an unwrap delta (move + remove).
-			modelWriter.move(
+			batch.move(
 				ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ),
 				ModelPosition.createAt( modelRoot, 'end' )
 			);
@@ -1111,11 +1103,8 @@ describe( 'model-to-view-converters', () => {
 
 			dispatcher.on( 'remove', remove() );
 
-			// Move <a></a> to graveyard.
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			// Remove <a></a>.
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 0 ),
@@ -1188,10 +1177,7 @@ describe( 'model-to-view-converters', () => {
 
 			dispatcher.on( 'remove', remove() );
 
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 2 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 2 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 1 ),
@@ -1215,10 +1201,7 @@ describe( 'model-to-view-converters', () => {
 
 			dispatcher.on( 'remove', remove() );
 
-			modelWriter.move(
-				ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ),
-				ModelPosition.createAt( modelDoc.graveyard, 'end' )
-			);
+			batch.remove( ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ) );
 
 			dispatcher.convertRemove(
 				ModelPosition.createFromParentAndOffset( modelRoot, 3 ),

+ 16 - 0
packages/ckeditor5-engine/tests/model/operation/attributeoperation.js

@@ -398,6 +398,22 @@ describe( 'AttributeOperation', () => {
 		expect( root.getChild( 1 ).data ).to.equal( 'bcxyz' );
 	} );
 
+	it( 'should do nothing when attribute value is the same', () => {
+		root.insertChildren( 0, new Text( 'x', { foo: true } ) );
+
+		expect( () => {
+			doc.applyOperation( wrapInDelta(
+				new AttributeOperation(
+					new Range( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ),
+					'foo',
+					true,
+					true,
+					doc.version
+				)
+			) );
+		} ).to.not.throw();
+	} );
+
 	describe( 'toJSON', () => {
 		it( 'should create proper serialized object', () => {
 			const range = new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) );

+ 17 - 1
packages/ckeditor5-engine/tests/model/operation/detachoperation.js

@@ -5,7 +5,7 @@
 
 import Document from '../../../src/model/document';
 import DetachOperation from '../../../src/model/operation/detachoperation';
-import { wrapInDelta } from '../../../tests/model/_utils/utils';
+import { jsonParseStringify, wrapInDelta } from '../../../tests/model/_utils/utils';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import Position from '../../../src/model/position';
 
@@ -52,4 +52,20 @@ describe( 'DetachOperation', () => {
 
 		expect( op.isDocumentOperation ).to.false;
 	} );
+
+	describe( 'toJSON', () => {
+		it( 'should create proper json object', () => {
+			const position = Position.createBefore( element );
+			const op = new DetachOperation( position, 1, doc.version );
+
+			const serialized = jsonParseStringify( op );
+
+			expect( serialized ).to.deep.equal( {
+				__className: 'engine.model.operation.DetachOperation',
+				baseVersion: 0,
+				sourcePosition: jsonParseStringify( position ),
+				howMany: 1
+			} );
+		} );
+	} );
 } );

+ 26 - 0
packages/ckeditor5-engine/tests/model/operation/operationfactory.js

@@ -0,0 +1,26 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import Document from '../../../src/model/document';
+import NoOperation from '../../../src/model/operation/nooperation';
+import OperationFactory from '../../../src/model/operation/operationfactory';
+
+describe( 'OperationFactory', () => {
+	let doc;
+
+	beforeEach( () => {
+		doc = new Document();
+	} );
+
+	it( 'should create operation from JSON', () => {
+		const operation = OperationFactory.fromJSON( {
+			__className: 'engine.model.operation.NoOperation',
+			baseVersion: 0
+		}, doc );
+
+		expect( operation ).to.instanceof( NoOperation );
+		expect( operation.baseVersion ).to.equal( 0 );
+	} );
+} );

+ 8 - 0
packages/ckeditor5-engine/tests/model/operation/renameoperation.js

@@ -92,6 +92,14 @@ describe( 'RenameOperation', () => {
 		expect( clone.newName ).to.equal( newName );
 	} );
 
+	it( 'should do nothing when new name is the same as previous', () => {
+		const op = new RenameOperation( position, oldName, oldName, doc.version );
+
+		expect( () => {
+			doc.applyOperation( wrapInDelta( op ) );
+		} ).to.not.throw();
+	} );
+
 	describe( 'isDocumentOperation', () => {
 		it( 'should be true when target item is in the document', () => {
 			const op = new RenameOperation( position, oldName, newName, doc.version );

+ 33 - 56
packages/ckeditor5-engine/tests/model/writer.js → packages/ckeditor5-engine/tests/model/operation/utils.js

@@ -3,21 +3,21 @@
  * For licensing, see LICENSE.md.
  */
 
-import Document from '../../src/model/document';
-import DocumentFragment from '../../src/model/documentfragment';
-import Element from '../../src/model/element';
-import Text from '../../src/model/text';
-import TextProxy from '../../src/model/textproxy';
-import Position from '../../src/model/position';
-import Range from '../../src/model/range';
-import writer from '../../src/model/writer';
-import { getData } from '../../src/dev-utils/model';
+import Document from '../../../src/model/document';
+import DocumentFragment from '../../../src/model/documentfragment';
+import Element from '../../../src/model/element';
+import Text from '../../../src/model/text';
+import TextProxy from '../../../src/model/textproxy';
+import Position from '../../../src/model/position';
+import Range from '../../../src/model/range';
+import * as utils from '../../../src/model/operation/utils';
+import { getData } from '../../../src/dev-utils/model';
 
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 let doc, root;
 
-describe( 'writer', () => {
+describe( 'writer utils', () => {
 	beforeEach( () => {
 		doc = new Document();
 		doc.schema.allow( { name: '$text', inside: '$root' } );
@@ -38,19 +38,19 @@ describe( 'writer', () => {
 
 	describe( 'insert', () => {
 		it( 'should insert nodes between nodes', () => {
-			writer.insert( Position.createAt( root, 3 ), [ 'xxx', new Element( 'p' ) ] );
+			utils._insert( Position.createAt( root, 3 ), [ 'xxx', new Element( 'p' ) ] );
 
 			expectData( 'fooxxx<p></p><$text bold="true">bar</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should split text node if nodes at inserted at offset inside text node', () => {
-			writer.insert( Position.createAt( root, 5 ), new Element( 'p' ) );
+			utils._insert( Position.createAt( root, 5 ), new Element( 'p' ) );
 
 			expectData( 'foo<$text bold="true">ba</$text><p></p><$text bold="true">r</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should merge text nodes if possible', () => {
-			writer.insert( Position.createAt( root, 3 ), new Text( 'xxx', { bold: true } ) );
+			utils._insert( Position.createAt( root, 3 ), new Text( 'xxx', { bold: true } ) );
 
 			expectData( 'foo<$text bold="true">xxxbar</$text><image src="img.jpg"></image>xyz' );
 		} );
@@ -59,21 +59,21 @@ describe( 'writer', () => {
 	describe( 'remove', () => {
 		it( 'should remove nodes in given range', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.remove( range );
+			utils._remove( range );
 
 			expectData( 'foo<image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should split text node if range starts or ends inside text node', () => {
 			const range = Range.createFromParentsAndOffsets( root, 1, root, 5 );
-			writer.remove( range );
+			utils._remove( range );
 
 			expectData( 'f<$text bold="true">r</$text><image src="img.jpg"></image>xyz' );
 		} );
 
 		it( 'should merge text nodes if possible', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 7 );
-			writer.remove( range );
+			utils._remove( range );
 
 			expectData( 'fooxyz' );
 			expect( root.childCount ).to.equal( 1 );
@@ -81,89 +81,66 @@ describe( 'writer', () => {
 
 		it( 'should throw if given range is not flat', () => {
 			expect( () => {
-				writer.remove( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ) );
-			} ).to.throw( CKEditorError, /model-writer-remove-range-not-flat/ );
+				utils._remove( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ) );
+			} ).to.throw( CKEditorError, /operation-utils-remove-range-not-flat/ );
 		} );
 	} );
 
 	describe( 'move', () => {
 		it( 'should move a range of nodes', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.move( range, Position.createAt( root, 0 ) );
+			utils._move( range, Position.createAt( root, 0 ) );
 
 			expectData( '<$text bold="true">bar</$text>foo<image src="img.jpg"></image>xyz' );
 		} );
 
-		it( 'should use remove and insert methods', () => {
-			sinon.spy( writer, 'remove' );
-			sinon.spy( writer, 'insert' );
-
-			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			const position = Position.createAt( root, 0 );
-			writer.move( range, position );
-
-			expect( writer.remove.calledWithExactly( range ) ).to.be.true;
-			expect( writer.insert.calledWith( position ) ).to.be.true;
-		} );
-
 		it( 'should correctly move if target position is in same element as moved range, but after range', () => {
 			const range = Range.createFromParentsAndOffsets( root, 3, root, 6 );
-			writer.move( range, Position.createAt( root, 10 ) );
+			utils._move( range, Position.createAt( root, 10 ) );
 
 			expectData( 'foo<image src="img.jpg"></image>xyz<$text bold="true">bar</$text>' );
 		} );
 
 		it( 'should throw if given range is not flat', () => {
 			expect( () => {
-				writer.move( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ), null );
-			} ).to.throw( CKEditorError, /model-writer-move-range-not-flat/ );
+				utils._move( new Range( new Position( root, [ 0 ] ), new Position( root, [ 1, 2 ] ) ), null );
+			} ).to.throw( CKEditorError, /operation-utils-move-range-not-flat/ );
 		} );
 	} );
 
 	describe( 'setAttribute', () => {
 		it( 'should set attribute on given range of nodes', () => {
 			const range = Range.createFromParentsAndOffsets( root, 6, root, 8 );
-			writer.setAttribute( range, 'newAttr', true );
+			utils._setAttribute( range, 'newAttr', true );
 
 			expectData( 'foo<$text bold="true">bar</$text><image newAttr="true" src="img.jpg"></image><$text newAttr="true">x</$text>yz' );
 		} );
 
 		it( 'should remove attribute if null was passed as a value', () => {
 			const range = Range.createFromParentsAndOffsets( root, 6, root, 7 );
-			writer.setAttribute( range, 'src', null );
+			utils._setAttribute( range, 'src', null );
 
 			expectData( 'foo<$text bold="true">bar</$text><image></image>xyz' );
 		} );
 
 		it( 'should merge nodes if possible', () => {
 			const range = Range.createFromParentsAndOffsets( root, 0, root, 3 );
-			writer.setAttribute( range, 'bold', true );
+			utils._setAttribute( range, 'bold', true );
 
 			expectData( '<$text bold="true">foobar</$text><image src="img.jpg"></image>xyz' );
 		} );
 	} );
-
-	describe( 'removeAttribute', () => {
-		it( 'should use setAttribute', () => {
-			sinon.spy( writer, 'setAttribute' );
-
-			const range = Range.createFromParentsAndOffsets( root, 6, root, 7 );
-			writer.removeAttribute( range, 'src' );
-
-			expect( writer.setAttribute.calledWithExactly( range, 'src', null ) ).to.be.true;
-		} );
-	} );
 } );
 
 describe( 'normalizeNodes', () => {
 	it( 'should change single object into an array', () => {
 		const p = new Element( 'p' );
 
-		expect( writer.normalizeNodes( p ) ).to.deep.equal( [ p ] );
+		expect( utils._normalizeNodes( p ) ).to.deep.equal( [ p ] );
 	} );
 
 	it( 'should change strings to text nodes', () => {
-		const text = writer.normalizeNodes( 'abc' )[ 0 ];
+		const text = utils._normalizeNodes( 'abc' )[ 0 ];
 
 		expect( text ).to.be.instanceof( Text );
 		expect( text.data ).to.equal( 'abc' );
@@ -173,7 +150,7 @@ describe( 'normalizeNodes', () => {
 		const textNode = new Text( 'abc' );
 		const textProxy = new TextProxy( textNode, 1, 1 );
 
-		const text = writer.normalizeNodes( textProxy )[ 0 ];
+		const text = utils._normalizeNodes( textProxy )[ 0 ];
 
 		expect( text ).to.be.instanceof( Text );
 		expect( text.data ).to.equal( 'b' );
@@ -182,11 +159,11 @@ describe( 'normalizeNodes', () => {
 	it( 'should not change elements', () => {
 		const p = new Element( 'p' );
 
-		expect( writer.normalizeNodes( p )[ 0 ] ).to.equal( p );
+		expect( utils._normalizeNodes( p )[ 0 ] ).to.equal( p );
 	} );
 
 	it( 'should omit unrecognized objects', () => {
-		expect( writer.normalizeNodes( 1 ) ).to.deep.equal( [] );
+		expect( utils._normalizeNodes( 1 ) ).to.deep.equal( [] );
 	} );
 
 	it( 'should accept arrays', () => {
@@ -194,7 +171,7 @@ describe( 'normalizeNodes', () => {
 		const image = new Element( 'image' );
 		const nodes = [ 'abc', text, image, 1, 'xyz' ];
 
-		const normalized = writer.normalizeNodes( nodes );
+		const normalized = utils._normalizeNodes( nodes );
 
 		expect( normalized[ 0 ] ).to.be.instanceof( Text );
 		expect( normalized[ 1 ] ).to.equal( text );
@@ -203,7 +180,7 @@ describe( 'normalizeNodes', () => {
 	} );
 
 	it( 'should merge text nodes if mergeTextNodes flag is set to true', () => {
-		const normalized = writer.normalizeNodes( [ 'foo', 'bar' ], true );
+		const normalized = utils._normalizeNodes( [ 'foo', 'bar' ], true );
 
 		expect( normalized.length ).to.equal( 1 );
 		expect( normalized[ 0 ].data ).to.equal( 'foobar' );
@@ -216,7 +193,7 @@ describe( 'normalizeNodes', () => {
 			'xyz'
 		];
 
-		const normalized = writer.normalizeNodes( nodes, true );
+		const normalized = utils._normalizeNodes( nodes, true );
 
 		expect( normalized[ 0 ] ).to.be.instanceof( Text );
 		expect( normalized[ 0 ].getAttribute( 'bold' ) ).to.be.true;