Merge 4a9e4e8 into 05e4e20

src/controller/editingcontroller.js

@@ -75,14 +75,11 @@ export default class EditingController {
 		//
 		// See  https://github.com/ckeditor/ckeditor5-engine/issues/1528
 		this.listenTo( this.model, '_beforeChanges', () => {
-			this.view._renderingDisabled = true;
+			this.view._disableRendering( true );
 		}, { priority: 'highest' } );
 
-		this.listenTo( this.model, '_afterChanges', ( evt, { hasModelDocumentChanged } ) => {
-			this.view._renderingDisabled = false;
-			if ( hasModelDocumentChanged ) {
-				this.view.render();
-			}
+		this.listenTo( this.model, '_afterChanges', () => {
+			this.view._disableRendering( false );
 		}, { priority: 'lowest' } );
 
 		// Whenever model document is changed, convert those changes to the view (using model.Document#differ).

src/model/document.js

@@ -240,8 +240,8 @@ export default class Document {
 	}
 
 	/**
-	 * Used to register a post-fixer callback. A post-fixer mechanism guarantees that the features that listen to
-	 * the {@link module:engine/model/model~Model#event:_change model's change event} will operate on a correct model state.
+	 * Used to register a post-fixer callback. A post-fixer mechanism guarantees that the features
+	 * will operate on a correct model state.
 	 *
 	 * An execution of a feature may lead to an incorrect document tree state. The callbacks are used to fix the document tree after
 	 * it has changed. Post-fixers are fired just after all changes from the outermost change block were applied but

src/model/model.js

@@ -683,7 +683,6 @@ export default class Model {
 	 */
 	_runPendingChanges() {
 		const ret = [];
-		let hasModelDocumentChanged = false;
 
 		this.fire( '_beforeChanges' );
 
@@ -696,9 +695,6 @@ export default class Model {
 			const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter );
 			ret.push( callbackReturnValue );
 
-			// Collect an information whether the model document has changed during from the last pending change.
-			hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock();
-
 			// Fire '_change' event before resetting differ.
 			this.fire( '_change', this._currentWriter );
 
@@ -708,7 +704,7 @@ export default class Model {
 			this._currentWriter = null;
 		}
 
-		this.fire( '_afterChanges', { hasModelDocumentChanged } );
+		this.fire( '_afterChanges' );
 
 		return ret;
 	}
@@ -739,8 +735,6 @@ export default class Model {
 	 *
 	 * @protected
 	 * @event _afterChanges
-	 * @param {Object} options
-	 * @param {Boolean} options.hasModelDocumentChanged `true` if the model document has changed during the
 	 * {@link module:engine/model/model~Model#change} or {@link module:engine/model/model~Model#enqueueChange} blocks.
 	 */
 

src/view/document.js

@@ -115,6 +115,9 @@ export default class Document {
 	 * As a parameter, a post-fixer callback receives a {@link module:engine/view/downcastwriter~DowncastWriter downcast writer}
 	 * instance connected with the executed changes block.
 	 *
+	 * Note that registering a post-fixer won't re-render the editor's view. If the view should change after registering the post-fixer then
+	 * it should be done manually calling `view.forceRender();`.
+	 *
 	 * @param {Function} postFixer
 	 */
 	registerPostFixer( postFixer ) {

src/view/observer/focusobserver.js

@@ -37,7 +37,7 @@ export default class FocusObserver extends DomEventObserver {
 			// overwrite new DOM selection with selection from the view.
 			// See https://github.com/ckeditor/ckeditor5-engine/issues/795 for more details.
 			// Long timeout is needed to solve #676 and https://github.com/ckeditor/ckeditor5-engine/issues/1157 issues.
-			this._renderTimeoutId = setTimeout( () => view.render(), 50 );
+			this._renderTimeoutId = setTimeout( () => view.forceRender(), 50 );
 		} );
 
 		document.on( 'blur', ( evt, data ) => {
@@ -47,7 +47,7 @@ export default class FocusObserver extends DomEventObserver {
 				document.isFocused = false;
 
 				// Re-render the document to update view elements.
-				view.render();
+				view.forceRender();
 			}
 		} );
 

src/view/observer/mutationobserver.js

@@ -248,7 +248,7 @@ export default class MutationObserver extends Observer {
 
 		// If nothing changes on `mutations` event, at this point we have "dirty DOM" (changed) and de-synched
 		// view (which has not been changed). In order to "reset DOM" we render the view again.
-		this.view.render();
+		this.view.forceRender();
 
 		function sameNodes( child1, child2 ) {
 			// First level of comparison (array of children vs array of children) – use the Lodash's default behavior.

src/view/observer/selectionobserver.js

@@ -166,7 +166,7 @@ export default class SelectionObserver extends Observer {
 		if ( this.selection.isSimilar( newViewSelection ) ) {
 			// If selection was equal and we are at this point of algorithm, it means that it was incorrect.
 			// Just re-render it, no need to fire any events, etc.
-			this.view.render();
+			this.view.forceRender();
 		} else {
 			const data = {
 				oldSelection: this.selection,

src/view/placeholder.js

@@ -38,9 +38,6 @@ export function attachPlaceholder( view, element, placeholderText, checkFunction
 		placeholderText,
 		checkFunction
 	} );
-
-	// Update view right away.
-	view.render();
 }
 
 /**

src/view/view.js

@@ -66,7 +66,7 @@ export default class View {
 		 * Instance of the {@link module:engine/view/document~Document} associated with this view controller.
 		 *
 		 * @readonly
-		 * @member {module:engine/view/document~Document} module:engine/view/view~View#document
+		 * @type {module:engine/view/document~Document}
 		 */
 		this.document = new Document();
 
@@ -76,15 +76,15 @@ export default class View {
 		 * and {@link module:engine/view/observer/observer~Observer observers}.
 		 *
 		 * @readonly
-		 * @member {module:engine/view/domconverter~DomConverter} module:engine/view/view~View#domConverter
+		 * @type {module:engine/view/domconverter~DomConverter}
 		 */
 		this.domConverter = new DomConverter();
 
 		/**
 		 * Instance of the {@link module:engine/view/renderer~Renderer renderer}.
 		 *
 		 * @protected
-		 * @member {module:engine/view/renderer~Renderer} module:engine/view/view~View#renderer
+		 * @type {module:engine/view/renderer~Renderer}
 		 */
 		this._renderer = new Renderer( this.domConverter, this.document.selection );
 		this._renderer.bind( 'isFocused' ).to( this.document );
@@ -93,55 +93,64 @@ export default class View {
 		 * Roots of the DOM tree. Map on the `HTMLElement`s with roots names as keys.
 		 *
 		 * @readonly
-		 * @member {Map} module:engine/view/view~View#domRoots
+		 * @type {Map.<String, HTMLElement>}
 		 */
 		this.domRoots = new Map();
 
 		/**
 		 * Map of registered {@link module:engine/view/observer/observer~Observer observers}.
 		 *
 		 * @private
-		 * @member {Map.<Function, module:engine/view/observer/observer~Observer>} module:engine/view/view~View#_observers
+		 * @type {Map.<Function, module:engine/view/observer/observer~Observer>}
 		 */
 		this._observers = new Map();
 
 		/**
 		 * Is set to `true` when {@link #change view changes} are currently in progress.
 		 *
 		 * @private
-		 * @member {Boolean} module:engine/view/view~View#_ongoingChange
+		 * @type {Boolean}
 		 */
 		this._ongoingChange = false;
 
 		/**
 		 * Used to prevent calling {@link #render} and {@link #change} during rendering view to the DOM.
 		 *
 		 * @private
-		 * @member {Boolean} module:engine/view/view~View#_renderingInProgress
+		 * @type {Boolean}
 		 */
 		this._renderingInProgress = false;
 
 		/**
 		 * Used to prevent calling {@link #render} and {@link #change} during rendering view to the DOM.
 		 *
 		 * @private
-		 * @member {Boolean} module:engine/view/view~View#_renderingInProgress
+		 * @type {Boolean}
 		 */
 		this._postFixersInProgress = false;
 
 		/**
-		 * Internal flag to temporary disable rendering. See usage in the editing controller.
+		 * Internal flag to temporary disable rendering. See the usage in the {@link #_disableRendering}.
 		 *
-		 * @protected
-		 * @member {Boolean} module:engine/view/view~View#_renderingDisabled
+		 * @private
+		 * @type {Boolean}
 		 */
 		this._renderingDisabled = false;
 
 		/**
-		 * DowncastWriter instance used in {@link #change change method) callbacks.
+		 * Internal flag that disables rendering when there are no changes since the last rendering.
+		 * It stores information about changed selection and changed elements from attached document roots.
+		 *
+		 * @private
+		 * @type {Boolean}
+		 */
+		this._hasChangedSinceTheLastRendering = false;
+
+		/**
+		 * DowncastWriter instance used in {@link #change change method} callbacks.
 		 *
 		 * @private
-		 * @member {module:engine/view/downcastwriter~DowncastWriter} module:engine/view/view~View#_writer
+		 * @type {module:engine/view/downcastwriter~DowncastWriter}
 		 */
 		this._writer = new DowncastWriter( this.document );
 
@@ -163,6 +172,14 @@ export default class View {
 
 			// Informs that layout has changed after render.
 			this.document.fire( 'layoutChanged' );
+
+			// Reset the `_hasChangedSinceTheLastRendering` flag after rendering.
+			this._hasChangedSinceTheLastRendering = false;
+		} );
+
+		// Listen to the document selection changes directly.
+		this.listenTo( this.document.selection, 'change', () => {
+			this._hasChangedSinceTheLastRendering = true;
 		} );
 	}
 
@@ -192,6 +209,10 @@ export default class View {
 		viewRoot.on( 'change:attributes', ( evt, node ) => this._renderer.markToSync( 'attributes', node ) );
 		viewRoot.on( 'change:text', ( evt, node ) => this._renderer.markToSync( 'text', node ) );
 
+		viewRoot.on( 'change', () => {
+			this._hasChangedSinceTheLastRendering = true;
+		} );
+
 		for ( const observer of this._observers.values() ) {
 			observer.observe( domRoot, name );
 		}
@@ -293,7 +314,7 @@ export default class View {
 
 			if ( editable ) {
 				this.domConverter.focus( editable );
-				this.render();
+				this.forceRender();
 			} else {
 				/**
 				 * Before focusing view document, selection should be placed inside one of the view's editables.
@@ -309,9 +330,10 @@ export default class View {
 
 	/**
 	 * The `change()` method is the primary way of changing the view. You should use it to modify any node in the view tree.
-	 * It makes sure that after all changes are made the view is rendered to the DOM. It prevents situations when the DOM is updated
-	 * when the view state is not yet correct. It allows to nest calls one inside another and still performs a single rendering
-	 * after all those changes are made. It also returns the return value of its callback.
+	 * It makes sure that after all changes are made the view is rendered to the DOM (assuming that the view will be changed
+	 * inside the callback). It prevents situations when the DOM is updated when the view state is not yet correct.It allows
+	 * to nest calls one inside another and still performs a single rendering after all those changes are made.
+	 * It also returns the return value of its callback.
 	 *
 	 *		const text = view.change( writer => {
 	 *			const newText = writer.createText( 'foo' );
@@ -368,7 +390,8 @@ export default class View {
 
 		// This lock is used by editing controller to render changes from outer most model.change() once. As plugins might call
 		// view.change() inside model.change() block - this will ensures that postfixers and rendering are called once after all changes.
-		if ( !this._renderingDisabled ) {
+		// Also, we don't need to render anything if there're no changes since last rendering.
+		if ( !this._renderingDisabled && this._hasChangedSinceTheLastRendering ) {
 			this._postFixersInProgress = true;
 			this.document._callPostFixers( this._writer );
 			this._postFixersInProgress = false;
@@ -386,7 +409,8 @@ export default class View {
 	 * Throws {@link module:utils/ckeditorerror~CKEditorError CKEditorError} `applying-view-changes-on-rendering` when
 	 * trying to re-render when rendering to DOM has already started.
 	 */
-	render() {
+	forceRender() {
+		this._hasChangedSinceTheLastRendering = true;
 		this.change( () => {} );
 	}
 
@@ -542,6 +566,22 @@ export default class View {
 		return new Selection( selectable, placeOrOffset, options );
 	}
 
+	/**
+	 * Disables or enables rendering. If the flag is set to `true` then the rendering will be disabled.
+	 * If the flag is set to `false` and if there was some change in the meantime, then the rendering action will be performed.
+	 *
+	 * @protected
+	 * @param {Boolean} flag A flag indicates whether the rendering should be disabled.
+	 */
+	_disableRendering( flag ) {
+		this._renderingDisabled = flag;
+
+		if ( flag == false ) {
+			// Render when you stop blocking rendering.
+			this.change( () => {} );
+		}
+	}
+
 	/**
 	 * Renders all changes. In order to avoid triggering the observers (e.g. mutations) all observers are disabled
 	 * before rendering and re-enabled after that.

tests/controller/editingcontroller.js

@@ -186,7 +186,7 @@ describe( 'EditingController', () => {
 			} );
 
 			editing.view.document.isFocused = true;
-			editing.view.render();
+			editing.view.forceRender();
 
 			const domSelection = document.getSelection();
 			domSelection.removeAllRanges();

tests/tickets/1653.js

@@ -9,7 +9,7 @@ import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictest
 import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 
 describe( 'Bug ckeditor5-engine#1653', () => {
-	it( '`DataController.parse()` should not invoke `editing.view.render()`', () => {
+	it( '`DataController.parse()` should not fire `editing.view#render`', () => {
 		let editor;
 
 		const element = document.createElement( 'div' );
@@ -20,10 +20,12 @@ describe( 'Bug ckeditor5-engine#1653', () => {
 			.then( newEditor => {
 				editor = newEditor;
 
-				const spy = sinon.spy( editor.editing.view, 'render' );
+				const editingViewSpy = sinon.spy();
+
+				editor.editing.view.on( 'fire', editingViewSpy );
 				editor.data.parse( '<p></p>' );
 
-				sinon.assert.notCalled( spy );
+				sinon.assert.notCalled( editingViewSpy );
 			} )
 			.then( () => {
 				element.remove();

tests/view/manual/immutable.js

@@ -24,7 +24,7 @@ setData( view,
 viewDocument.on( 'selectionChange', () => {
 	// Re-render view selection each time selection is changed.
 	// See https://github.com/ckeditor/ckeditor5-engine/issues/796.
-	view.render();
+	view.forceRender();
 } );
 
 view.focus();

tests/view/manual/noselection.js

@@ -17,7 +17,7 @@ view.attachDomRoot( document.getElementById( 'editor' ) );
 viewDocument.on( 'selectionChange', () => {
 	// Re-render view selection each time selection is changed.
 	// See https://github.com/ckeditor/ckeditor5-engine/issues/796.
-	view.render();
+	view.forceRender();
 } );
 
 setData( view,

tests/view/observer/domeventobserver.js

@@ -183,7 +183,7 @@ describe( 'DomEventObserver', () => {
 			view.attachDomRoot( domRoot );
 			uiElement = createUIElement( 'p' );
 			viewRoot._appendChild( uiElement );
-			view.render();
+			view.forceRender();
 
 			domEvent = new MouseEvent( 'click', { bubbles: true } );
 			evtSpy = sinon.spy();