Merge pull request #27 from ckeditor/t/26

Fix: Autoparagraphing empty roots will not be triggered if the change-to-fix was in a `transparent` batch. Closes #26.

src/paragraph.js

@@ -79,7 +79,13 @@ export default class Paragraph extends Plugin {
 		// Post-fixer which takes care of adding empty paragraph elements to empty roots.
 		// Besides fixing content on #changesDone we also need to handle #dataReady because
 		// if initial data is empty or setData() wasn't even called there will be no #change fired.
-		doc.on( 'change', ( evt, type, changes, batch ) => findEmptyRoots( doc, batch ) );
+		doc.on( 'change', ( evt, type, changes, batch ) => {
+			if ( batch.type == 'transparent' ) {
+				return;
+			}
+
+			findEmptyRoots( doc, batch );
+		} );
 		doc.on( 'changesDone', autoparagraphEmptyRoots, { priority: 'lowest' } );
 		editor.on( 'dataReady', () => {
 			findEmptyRoots( doc, doc.batch( 'transparent' ) );
@@ -285,10 +291,12 @@ function autoparagraphEmptyRoots() {
 		// If paragraph element is allowed in the root, create paragraph element.
 		if ( schema.check( query ) ) {
 			doc.enqueueChanges( () => {
+				// Remove root from `rootsToFix` here, before executing batch, to prevent infinite loops.
+				rootsToFix.delete( root );
+
+				// Fix empty root.
 				batch.insert( ModelPosition.createAt( root ), new ModelElement( 'paragraph' ) );
 			} );
 		}
 	}
-
-	rootsToFix.clear();
 }

tests/paragraph.js

@@ -20,15 +20,17 @@ import ModelDocumentFragment from '@ckeditor/ckeditor5-engine/src/model/document
 import ModelElement from '@ckeditor/ckeditor5-engine/src/model/element';
 import ModelText from '@ckeditor/ckeditor5-engine/src/model/text';
 import ModelPosition from '@ckeditor/ckeditor5-engine/src/model/position';
+import ModelRange from '@ckeditor/ckeditor5-engine/src/model/range';
 
 describe( 'Paragraph feature', () => {
-	let editor, doc;
+	let editor, doc, root;
 
 	beforeEach( () => {
 		return VirtualTestEditor.create( { plugins: [ Paragraph ] } )
 			.then( newEditor => {
 				editor = newEditor;
 				doc = editor.document;
+				root = doc.getRoot();
 			} );
 	} );
 
@@ -384,15 +386,17 @@ describe( 'Paragraph feature', () => {
 			expect( doc.getRoot().getChild( 0 ).is( 'paragraph' ) ).to.be.true;
 		} );
 
-		it( 'should fix roots if it becomes empty', () => {
+		it( 'should fix root if it becomes empty', () => {
 			editor.setData( '<p>Foobar</p>' );
 
 			// Since `setData` first removes all contents from editor and then sets content during same enqueue
-			// changes block, this checks whether fixing empty roots does not kick too early and does not
+			// changes block, the line below checks whether fixing empty roots does not kick too early and does not
 			// fix root if it is not needed.
 			expect( editor.getData() ).to.equal( '<p>Foobar</p>' );
 
-			editor.setData( '' );
+			doc.enqueueChanges( () => {
+				doc.batch().remove( ModelRange.createIn( root ) );
+			} );
 
 			expect( doc.getRoot().childCount ).to.equal( 1 );
 			expect( doc.getRoot().getChild( 0 ).is( 'paragraph' ) ).to.be.true;
@@ -416,7 +420,9 @@ describe( 'Paragraph feature', () => {
 				}
 			} );
 
-			editor.setData( '' );
+			doc.enqueueChanges( () => {
+				doc.batch().remove( ModelRange.createIn( root ) );
+			} );
 
 			expect( doc.getRoot().childCount ).to.equal( 1 );
 			expect( doc.getRoot().getChild( 0 ).name ).to.equal( 'heading' );
@@ -425,7 +431,18 @@ describe( 'Paragraph feature', () => {
 		it( 'should not fix root which does not allow paragraph', () => {
 			doc.schema.disallow( { name: 'paragraph', inside: '$root' } );
 
-			editor.setData( '' );
+			doc.enqueueChanges( () => {
+				doc.batch().remove( ModelRange.createIn( root ) );
+			} );
+			expect( editor.getData() ).to.equal( '' );
+		} );
+
+		it( 'should not fix root if change was in transparent batch', () => {
+			editor.setData( '<p>Foobar</p>' );
+
+			doc.enqueueChanges( () => {
+				doc.batch( 'transparent' ).remove( ModelRange.createIn( root ) );
+			} );
 
 			expect( editor.getData() ).to.equal( '' );
 		} );