Selaa lähdekoodia

Merge branch t/ckeditor5-engine/858

Internal: Update API usage after merge t/858 on engine.
Piotr Jasiun 8 vuotta sitten
vanhempi
sitoutus
764c89ed7d

+ 27 - 22
packages/ckeditor5-undo/src/basecommand.js

@@ -133,29 +133,34 @@ export default class BaseCommand extends Command {
 		// We will process each delta from `batchToUndo`, in reverse order. If there were deltas A, B and C in undone batch,
 		// we need to revert them in reverse order, so first C' (reversed C), then B', then A'.
 		for ( const deltaToUndo of deltasToUndo ) {
-			// Keep in mind that transformation algorithms return arrays. That's because the transformation might result in multiple
-			// deltas, so we need arrays to handle them. To simplify algorithms, it is better to always operate on arrays.
-			const nextBaseVersion = deltaToUndo.baseVersion + deltaToUndo.operations.length;
-
-			// Reverse delta from the history.
-			const historyDeltas = Array.from( document.history.getDeltas( nextBaseVersion ) );
-			const transformedSets = document.transformDeltas( [ deltaToUndo.getReversed() ], historyDeltas, true );
-			const reversedDeltas = transformedSets.deltasA;
-
-			// After reversed delta has been transformed by all history deltas, apply it.
-			for ( const delta of reversedDeltas ) {
-				// Fix base version.
-				delta.baseVersion = document.version;
-
-				// Before applying, add the delta to the `undoingBatch`.
-				undoingBatch.addDelta( delta );
-
-				// Now, apply all operations of the delta.
-				for ( const operation of delta.operations ) {
-					document.applyOperation( operation );
+			// For now let's skip deltas with operation applied on detached document.
+			// We assumed that there is no deltas with mixed (document and document fragment) operations
+			// so we can skip entire delta.
+			if ( deltaToUndo.operations.some( op => op.isDocumentOperation ) ) {
+				// Keep in mind that transformation algorithms return arrays. That's because the transformation might result in multiple
+				// deltas, so we need arrays to handle them. To simplify algorithms, it is better to always operate on arrays.
+				const nextBaseVersion = deltaToUndo.baseVersion + deltaToUndo.operations.length;
+
+				// Reverse delta from the history.
+				const historyDeltas = Array.from( document.history.getDeltas( nextBaseVersion ) );
+				const transformedSets = document.transformDeltas( [ deltaToUndo.getReversed() ], historyDeltas, true );
+				const reversedDeltas = transformedSets.deltasA;
+
+				// After reversed delta has been transformed by all history deltas, apply it.
+				for ( const delta of reversedDeltas ) {
+					// Fix base version.
+					delta.baseVersion = document.version;
+
+					// Before applying, add the delta to the `undoingBatch`.
+					undoingBatch.addDelta( delta );
+
+					// Now, apply all operations of the delta.
+					for ( const operation of delta.operations ) {
+						document.applyOperation( operation );
+					}
+
+					document.history.setDeltaAsUndone( deltaToUndo, delta );
 				}
-
-				document.history.setDeltaAsUndone( deltaToUndo, delta );
 			}
 		}
 

+ 3 - 3
packages/ckeditor5-undo/tests/redocommand.js

@@ -52,7 +52,7 @@ describe( 'RedoCommand', () => {
 				editor.document.selection.setRanges( [ r( 0, 0 ) ] );
 				batch0 = doc.batch();
 				undo.addBatch( batch0 );
-				batch0.insert( p( 0 ), 'foobar' );
+				batch0.insertText( 'foobar', p( 0 ) );
 
 				/*
 				 [root]
@@ -67,7 +67,7 @@ describe( 'RedoCommand', () => {
 				editor.document.selection.setRanges( [ r( 2, 4 ) ], true );
 				batch1 = doc.batch();
 				undo.addBatch( batch1 );
-				batch1.setAttribute( r( 2, 4 ), 'key', 'value' );
+				batch1.setAttribute( 'key', 'value', r( 2, 4 ) );
 
 				/*
 				 [root]
@@ -233,7 +233,7 @@ describe( 'RedoCommand', () => {
 				undo.execute();
 
 				// Append "xx" at the beginning. Now it is "xxfoob".
-				doc.batch().insert( p( 0 ), 'xx' );
+				doc.batch().insertText( 'xx', p( 0 ) );
 
 				// Redo setting attribute on "ob". Now it is "xxfoOB".
 				redo.execute();

+ 18 - 3
packages/ckeditor5-undo/tests/undocommand.js

@@ -41,7 +41,7 @@ describe( 'UndoCommand', () => {
 				editor.document.selection.setRanges( [ r( 0, 0 ) ] );
 				batch0 = doc.batch();
 				undo.addBatch( batch0 );
-				batch0.insert( p( 0 ), 'foobar' );
+				batch0.insertText( 'foobar', p( 0 ) );
 
 				/*
 				 [root]
@@ -56,7 +56,7 @@ describe( 'UndoCommand', () => {
 				editor.document.selection.setRanges( [ r( 2, 4 ) ], true );
 				batch1 = doc.batch();
 				undo.addBatch( batch1 );
-				batch1.setAttribute( r( 2, 4 ), 'key', 'value' );
+				batch1.setAttribute( 'key', 'value', r( 2, 4 ) );
 
 				/*
 				 [root]
@@ -256,6 +256,21 @@ describe( 'UndoCommand', () => {
 				expect( editor.document.selection.getFirstRange().isEqual( r( 0, 0 ) ) ).to.be.true;
 				expect( editor.document.selection.isBackward ).to.be.false;
 			} );
+
+			it( 'should omit deltas with non-document operations', () => {
+				const batch = doc.batch();
+				const element = batch.createElement( 'p' );
+
+				undo.addBatch( batch );
+
+				batch.setAttribute( 'foo', 'bar', element );
+				batch.setAttribute( 'foo', 'bar', root );
+
+				undo.execute();
+
+				expect( element.getAttribute( 'foo' ) ).to.equal( 'bar' );
+				expect( root.getAttribute( 'foo' ) ).to.not.equal( 'bar' );
+			} );
 		} );
 
 		it( 'merges touching ranges when restoring selection', () => {
@@ -276,7 +291,7 @@ describe( 'UndoCommand', () => {
 			editor.document.selection.setRanges( [ r( 1, 4 ) ] );
 			const batch0 = doc.batch();
 			undo.addBatch( batch0 );
-			batch0.setAttribute( r( 1, 4 ), 'uppercase', true );
+			batch0.setAttribute( 'uppercase', true, r( 1, 4 ) );
 			expect( getCaseText( root ) ).to.equal( 'aBCDef' );
 
 			editor.document.selection.setRanges( [ r( 3, 4 ) ] );

+ 20 - 19
packages/ckeditor5-undo/tests/undoengine-integration.js

@@ -9,7 +9,6 @@ import ClassicEditor from '@ckeditor/ckeditor5-editor-classic/src/classiceditor'
 
 import Range from '@ckeditor/ckeditor5-engine/src/model/range';
 import Position from '@ckeditor/ckeditor5-engine/src/model/position';
-import Element from '@ckeditor/ckeditor5-engine/src/model/element';
 import UndoEngine from '../src/undoengine';
 
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
@@ -71,7 +70,7 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().insert( doc.selection.getFirstPosition(), 'zzz' );
+				doc.batch().insertText( 'zzz', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>fozzz[]o</paragraph><paragraph>bar</paragraph>' );
 
@@ -85,16 +84,17 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch()
-					.insert( doc.selection.getFirstPosition(), 'zzz' )
-					.insert( new Position( root, [ 1, 0 ] ), 'xxx' );
+				const batch = doc.batch();
+
+				batch.insertText( 'zzz', doc.selection.getFirstPosition() );
+				batch.insertText( 'xxx', new Position( root, [ 1, 0 ] ) );
 			} );
 
 			output( '<paragraph>fozzz[]o</paragraph><paragraph>xxxbar</paragraph>' );
 
 			doc.enqueueChanges( () => {
 				setSelection( [ 1, 0 ], [ 1, 0 ] );
-				doc.batch().insert( doc.selection.getFirstPosition(), 'yyy' );
+				doc.batch().insertText( 'yyy', doc.selection.getFirstPosition() );
 			} );
 
 			output( '<paragraph>fozzzo</paragraph><paragraph>yyy[]xxxbar</paragraph>' );
@@ -112,13 +112,13 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().insert( doc.selection.getFirstPosition(), 'zzz' );
+				doc.batch().insertText( 'zzz', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>fozzz[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
 				setSelection( [ 1, 0 ], [ 1, 0 ] );
-				doc.batch().insert( doc.selection.getFirstPosition(), 'yyy' );
+				doc.batch().insertText( 'yyy', doc.selection.getFirstPosition() );
 			} );
 
 			output( '<paragraph>fozzzo</paragraph><paragraph>yyy[]bar</paragraph>' );
@@ -128,7 +128,7 @@ describe( 'UndoEngine integration', () => {
 
 			doc.enqueueChanges( () => {
 				setSelection( [ 0, 0 ], [ 0, 0 ] );
-				doc.batch().insert( doc.selection.getFirstPosition(), 'xxx' );
+				doc.batch().insertText( 'xxx', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>xxx[]fozzzo</paragraph><paragraph>bar</paragraph>' );
 
@@ -170,7 +170,7 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().insert( doc.selection.getFirstPosition(), 'zzz' );
+				doc.batch().insertText( 'zzz', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>fozzz[]o</paragraph><paragraph>bar</paragraph>' );
 
@@ -193,7 +193,7 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[]o</paragraph><paragraph>bar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().insert( doc.selection.getFirstPosition(), 'zzz' );
+				doc.batch().insertText( 'zzz', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>fozzz[]o</paragraph><paragraph>bar</paragraph>' );
 
@@ -229,7 +229,7 @@ describe( 'UndoEngine integration', () => {
 			input( '' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().insert( doc.selection.getFirstPosition(), new Element( 'heading1' ) );
+				doc.batch().insertElement( 'heading1', doc.selection.getFirstPosition() );
 			} );
 			output( '<heading1>[]</heading1>' );
 
@@ -247,10 +247,11 @@ describe( 'UndoEngine integration', () => {
 			const pos = new Position( root, [ 0 ] );
 
 			doc.enqueueChanges( () => {
-				doc.batch()
-					.remove( p )
-					.insert( pos, new Element( 'heading1' ) )
-					.insert( pos.getShiftedBy( 1 ), new Element( 'heading2' ) );
+				const batch = doc.batch();
+
+				batch.remove( p );
+				batch.insertElement( 'heading1', pos );
+				batch.insertElement( 'heading2', pos.getShiftedBy( 1 ) );
 			} );
 
 			output( '<heading1>[]</heading1><heading2></heading2>' );
@@ -315,13 +316,13 @@ describe( 'UndoEngine integration', () => {
 			input( '<paragraph>fo[ob]ar</paragraph>' );
 
 			doc.enqueueChanges( () => {
-				doc.batch().setAttribute( doc.selection.getFirstRange(), 'bold', true );
+				doc.batch().setAttribute( 'bold', true, doc.selection.getFirstRange() );
 			} );
 			output( '<paragraph>fo[<$text bold="true">ob</$text>]ar</paragraph>' );
 
 			doc.enqueueChanges( () => {
 				setSelection( [ 0, 3 ], [ 0, 3 ] );
-				doc.batch().insert( doc.selection.getFirstPosition(), 'zzz' );
+				doc.batch().insertText( 'zzz', doc.selection.getFirstPosition() );
 			} );
 			output( '<paragraph>fo<$text bold="true">o</$text>zzz<$text bold="true">[]b</$text>ar</paragraph>' );
 			expect( doc.selection.getAttribute( 'bold' ) ).to.true;
@@ -959,7 +960,7 @@ describe( 'UndoEngine integration', () => {
 
 			doc.enqueueChanges( () => {
 				doc.batch().remove( p );
-				doc.batch().setAttribute( p, 'bold', true );
+				doc.batch().setAttribute( 'bold', true, p );
 			} );
 
 			editor.execute( 'undo' );

+ 4 - 4
packages/ckeditor5-undo/tests/undoengine.js

@@ -4,7 +4,6 @@
  */
 
 import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';
-import Position from '@ckeditor/ckeditor5-engine/src/model/position';
 import UndoEngine from '../src/undoengine';
 
 describe( 'UndoEngine', () => {
@@ -38,7 +37,7 @@ describe( 'UndoEngine', () => {
 			expect( undo._undoCommand.addBatch.called ).to.be.false;
 			expect( undo._redoCommand.clearStack.called ).to.be.false;
 
-			batch.insert( new Position( root, [ 0 ] ), 'foobar' );
+			batch.insertText( 'foobar', root );
 
 			expect( undo._undoCommand.addBatch.calledOnce ).to.be.true;
 			expect( undo._redoCommand.clearStack.calledOnce ).to.be.true;
@@ -47,7 +46,8 @@ describe( 'UndoEngine', () => {
 		it( 'should add each batch only once', () => {
 			sinon.spy( undo._undoCommand, 'addBatch' );
 
-			batch.insert( new Position( root, [ 0 ] ), 'foobar' ).insert( new Position( root, [ 0 ] ), 'foobar' );
+			batch.insertText( 'foobar', root );
+			batch.insertText( 'foobar', root );
 
 			expect( undo._undoCommand.addBatch.calledOnce ).to.be.true;
 		} );
@@ -58,7 +58,7 @@ describe( 'UndoEngine', () => {
 
 			undo._redoCommand._createdBatches.add( batch );
 
-			batch.insert( new Position( root, [ 0 ] ), 'foobar' );
+			batch.insertText( 'foobar', root );
 
 			expect( undo._undoCommand.addBatch.calledOnce ).to.be.true;
 			expect( undo._redoCommand.clearStack.called ).to.be.false;