Szymon Kupś пре 8 година
родитељ
комит
f5ab3ad3fa

+ 1 - 0
packages/ckeditor5-engine/src/view/document.js

@@ -109,6 +109,7 @@ export default class Document {
 	 * Performs post-fixer loops. Executes post-fixer callbacks as long as none of them has done any changes to the model.
 	 *
 	 * @protected
+	 * @param {module:engine/view/writer~Writer} writer
 	 */
 	_callPostFixers( writer ) {
 		let wasFixed = false;

+ 0 - 1
packages/ckeditor5-engine/src/view/view.js

@@ -318,7 +318,6 @@ export default class View {
 	 */
 	change( callback ) {
 		if ( this._renderingInProgress || this._postFixersInProgress ) {
-			// TODO: better description
 			/**
 			 * Thrown when there is an attempt to make changes to the view tree when it is in incorrect state. This may
 			 * cause some unexpected behaviour and inconsistency between the DOM and the view.

+ 33 - 0
packages/ckeditor5-engine/tests/view/document.js

@@ -58,4 +58,37 @@ describe( 'Document', () => {
 			expect( viewDocument.getRoot( 'not-existing' ) ).to.null;
 		} );
 	} );
+
+	describe( 'post fixers', () => {
+		it( 'should add a callback that is called on _callPostFixers', () => {
+			const spy1 = sinon.spy();
+			const spy2 = sinon.spy();
+			const writerMock = {};
+
+			viewDocument.registerPostFixer( spy1 );
+			viewDocument.registerPostFixer( spy2 );
+
+			sinon.assert.notCalled( spy1 );
+			sinon.assert.notCalled( spy2 );
+			viewDocument._callPostFixers( writerMock );
+			sinon.assert.calledOnce( spy1 );
+			sinon.assert.calledOnce( spy2 );
+			sinon.assert.calledWithExactly( spy1, writerMock );
+			sinon.assert.calledWithExactly( spy2, writerMock );
+		} );
+
+		it( 'should call post fixer until all returns false', () => {
+			let calls = 0;
+
+			const spy1 = sinon.spy( () => calls++ < 2 );
+			const spy2 = sinon.spy( () => calls++ < 2 );
+
+			viewDocument.registerPostFixer( spy1 );
+			viewDocument.registerPostFixer( spy2 );
+
+			viewDocument._callPostFixers();
+
+			expect( calls ).to.equal( 4 );
+		} );
+	} );
 } );

+ 16 - 1
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -74,12 +74,17 @@ describe( 'placeholder', () => {
 		it( 'use check function if one is provided', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
-			const spy = sinon.spy( () => false );
+			let result = true;
+			const spy = sinon.spy( () => result );
 
 			attachPlaceholder( view, element, 'foo bar baz', spy );
 
 			sinon.assert.called( spy );
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
+
+			result = false;
+			view.render();
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
@@ -190,5 +195,15 @@ describe( 'placeholder', () => {
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should not blow up when called on element without placeholder', () => {
+			setData( view, '<div></div><div>{another div}</div>' );
+			const element = viewRoot.getChild( 0 );
+
+			detachPlaceholder( view, element );
+
+			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
 	} );
 } );

+ 41 - 0
packages/ckeditor5-engine/tests/view/view/view.js

@@ -15,8 +15,10 @@ import global from '@ckeditor/ckeditor5-utils/src/dom/global';
 import ViewRange from '../../../src/view/range';
 import RootEditableElement from '../../../src/view/rooteditableelement';
 import ViewElement from '../../../src/view/element';
+import ViewPosition from '../../../src/view/position';
 import { isBlockFiller, BR_FILLER } from '../../../src/view/filler';
 import createElement from '@ckeditor/ckeditor5-utils/src/dom/createelement';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 describe( 'view', () => {
 	const DEFAULT_OBSERVERS_COUNT = 5;
@@ -443,6 +445,45 @@ describe( 'view', () => {
 	} );
 
 	describe( 'change()', () => {
+		it( 'should throw when someone tries to change view during rendering', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+			let renderingCalled = false;
+			view.attachDomRoot( domDiv );
+
+			view.change( writer => {
+				const p = writer.createContainerElement( 'p' );
+				const ui = writer.createUIElement( 'span', null, function( domDocument ) {
+					const element = this.toDomElement( domDocument );
+
+					expect( () => view.change( () => {} ) ).to.throw( CKEditorError, /^cannot-change-view-tree/ );
+					renderingCalled = true;
+
+					return element;
+				} );
+				writer.insert( ViewPosition.createAt( p ), ui );
+				writer.insert( ViewPosition.createAt( viewRoot ), p );
+			} );
+
+			expect( renderingCalled ).to.be.true;
+			domDiv.remove();
+		} );
+
+		it( 'should throw when someone tries to use change() method in post fixer', () => {
+			const domDiv = document.createElement( 'div' );
+			createViewRoot( viewDocument, 'div', 'main' );
+			view.attachDomRoot( domDiv );
+
+			viewDocument.registerPostFixer( () => {
+				expect( () => {
+					view.change( () => {} );
+				} ).to.throw( CKEditorError, /^cannot-change-view-tree/ );
+			} );
+
+			view.render();
+			domDiv.remove();
+		} );
+
 		it( 'should fire render event and it should trigger rendering before listeners on normal priority', () => {
 			const renderSpy = sinon.spy( view._renderer, 'render' );
 			const eventSpy = sinon.spy();