Merge pull request #33 from ckeditor/t/32

Fixed: Autoformatting will not be triggered if the batch with changes is `transparent` (e.g. it represents other user's changes).
ckeditor · Jul 11, 2017 · f1131bc · f1131bc
2 parents 4a84cc1 + 8b928d6
commit f1131bc
Show file tree

Hide file tree

Showing 4 changed files with 66 additions and 9 deletions.
diff --git a/src/blockautoformatengine.js b/src/blockautoformatengine.js
@@ -69,7 +69,11 @@ export default class BlockAutoformatEngine {
 			};
 		}
 
-		editor.document.on( 'change', ( event, type, changes ) => {
+		editor.document.on( 'change', ( event, type, changes, batch ) => {
+			if ( batch.type == 'transparent' ) {
+				return;
+			}
+
 			if ( type != 'insert' ) {
 				return;
 			}
@@ -98,15 +102,15 @@ export default class BlockAutoformatEngine {
 
 			editor.document.enqueueChanges( () => {
 				// Create new batch to separate typing batch from the Autoformat changes.
-				const batch = editor.document.batch();
+				const fixBatch = editor.document.batch();
 
 				// Matched range.
 				const range = Range.createFromParentsAndOffsets( textNode.parent, 0, textNode.parent, match[ 0 ].length );
 
 				// Remove matched text.
-				batch.remove( range );
+				fixBatch.remove( range );
 
-				callback( { batch, match } );
+				callback( { fixBatch, match } );
 			} );
 		} );
 	}

diff --git a/src/inlineautoformatengine.js b/src/inlineautoformatengine.js
@@ -136,7 +136,11 @@ export default class InlineAutoformatEngine {
 			}
 		} );
 
-		editor.document.on( 'change', ( evt, type ) => {
+		editor.document.on( 'change', ( evt, type, changes, batch ) => {
+			if ( batch.type == 'transparent' ) {
+				return;
+			}
+
 			if ( type !== 'insert' ) {
 				return;
 			}
@@ -182,17 +186,18 @@ export default class InlineAutoformatEngine {
 				return;
 			}
 
-			const batch = editor.document.batch();
-
 			editor.document.enqueueChanges( () => {
+				// Create new batch to separate typing batch from the Autoformat changes.
+				const fixBatch = editor.document.batch();
+
 				const validRanges = editor.document.schema.getValidRanges( rangesToFormat, command );
 
 				// Apply format.
-				formatCallback( batch, validRanges );
+				formatCallback( fixBatch, validRanges );
 
 				// Remove delimiters.
 				for ( const range of rangesToRemove ) {
-					batch.remove( range );
+					fixBatch.remove( range );
 				}
 			} );
 		} );

diff --git a/tests/blockautoformatengine.js b/tests/blockautoformatengine.js
@@ -41,6 +41,19 @@ describe( 'BlockAutoformatEngine', () => {
 			sinon.assert.calledOnce( spy );
 		} );
 
+		it( 'should not run a command when changes are in transparent batch', () => {
+			const spy = testUtils.sinon.spy();
+			editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
+			new BlockAutoformatEngine( editor, /^[*]\s$/, 'testCommand' ); // eslint-disable-line no-new
+
+			setData( doc, '<paragraph>*[]</paragraph>' );
+			doc.enqueueChanges( () => {
+				doc.batch( 'transparent' ).insert( doc.selection.getFirstPosition(), ' ' );
+			} );
+
+			sinon.assert.notCalled( spy );
+		} );
+
 		it( 'should remove found pattern', () => {
 			const spy = testUtils.sinon.spy();
 			editor.commands.add( 'testCommand', new TestCommand( editor, spy ) );
@@ -69,6 +82,18 @@ describe( 'BlockAutoformatEngine', () => {
 			sinon.assert.calledOnce( spy );
 		} );
 
+		it( 'should not run a callback when changes are in transparent batch', () => {
+			const spy = testUtils.sinon.spy();
+			new BlockAutoformatEngine( editor, /^[*]\s$/, spy ); // eslint-disable-line no-new
+
+			setData( doc, '<paragraph>*[]</paragraph>' );
+			doc.enqueueChanges( () => {
+				doc.batch( 'transparent' ).insert( doc.selection.getFirstPosition(), ' ' );
+			} );
+
+			sinon.assert.notCalled( spy );
+		} );
+
 		it( 'should ignore other delta operations', () => {
 			const spy = testUtils.sinon.spy();
 			new BlockAutoformatEngine( editor, /^[*]\s/, spy ); // eslint-disable-line no-new

diff --git a/tests/inlineautoformatengine.js b/tests/inlineautoformatengine.js
@@ -50,6 +50,17 @@ describe( 'InlineAutoformatEngine', () => {
 			expect( getData( doc ) ).to.equal( '<paragraph><$text testAttribute="true">foobar</$text>[]</paragraph>' );
 		} );
 
+		it( 'should not apply an attribute when changes are in transparent batch', () => {
+			new InlineAutoformatEngine( editor, /(\*)(.+?)(\*)/g, 'testAttribute' ); // eslint-disable-line no-new
+
+			setData( doc, '<paragraph>*foobar[]</paragraph>' );
+			doc.enqueueChanges( () => {
+				doc.batch( 'transparent' ).insert( doc.selection.getFirstPosition(), '*' );
+			} );
+
+			expect( getData( doc ) ).to.equal( '<paragraph>*foobar*[]</paragraph>' );
+		} );
+
 		it( 'should stop early if selection is not collapsed', () => {
 			new InlineAutoformatEngine( editor, /(\*)(.+?)\*/g, 'testAttribute' ); // eslint-disable-line no-new
 
@@ -63,6 +74,18 @@ describe( 'InlineAutoformatEngine', () => {
 	} );
 
 	describe( 'Callback', () => {
+		it( 'should not run a callback when changes are in transparent batch', () => {
+			const spy = testUtils.sinon.spy();
+			new InlineAutoformatEngine( editor, /(\*)(.+?)(\*)/g, spy ); // eslint-disable-line no-new
+
+			setData( doc, '<paragraph>*foobar[]</paragraph>' );
+			doc.enqueueChanges( () => {
+				doc.batch( 'transparent' ).insert( doc.selection.getFirstPosition(), '*' );
+			} );
+
+			sinon.assert.notCalled( spy );
+		} );
+
 		it( 'should stop when there are no format ranges returned from testCallback', () => {
 			const formatSpy = testUtils.sinon.spy();
 			const testStub = testUtils.sinon.stub().returns( {