8
0
Просмотр исходного кода

Merge branch 't/1464' into t/ot-followups

# Conflicts:
#	src/model/operation/transform.js
#	tests/dev-utils/enableenginedebug.js
#	tests/model/operation/transform.js
Szymon Cofalik 7 лет назад
Родитель
Сommit
5422478f28

+ 0 - 19
packages/ckeditor5-engine/src/dev-utils/enableenginedebug.js

@@ -25,7 +25,6 @@ import MoveOperation from '../model/operation/moveoperation';
 import NoOperation from '../model/operation/nooperation';
 import RenameOperation from '../model/operation/renameoperation';
 import RootAttributeOperation from '../model/operation/rootattributeoperation';
-import transform from '../model/operation/transform';
 import Model from '../model/model';
 import ModelDocument from '../model/document';
 import ModelDocumentFragment from '../model/documentfragment';
@@ -349,24 +348,6 @@ function enableLoggingTools() {
 			`"${ this.key }": ${ JSON.stringify( this.oldValue ) } -> ${ JSON.stringify( this.newValue ) }, ${ this.root.rootName }`;
 	} );
 
-	const _transformTransform = transform.transform;
-
-	sandbox.mock( transform, 'transform', function( a, b, context ) {
-		let results;
-
-		try {
-			results = _transformTransform( a, b, context );
-		} catch ( e ) {
-			logger.error( 'Error during operation transformation!' );
-			logger.error( a.toString() + ( context.isStrong ? ' (important)' : '' ) );
-			logger.error( b.toString() + ( context.isStrong ? '' : ' (important)' ) );
-
-			throw e;
-		}
-
-		return results;
-	} );
-
 	sandbox.mock( ViewText.prototype, 'toString', function() {
 		return `#${ this.data }`;
 	} );

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

@@ -11,7 +11,9 @@ import UnwrapOperation from './unwrapoperation';
 import NoOperation from './nooperation';
 import Range from '../range';
 import Position from '../position';
+
 import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays';
+import log from '@ckeditor/ckeditor5-utils/src/log';
 
 const transformations = new Map();
 
@@ -51,7 +53,20 @@ function updateBaseVersions( operations, baseVersion ) {
 function transform( a, b, context = {} ) {
 	const transformationFunction = getTransformation( a, b );
 
-	return transformationFunction( a.clone(), b, context );
+	try {
+		return transformationFunction( a.clone(), b, context );
+	} catch ( e ) {
+		log.error( 'Error during operation transformation!' );
+		log.error( 'Transformed operation', a );
+		log.error( 'Operation transformed by', b );
+		log.error( 'context.aIsStrong', context.aIsStrong );
+		log.error( 'context.wasUndone( a )', context.wasUndone( a ) );
+		log.error( 'context.wasUndone( b )', context.wasUndone( b ) );
+		log.error( 'context.getRelation( a, b )', context.getRelation( a, b ) );
+		log.error( 'context.getRelation( b, a )', context.getRelation( b, a ) );
+
+		throw e;
+	}
 }
 
 function transformSets( operationsA, operationsB, options ) {
@@ -115,6 +130,8 @@ function transformSets( operationsA, operationsB, options ) {
 					if ( options.useContext ) {
 						updateOriginalOperation( context, opA, newOpA );
 						updateOriginalOperation( context, opB, newOpB );
+
+						updateRelations( context, opA, opB );
 					}
 
 					opsA.splice( k, 1, ...newOpA );
@@ -996,7 +1013,7 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 
 	// Assign `context.aIsStrong` to a different variable, because the value may change during execution of
 	// this algorithm and we do not want to override original `context.aIsStrong` that will be used in later transformations.
-	let aIsStrong = context.aIsStrong;
+	let aIsStrong = context.aIsStrong || context.getRelation( a, b ) == 'insertBefore';
 
 	if ( context.abRelation == 'insertBefore' ) {
 		aIsStrong = true;

+ 0 - 34
packages/ckeditor5-engine/tests/dev-utils/enableenginedebug.js

@@ -21,7 +21,6 @@ import MoveOperation from '../../src/model/operation/moveoperation';
 import NoOperation from '../../src/model/operation/nooperation';
 import RenameOperation from '../../src/model/operation/renameoperation';
 import RootAttributeOperation from '../../src/model/operation/rootattributeoperation';
-import transform from '../../src/model/operation/transform';
 import Model from '../../src/model/model';
 import ModelDocumentFragment from '../../src/model/documentfragment';
 
@@ -667,39 +666,6 @@ describe( 'debug tools', () => {
 		} );
 	} );
 
-	describe( 'should provide error logging for transformation', () => {
-		let model, document, root, otherRoot;
-
-		beforeEach( () => {
-			model = new Model();
-			document = model.document;
-			root = document.createRoot();
-			otherRoot = document.createRoot( 'other', 'other' );
-		} );
-
-		it.skip( 'with more important operation A', () => {
-			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
-			const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
-
-			expect( () => {
-				transform.transform( opA, opB, { isStrong: true } );
-			} ).to.throw( Error );
-			expect( error.calledWith( opA.toString() + ' (important)' ) ).to.be.true;
-			expect( error.calledWith( opB.toString() ) ).to.be.true;
-		} );
-
-		it.skip( 'with more important operation B', () => {
-			const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 );
-			const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 );
-
-			testUtils.sinon.stub( transform, 'transform' ).throws( new Error() );
-
-			expect( () => transform.transform( opA, opB, { isStrong: true } ) ).to.throw( Error );
-			expect( error.calledWith( opA.toString() ) ).to.be.true;
-			expect( error.calledWith( opB.toString() + ' (important)' ) ).to.be.true;
-		} );
-	} );
-
 	function expectLog( expectedLogMsg ) {
 		expect( log.calledWithExactly( expectedLogMsg ) ).to.be.true;
 		log.resetHistory();

+ 24 - 3
packages/ckeditor5-engine/tests/model/operation/transform.js

@@ -19,6 +19,8 @@ import MoveOperation from '../../../src/model/operation/moveoperation';
 import RenameOperation from '../../../src/model/operation/renameoperation';
 import NoOperation from '../../../src/model/operation/nooperation';
 
+import log from '@ckeditor/ckeditor5-utils/src/log';
+
 describe( 'transform', () => {
 	let model, doc, root, op, nodeA, nodeB, expected;
 
@@ -53,11 +55,30 @@ describe( 'transform', () => {
 	}
 
 	const contextIsStrong = {
-		aIsStrong: true,
-		wasUndone: () => false,
-		getRelation: () => false
+		aIsStrong: true
 	};
 
+	it( 'error logging', () => {
+		const spy = sinon.spy( log, 'error' );
+
+		const nodeA = new Node();
+		const nodeB = new Node();
+
+		const position = new Position( root, [ 0 ] );
+
+		const a = new InsertOperation( position, [ nodeA ], 0 );
+		const b = new InsertOperation( position, [ nodeB ], 0 );
+
+		// Modify an operation so it will throw an error.
+		a.position = null;
+
+		expect( () => {
+			transform.transform( a, b );
+		} ).to.throw();
+
+		sinon.assert.called( spy );
+	} );
+
 	describe( 'InsertOperation', () => {
 		let nodeC, nodeD, position;