Merge branch 'master' into t/1289

src/conversion/downcastdispatcher.js

@@ -476,7 +476,6 @@ export default class DowncastDispatcher {
 	 * @param {Object} data Additional information about the change.
 	 * @param {module:engine/model/item~Item} data.item Inserted item.
 	 * @param {module:engine/model/range~Range} data.range Range spanning over inserted item.
-	 * @param {module:engine/conversion/modelconsumable~ModelConsumable} consumable Values to consume.
 	 * @param {Object} conversionApi Conversion interface to be used by callback, passed in `DowncastDispatcher` constructor.
 	 */
 

src/model/differ.js

@@ -229,13 +229,6 @@ export default class Differ {
 
 		// Check all changed elements.
 		for ( const element of this._changesInElement.keys() ) {
-			// Each item in `this._changesInElement` describes changes of the _children_ of that element.
-			// If the element itself has been inserted we should skip all the changes in it because the element will be reconverted.
-			// If the element itself has been removed we should skip all the changes in it because they would be incorrect.
-			if ( this._isInsertedOrRemoved( element ) ) {
-				continue;
-			}
-
 			// Get changes for this element and sort them.
 			const changes = this._changesInElement.get( element ).sort( ( a, b ) => {
 				if ( a.offset === b.offset ) {
@@ -394,46 +387,6 @@ export default class Differ {
 		this._cachedChanges = null;
 	}
 
-	/**
-	 * Checks whether a given element is inserted or removed or one of its ancestors is inserted or removed. Used to
-	 * filter out sub-changes in elements that are changed.
-	 *
-	 * @private
-	 * @param {module:engine/model/element~Element} element An element to check.
-	 * @returns {Boolean}
-	 */
-	_isInsertedOrRemoved( element ) {
-		let parent = element.parent;
-
-		// Check all ancestors of given element.
-		while ( parent ) {
-			// Get the checked element's offset.
-			const offset = element.startOffset;
-
-			if ( this._changesInElement.has( parent ) ) {
-				const changes = this._changesInElement.get( parent );
-
-				// If there were changes in that element's ancestor, check all of them.
-				for ( const change of changes ) {
-					// Skip attribute changes. We are interested only if the element was inserted or removed.
-					if ( change.type == 'attribute' ) {
-						continue;
-					}
-
-					if ( change.offset <= offset && change.offset + change.howMany > offset ) {
-						return true;
-					}
-				}
-			}
-
-			// Move up.
-			parent = parent.parent;
-			element = element.parent;
-		}
-
-		return false;
-	}
-
 	/**
 	 * Saves and handles an insert change.
 	 *
@@ -443,6 +396,10 @@ export default class Differ {
 	 * @param {Number} howMany
 	 */
 	_markInsert( parent, offset, howMany ) {
+		if ( this._isInInsertedElement( parent ) ) {
+			return;
+		}
+
 		const changeItem = { type: 'insert', offset, howMany, count: this._changeCount++ };
 
 		this._markChange( parent, changeItem );
@@ -457,9 +414,15 @@ export default class Differ {
 	 * @param {Number} howMany
 	 */
 	_markRemove( parent, offset, howMany ) {
+		if ( this._isInInsertedElement( parent ) ) {
+			return;
+		}
+
 		const changeItem = { type: 'remove', offset, howMany, count: this._changeCount++ };
 
 		this._markChange( parent, changeItem );
+
+		this._removeAllNestedChanges( parent, offset, howMany );
 	}
 
 	/**
@@ -469,6 +432,10 @@ export default class Differ {
 	 * @param {module:engine/model/item~Item} item
 	 */
 	_markAttribute( item ) {
+		if ( this._isInInsertedElement( item.parent ) ) {
+			return;
+		}
+
 		const changeItem = { type: 'attribute', offset: item.startOffset, howMany: item.offsetSize, count: this._changeCount++ };
 
 		this._markChange( item.parent, changeItem );
@@ -831,6 +798,56 @@ export default class Differ {
 
 		return diffs;
 	}
+
+	/**
+	 * Checks whether given element or any of its parents is an element that is buffered as an inserted element.
+	 *
+	 * @private
+	 * @param {module:engine/model/element~Element} element Element to check.
+	 * @returns {Boolean}
+	 */
+	_isInInsertedElement( element ) {
+		const parent = element.parent;
+
+		if ( !parent ) {
+			return false;
+		}
+
+		const changes = this._changesInElement.get( parent );
+		const offset = element.startOffset;
+
+		if ( changes ) {
+			for ( const change of changes ) {
+				if ( change.type == 'insert' && offset >= change.offset && offset < change.offset + change.howMany ) {
+					return true;
+				}
+			}
+		}
+
+		return this._isInInsertedElement( parent );
+	}
+
+	/**
+	 * Removes deeply all buffered changes that are registered in elements from range specified by `parent`, `offset`
+	 * and `howMany`.
+	 *
+	 * @private
+	 * @param {module:engine/model/element~Element} parent
+	 * @param {Number} offset
+	 * @param {Number} howMany
+	 */
+	_removeAllNestedChanges( parent, offset, howMany ) {
+		const range = Range.createFromParentsAndOffsets( parent, offset, parent, offset + howMany );
+
+		for ( const item of range.getItems( { shallow: true } ) ) {
+			if ( item.is( 'element' ) ) {
+				this._elementSnapshots.delete( item );
+				this._changesInElement.delete( item );
+
+				this._removeAllNestedChanges( item, 0, item.maxOffset );
+			}
+		}
+	}
 }
 
 // Returns an array that is a copy of passed child list with the exception that text nodes are split to one or more

src/view/document.js

@@ -65,6 +65,14 @@ export default class Document {
 		 * @member {Boolean} module:engine/view/document~Document#isFocused
 		 */
 		this.set( 'isFocused', false );
+
+		/**
+		 * Post-fixer callbacks registered to the view document.
+		 *
+		 * @private
+		 * @member {Set}
+		 */
+		this._postFixers = new Set();
 	}
 
 	/**
@@ -78,6 +86,44 @@ export default class Document {
 	getRoot( name = 'main' ) {
 		return this.roots.get( name );
 	}
+
+	/**
+	 * Used to register a post-fixer callback. A post-fixers mechanism allows to update view tree just before rendering
+	 * to the DOM.
+	 *
+	 * Post-fixers are fired just after all changes from the outermost change block were applied but
+	 * before the {@link module:engine/view/view~View#event:render render event} is fired. If a post-fixer callback made
+	 * a change, it should return `true`. When this happens, all post-fixers are fired again to check if something else should
+	 * not be fixed in the new document tree state.
+	 *
+	 * As a parameter, a post-fixer callback receives a {@link module:engine/view/writer~Writer writer} instance connected with the
+	 * executed changes block.
+	 *
+	 * @param {Function} postFixer
+	 */
+	registerPostFixer( postFixer ) {
+		this._postFixers.add( postFixer );
+	}
+
+	/**
+	 * 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;
+
+		do {
+			for ( const callback of this._postFixers ) {
+				wasFixed = callback( writer );
+
+				if ( wasFixed ) {
+					break;
+				}
+			}
+		} while ( wasFixed );
+	}
 }
 
 mix( Document, ObservableMixin );

src/view/placeholder.js

@@ -7,20 +7,16 @@
  * @module engine/view/placeholder
  */
 
-import extend from '@ckeditor/ckeditor5-utils/src/lib/lodash/extend';
-import EmitterMixin from '@ckeditor/ckeditor5-utils/src/emittermixin';
 import '../../theme/placeholder.css';
 
-const listener = {};
-extend( listener, EmitterMixin );
-
 // Each document stores information about its placeholder elements and check functions.
 const documentPlaceholders = new WeakMap();
 
 /**
  * Attaches placeholder to provided element and updates it's visibility. To change placeholder simply call this method
  * once again with new parameters.
  *
+ * @param {module:engine/view/view~View} view View controller.
  * @param {module:engine/view/element~Element} element Element to attach placeholder to.
  * @param {String} placeholderText Placeholder text to use.
  * @param {Function} [checkFunction] If provided it will be called before checking if placeholder should be displayed.
@@ -29,28 +25,19 @@ const documentPlaceholders = new WeakMap();
 export function attachPlaceholder( view, element, placeholderText, checkFunction ) {
 	const document = view.document;
 
-	// Detach placeholder if was used before.
-	detachPlaceholder( view, element );
-
 	// Single listener per document.
 	if ( !documentPlaceholders.has( document ) ) {
 		documentPlaceholders.set( document, new Map() );
 
-		// Attach listener just before rendering and update placeholders.
-		listener.listenTo( view, 'render', () => updateAllPlaceholders( view ) );
+		// Create view post fixer that will add placeholder where needed.
+		document.registerPostFixer( writer => updateAllPlaceholders( document, writer ) );
 	}
 
-	// Store text in element's data attribute.
-	// This data attribute is used in CSS class to show the placeholder.
-	view.change( writer => {
-		writer.setAttribute( 'data-placeholder', placeholderText, element );
-	} );
-
-	// Store information about placeholder.
-	documentPlaceholders.get( document ).set( element, checkFunction );
+	// Store information about element with placeholder.
+	documentPlaceholders.get( document ).set( element, { placeholderText, checkFunction } );
 
-	// Update right away too.
-	updateSinglePlaceholder( view, element, checkFunction );
+	// Update view right away.
+	view.render();
 }
 
 /**
@@ -75,39 +62,55 @@ export function detachPlaceholder( view, element ) {
 // Updates all placeholders of given document.
 //
 // @private
-// @param {module:engine/view/view~View} view
-function updateAllPlaceholders( view ) {
-	const placeholders = documentPlaceholders.get( view.document );
-
-	for ( const [ element, checkFunction ] of placeholders ) {
-		updateSinglePlaceholder( view, element, checkFunction );
+// @param {module:engine/view/document~Document} view
+// @param {module:engine/view/writer~Writer} writer
+function updateAllPlaceholders( document, writer ) {
+	const placeholders = documentPlaceholders.get( document );
+	let changed = false;
+
+	for ( const [ element, info ] of placeholders ) {
+		if ( updateSinglePlaceholder( writer, element, info ) ) {
+			changed = true;
+		}
 	}
+
+	return changed;
 }
 
 // Updates placeholder class of given element.
 //
 // @private
-// @param {module:engine/view/view~View} view
+// @param {module:engine/view/writer~Writer} writer
 // @param {module:engine/view/element~Element} element
-// @param {Function} checkFunction
-function updateSinglePlaceholder( view, element, checkFunction ) {
+// @param {Object} info
+function updateSinglePlaceholder( writer, element, info ) {
 	const document = element.document;
+	const text = info.placeholderText;
+	let changed = false;
 
 	// Element was removed from document.
 	if ( !document ) {
-		return;
+		return false;
+	}
+
+	// Update data attribute if needed.
+	if ( element.getAttribute( 'data-placeholder' ) !== text ) {
+		writer.setAttribute( 'data-placeholder', text, element );
+		changed = true;
 	}
 
 	const viewSelection = document.selection;
 	const anchor = viewSelection.anchor;
+	const checkFunction = info.checkFunction;
 
 	// If checkFunction is provided and returns false - remove placeholder.
 	if ( checkFunction && !checkFunction() ) {
-		view.change( writer => {
+		if ( element.hasClass( 'ck-placeholder' ) ) {
 			writer.removeClass( 'ck-placeholder', element );
-		} );
+			changed = true;
+		}
 
-		return;
+		return changed;
 	}
 
 	// Element is empty for placeholder purposes when it has no children or only ui elements.
@@ -116,21 +119,26 @@ function updateSinglePlaceholder( view, element, checkFunction ) {
 
 	// If element is empty and editor is blurred.
 	if ( !document.isFocused && isEmptyish ) {
-		view.change( writer => {
+		if ( !element.hasClass( 'ck-placeholder' ) ) {
 			writer.addClass( 'ck-placeholder', element );
-		} );
+			changed = true;
+		}
 
-		return;
+		return changed;
 	}
 
 	// It there are no child elements and selection is not placed inside element.
 	if ( isEmptyish && anchor && anchor.parent !== element ) {
-		view.change( writer => {
+		if ( !element.hasClass( 'ck-placeholder' ) ) {
 			writer.addClass( 'ck-placeholder', element );
-		} );
+			changed = true;
+		}
 	} else {
-		view.change( writer => {
+		if ( element.hasClass( 'ck-placeholder' ) ) {
 			writer.removeClass( 'ck-placeholder', element );
-		} );
+			changed = true;
+		}
 	}
+
+	return changed;
 }