Selaa lähdekoodia

Merge pull request #1644 from ckeditor/t/ckeditor5/479

Feature: Moved the root element DOM attributes management from the UI to the engine. Made it possible to use `addPlaceholder()` (now `enablePlaceholder()`) on the root editable. Introduced the `View.detachDomRoot()` method. Implemented additional placeholder helpers (`showPlaceholder()`, `hidePlaceholder()`, `needsPlaceholder()`) (see ckeditor/ckeditor5#479). Closes #899.

BREAKING CHANGE: The `attachPlaceholder()` has been renamed to `enablePlaceholder()`.
BREAKING CHANGE: `enablePlaceholder()` accepts a configuration object instead of separate parameters.
BREAKING CHANGE: The `detachPlaceholder()` has been renamed to `disablePlaceholder()`.
Piotr Jasiun 6 vuotta sitten
vanhempi
commit
425976ea4e

+ 191 - 83
packages/ckeditor5-engine/src/view/placeholder.js

@@ -13,134 +13,242 @@ import '../../theme/placeholder.css';
 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.
+ * A helper that enables a placeholder on the provided view element (also updates its visibility).
+ * The placeholder is a CSS pseudo–element (with a text content) attached to the element.
  *
- * @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.
- * If function returns `false` placeholder will not be showed.
+ * To change the placeholder text, simply call this method again with new options.
+ *
+ * To disable the placeholder, use {@link module:engine/view/placeholder~disablePlaceholder `disablePlaceholder()`} helper.
+ *
+ * @param {Object} [options] Configuration options of the placeholder.
+ * @param {module:engine/view/view~View} options.view Editing view instance.
+ * @param {module:engine/view/element~Element} options.element Element that will gain a placeholder.
+ * See `options.isDirectHost` to learn more.
+ * @param {String} options.text Placeholder text.
+ * @param {Boolean} [options.isDirectHost=true] If set `false`, the placeholder will not be enabled directly
+ * in the passed `element` but in one of its children (selected automatically, i.e. a first empty child element).
+ * Useful when attaching placeholders to elements that can host other elements (not just text), for instance,
+ * editable root elements.
  */
-export function attachPlaceholder( view, element, placeholderText, checkFunction ) {
-	const document = view.document;
+export function enablePlaceholder( options ) {
+	const { view, element, text, isDirectHost = true } = options;
+	const doc = view.document;
 
-	// Single listener per document.
-	if ( !documentPlaceholders.has( document ) ) {
-		documentPlaceholders.set( document, new Map() );
+	// Use a single a single post fixer per—document to update all placeholders.
+	if ( !documentPlaceholders.has( doc ) ) {
+		documentPlaceholders.set( doc, new Map() );
 
-		// Create view post-fixer that will add placeholder where needed.
-		document.registerPostFixer( writer => updateAllPlaceholders( document, writer ) );
+		// If a post-fixer callback makes a change, it should return `true` so other post–fixers
+		// can re–evaluate the document again.
+		doc.registerPostFixer( writer => updateDocumentPlaceholders( doc, writer ) );
 	}
 
-	// Store information about element with placeholder.
-	documentPlaceholders.get( document ).set( element, {
-		placeholderText,
-		checkFunction
+	// Store information about the element placeholder under its document.
+	documentPlaceholders.get( doc ).set( element, {
+		text,
+		isDirectHost
 	} );
 
-	view.change( writer => updateAllPlaceholders( document, writer ) );
+	// Update the placeholders right away.
+	view.change( writer => updateDocumentPlaceholders( doc, writer ) );
 }
 
 /**
- * Removes placeholder functionality from given element.
+ * Disables the placeholder functionality from a given element.
+ *
+ * See {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`} to learn more.
  *
  * @param {module:engine/view/view~View} view
  * @param {module:engine/view/element~Element} element
  */
-export function detachPlaceholder( view, element ) {
+export function disablePlaceholder( view, element ) {
 	const doc = element.document;
 
 	view.change( writer => {
-		if ( documentPlaceholders.has( doc ) ) {
-			documentPlaceholders.get( doc ).delete( element );
+		if ( !documentPlaceholders.has( doc ) ) {
+			return;
 		}
 
-		writer.removeClass( 'ck-placeholder', element );
-		writer.removeAttribute( 'data-placeholder', element );
+		const placeholders = documentPlaceholders.get( doc );
+		const config = placeholders.get( element );
+
+		writer.removeAttribute( 'data-placeholder', config.hostElement );
+		hidePlaceholder( writer, config.hostElement );
+
+		placeholders.delete( element );
 	} );
 }
 
-// Updates all placeholders of given document.
+/**
+ * Shows a placeholder in the provided element by changing related attributes and CSS classes.
+ *
+ * **Note**: This helper will not update the placeholder visibility nor manage the
+ * it in any way in the future. What it does is a one–time state change of an element. Use
+ * {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`} and
+ * {@link module:engine/view/placeholder~disablePlaceholder `disablePlaceholder()`} for full
+ * placeholder functionality.
+ *
+ * **Note**: This helper will blindly show the placeholder directly in the root editable element if
+ * one is passed, which could result in a visual clash if the editable element has some children
+ * (for instance, an empty paragraph). Use {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`}
+ * in that case or make sure the correct element is passed to the helper.
+ *
+ * @param {module:engine/view/downcastwriter~DowncastWriter} writer
+ * @param {module:engine/view/element~Element} element
+ * @returns {Boolean} `true`, if any changes were made to the `element`.
+ */
+export function showPlaceholder( writer, element ) {
+	if ( !element.hasClass( 'ck-placeholder' ) ) {
+		writer.addClass( 'ck-placeholder', element );
+
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * Hides a placeholder in the element by changing related attributes and CSS classes.
+ *
+ * **Note**: This helper will not update the placeholder visibility nor manage the
+ * it in any way in the future. What it does is a one–time state change of an element. Use
+ * {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`} and
+ * {@link module:engine/view/placeholder~disablePlaceholder `disablePlaceholder()`} for full
+ * placeholder functionality.
+ *
+ * @param {module:engine/view/downcastwriter~DowncastWriter} writer
+ * @param {module:engine/view/element~Element} element
+ * @returns {Boolean} `true`, if any changes were made to the `element`.
+ */
+export function hidePlaceholder( writer, element ) {
+	if ( element.hasClass( 'ck-placeholder' ) ) {
+		writer.removeClass( 'ck-placeholder', element );
+
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * Checks if a placeholder should be displayed in the element.
+ *
+ * **Note**: This helper will blindly check the possibility of showing a placeholder directly in the
+ * root editable element if one is passed, which may not be the expected result. If an element can
+ * host other elements (not just text), most likely one of its children should be checked instead
+ * because it will be the final host for the placeholder. Use
+ * {@link module:engine/view/placeholder~enablePlaceholder `enablePlaceholder()`} in that case or make
+ * sure the correct element is passed to the helper.
+ *
+ * @param {module:engine/view/downcastwriter~DowncastWriter} writer
+ * @param {module:engine/view/element~Element} element
+ * @param {String} text
+ * @returns {Boolean}
+ */
+export function needsPlaceholder( element ) {
+	const doc = element.document;
+
+	// The element was removed from document.
+	if ( !doc ) {
+		return false;
+	}
+
+	// The element is empty only as long as it contains nothing but uiElements.
+	const isEmptyish = !Array.from( element.getChildren() )
+		.some( element => !element.is( 'uiElement' ) );
+
+	// If the element is empty and the document is blurred.
+	if ( !doc.isFocused && isEmptyish ) {
+		return true;
+	}
+
+	const viewSelection = doc.selection;
+	const selectionAnchor = viewSelection.anchor;
+
+	// If document is focused and the element is empty but the selection is not anchored inside it.
+	if ( isEmptyish && selectionAnchor && selectionAnchor.parent !== element ) {
+		return true;
+	}
+
+	return false;
+}
+
+// Updates all placeholders associated with a document in a post–fixer callback.
 //
 // @private
-// @param {module:engine/view/document~Document} view
+// @param { module:engine/model/document~Document} doc
 // @param {module:engine/view/downcastwriter~DowncastWriter} 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;
+// @returns {Boolean} True if any changes were made to the view document.
+function updateDocumentPlaceholders( doc, writer ) {
+	const placeholders = documentPlaceholders.get( doc );
+	let wasViewModified = false;
+
+	for ( const [ element, config ] of placeholders ) {
+		if ( updatePlaceholder( writer, element, config ) ) {
+			wasViewModified = true;
 		}
 	}
 
-	return changed;
+	return wasViewModified;
 }
 
-// Updates placeholder class of given element.
+// Updates a single placeholder in a post–fixer callback.
 //
 // @private
 // @param {module:engine/view/downcastwriter~DowncastWriter} writer
 // @param {module:engine/view/element~Element} element
-// @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 ) {
+// @param {Object} config Configuration of the placeholder
+// @param {String} config.text
+// @param {Boolean} config.isDirectHost
+// @returns {Boolean} True if any changes were made to the view document.
+function updatePlaceholder( writer, element, config ) {
+	const { text, isDirectHost } = config;
+	const hostElement = isDirectHost ? element : getChildPlaceholderHostSubstitute( element );
+	let wasViewModified = false;
+
+	// When not a direct host, it could happen that there is no child element
+	// capable of displaying a placeholder.
+	if ( !hostElement ) {
 		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() ) {
-		if ( element.hasClass( 'ck-placeholder' ) ) {
-			writer.removeClass( 'ck-placeholder', element );
-			changed = true;
-		}
+	// Cache the host element. It will be necessary for disablePlaceholder() to know
+	// which element should have class and attribute removed because, depending on
+	// the config.isDirectHost value, it could be the element or one of its descendants.
+	config.hostElement = hostElement;
 
-		return changed;
+	// This may be necessary when updating the placeholder text to something else.
+	if ( hostElement.getAttribute( 'data-placeholder' ) !== text ) {
+		writer.setAttribute( 'data-placeholder', text, hostElement );
+		wasViewModified = true;
 	}
 
-	// Element is empty for placeholder purposes when it has no children or only ui elements.
-	// This check is taken from `view.ContainerElement#getFillerOffset`.
-	const isEmptyish = !Array.from( element.getChildren() ).some( element => !element.is( 'uiElement' ) );
-
-	// If element is empty and editor is blurred.
-	if ( !document.isFocused && isEmptyish ) {
-		if ( !element.hasClass( 'ck-placeholder' ) ) {
-			writer.addClass( 'ck-placeholder', element );
-			changed = true;
+	if ( needsPlaceholder( hostElement ) ) {
+		if ( showPlaceholder( writer, hostElement ) ) {
+			wasViewModified = true;
 		}
-
-		return changed;
+	} else if ( hidePlaceholder( writer, hostElement ) ) {
+		wasViewModified = true;
 	}
 
-	// It there are no child elements and selection is not placed inside element.
-	if ( isEmptyish && anchor && anchor.parent !== element ) {
-		if ( !element.hasClass( 'ck-placeholder' ) ) {
-			writer.addClass( 'ck-placeholder', element );
-			changed = true;
-		}
-	} else {
-		if ( element.hasClass( 'ck-placeholder' ) ) {
-			writer.removeClass( 'ck-placeholder', element );
-			changed = true;
+	return wasViewModified;
+}
+
+// Gets a child element capable of displaying a placeholder if a parent element can host more
+// than just text (for instance, when it is a root editable element). The child element
+// can then be used in other placeholder helpers as a substitute of its parent.
+//
+// @private
+// @param {module:engine/view/element~Element} parent
+// @returns {module:engine/view/element~Element|null}
+function getChildPlaceholderHostSubstitute( parent ) {
+	if ( parent.childCount === 1 ) {
+		const firstChild = parent.getChild( 0 );
+
+		if ( firstChild.is( 'element' ) && !firstChild.is( 'uiElement' ) ) {
+			return firstChild;
 		}
 	}
 
-	return changed;
+	return null;
 }

+ 73 - 3
packages/ckeditor5-engine/src/view/view.js

@@ -98,6 +98,17 @@ export default class View {
 		this._renderer.bind( 'isFocused' ).to( this.document );
 
 		/**
+		 * A DOM root attributes cache. It saves the initial values of DOM root attributes before the DOM element
+		 * is {@link module:engine/view/view~View#attachDomRoot attached} to the view so later on, when
+		 * the view is destroyed ({@link module:engine/view/view~View#detachDomRoot}), they can be easily restored.
+		 * This way, the DOM element can go back to the (clean) state as if the editing view never used it.
+		 *
+		 * @private
+		 * @member {WeakMap.<HTMLElement,Object>}
+		 */
+		this._initialDomRootAttributes = new WeakMap();
+
+		/**
 		 * Map of registered {@link module:engine/view/observer/observer~Observer observers}.
 		 *
 		 * @private
@@ -184,13 +195,15 @@ export default class View {
 	}
 
 	/**
-	 * Attaches DOM root element to the view element and enable all observers on that element.
-	 * Also {@link module:engine/view/renderer~Renderer#markToSync mark element} to be synchronized with the view
-	 * what means that all child nodes will be removed and replaced with content of the view root.
+	 * Attaches a DOM root element to the view element and enable all observers on that element.
+	 * Also {@link module:engine/view/renderer~Renderer#markToSync mark element} to be synchronized
+	 * with the view what means that all child nodes will be removed and replaced with content of the view root.
 	 *
 	 * This method also will change view element name as the same as tag name of given dom root.
 	 * Name is always transformed to lower case.
 	 *
+	 * **Note:** Use {@link #detachDomRoot `detachDomRoot()`} to revert this action.
+	 *
 	 * @param {Element} domRoot DOM root element.
 	 * @param {String} [name='main'] Name of the root.
 	 */
@@ -200,14 +213,48 @@ export default class View {
 		// Set view root name the same as DOM root tag name.
 		viewRoot._name = domRoot.tagName.toLowerCase();
 
+		const initialDomRootAttributes = {};
+
+		// 1. Copy and cache the attributes to remember the state of the element before attaching.
+		//    The cached attributes will be restored in detachDomRoot() so the element goes to the
+		//    clean state as if the editing view never used it.
+		// 2. Apply the attributes using the view writer, so they all go under the control of the engine.
+		//    The editing view takes over the attribute management completely because various
+		//    features (e.g. addPlaceholder()) require dynamic changes of those attributes and they
+		//    cannot be managed by the engine and the UI library at the same time.
+		for ( const { name, value } of Array.from( domRoot.attributes ) ) {
+			initialDomRootAttributes[ name ] = value;
+
+			// Do not use writer.setAttribute() for the class attribute. The EditableUIView class
+			// and its descendants could have already set some using the writer.addClass() on the view
+			// document root. They haven't been rendered yet so they are not present in the DOM root.
+			// Using writer.setAttribute( 'class', ... ) would override them completely.
+			if ( name === 'class' ) {
+				this._writer.addClass( value.split( ' ' ), viewRoot );
+			} else {
+				this._writer.setAttribute( name, value, viewRoot );
+			}
+		}
+
+		this._initialDomRootAttributes.set( domRoot, initialDomRootAttributes );
+
+		const updateContenteditableAttribute = () => {
+			this._writer.setAttribute( 'contenteditable', !viewRoot.isReadOnly, viewRoot );
+		};
+
+		// Set initial value.
+		updateContenteditableAttribute();
+
 		this.domRoots.set( name, domRoot );
 		this.domConverter.bindElements( domRoot, viewRoot );
 		this._renderer.markToSync( 'children', viewRoot );
+		this._renderer.markToSync( 'attributes', viewRoot );
 		this._renderer.domDocuments.add( domRoot.ownerDocument );
 
 		viewRoot.on( 'change:children', ( evt, node ) => this._renderer.markToSync( 'children', node ) );
 		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:isReadOnly', () => this.change( updateContenteditableAttribute ) );
 
 		viewRoot.on( 'change', () => {
 			this._hasChangedSinceTheLastRendering = true;
@@ -219,6 +266,29 @@ export default class View {
 	}
 
 	/**
+	 * Detaches a DOM root element from the view element and restores its attributes to the state before
+	 * {@link #attachDomRoot `attachDomRoot()`}.
+	 *
+	 * @param {String} name Name of the root to detach.
+	 */
+	detachDomRoot( name ) {
+		const domRoot = this.domRoots.get( name );
+
+		// Remove all root attributes so the DOM element is "bare".
+		[ ...domRoot.attributes ].forEach( ( { name } ) => domRoot.removeAttribute( name ) );
+
+		const initialDomRootAttributes = this._initialDomRootAttributes.get( domRoot );
+
+		// Revert all view root attributes back to the state before attachDomRoot was called.
+		for ( const attribute in initialDomRootAttributes ) {
+			domRoot.setAttribute( attribute, initialDomRootAttributes[ attribute ] );
+		}
+
+		this.domRoots.delete( name );
+		this.domConverter.unbindDomElement( domRoot );
+	}
+
+	/**
 	 * Gets DOM root element.
 	 *
 	 * @param {String} [name='main']  Name of the root.

+ 13 - 3
packages/ckeditor5-engine/tests/manual/placeholder.js

@@ -12,7 +12,7 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import Undo from '@ckeditor/ckeditor5-undo/src/undo';
 import Heading from '@ckeditor/ckeditor5-heading/src/heading';
 import global from '@ckeditor/ckeditor5-utils/src/dom/global';
-import { attachPlaceholder } from '../../src/view/placeholder';
+import { enablePlaceholder } from '../../src/view/placeholder';
 
 ClassicEditor
 	.create( global.document.querySelector( '#editor' ), {
@@ -25,8 +25,18 @@ ClassicEditor
 		const header = viewDoc.getRoot().getChild( 0 );
 		const paragraph = viewDoc.getRoot().getChild( 1 );
 
-		attachPlaceholder( view, header, 'Type some header text...' );
-		attachPlaceholder( view, paragraph, 'Type some paragraph text...' );
+		enablePlaceholder( {
+			view,
+			element: header,
+			text: 'Type some header text...'
+		} );
+
+		enablePlaceholder( {
+			view,
+			element: paragraph,
+			text: 'Type some paragraph text...'
+		} );
+
 		view.render();
 	} )
 	.catch( err => {

+ 244 - 34
packages/ckeditor5-engine/tests/view/placeholder.js

@@ -3,7 +3,13 @@
  * For licensing, see LICENSE.md.
  */
 
-import { attachPlaceholder, detachPlaceholder } from '../../src/view/placeholder';
+import {
+	enablePlaceholder,
+	disablePlaceholder,
+	showPlaceholder,
+	hidePlaceholder,
+	needsPlaceholder
+} from '../../src/view/placeholder';
 import createViewRoot from './_utils/createroot';
 import View from '../../src/view/view';
 import ViewRange from '../../src/view/range';
@@ -19,22 +25,45 @@ describe( 'placeholder', () => {
 		viewDocument.isFocused = true;
 	} );
 
-	describe( 'createPlaceholder', () => {
+	describe( 'enablePlaceholder', () => {
 		it( 'should attach proper CSS class and data attribute', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 		} );
 
+		it( 'should attach proper CSS class and data attribute (isDirectHost=false)', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+		} );
+
 		it( 'if element has children set only data attribute', () => {
 			setData( view, '<div>first div</div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -44,7 +73,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div><ui:span></ui:span><ui:span></ui:span></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -54,7 +87,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div>[]</div><div>another div</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -65,34 +102,27 @@ describe( 'placeholder', () => {
 			const element = viewRoot.getChild( 0 );
 			viewDocument.isFocused = false;
 
-			attachPlaceholder( view, element, 'foo bar baz' );
-
-			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
-		} );
-
-		it( 'use check function if one is provided', () => {
-			setData( view, '<div></div><div>{another div}</div>' );
-			const element = viewRoot.getChild( 0 );
-			let result = true;
-			const spy = sinon.spy( () => result );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
-			attachPlaceholder( view, element, 'foo bar baz', spy );
+			view.forceRender();
 
-			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.forceRender();
-			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
 
 		it( 'should remove CSS class if selection is moved inside', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -108,8 +138,17 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
-			attachPlaceholder( view, element, 'new text' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
+
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'new text'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'new text' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -119,7 +158,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 			setData( view, '<p>paragraph</p>' );
 
 			view.forceRender();
@@ -136,8 +179,17 @@ describe( 'placeholder', () => {
 			setData( secondView, '<div></div><div>{another div}</div>' );
 			const secondElement = secondRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'first placeholder' );
-			attachPlaceholder( secondView, secondElement, 'second placeholder' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'first placeholder'
+			} );
+
+			enablePlaceholder( {
+				view: secondView,
+				element: secondElement,
+				text: 'second placeholder'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'first placeholder' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
@@ -165,7 +217,11 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			view.change( writer => {
 				writer.setSelection( ViewRange._createIn( element ) );
@@ -178,19 +234,53 @@ describe( 'placeholder', () => {
 			// After rendering - placeholder should be invisible since selection is moved there.
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should not set attributes/class when multiple children (isDirectHost=false)', () => {
+			setData( view, '<p></p><p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
+
+		it( 'should not set attributes/class when first child is not element (isDirectHost=false)', () => {
+			setData( view, '<ui:span></ui:span>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
 	} );
 
-	describe( 'detachPlaceholder', () => {
+	describe( 'disablePlaceholder', () => {
 		it( 'should remove placeholder from element', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			attachPlaceholder( view, element, 'foo bar baz' );
+			enablePlaceholder( {
+				view,
+				element,
+				text: 'foo bar baz'
+			} );
 
 			expect( element.getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
 
-			detachPlaceholder( view, element );
+			disablePlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
@@ -200,10 +290,130 @@ describe( 'placeholder', () => {
 			setData( view, '<div></div><div>{another div}</div>' );
 			const element = viewRoot.getChild( 0 );
 
-			detachPlaceholder( view, element );
+			disablePlaceholder( view, element );
 
 			expect( element.hasAttribute( 'data-placeholder' ) ).to.be.false;
 			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
 		} );
+
+		it( 'should remove placeholder from element (isDirectHost=false)', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			enablePlaceholder( {
+				view,
+				element: viewRoot,
+				text: 'foo bar baz',
+				isDirectHost: false
+			} );
+
+			expect( viewRoot.getChild( 0 ).getAttribute( 'data-placeholder' ) ).to.equal( 'foo bar baz' );
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.true;
+
+			disablePlaceholder( view, viewRoot );
+
+			expect( viewRoot.getChild( 0 ).hasAttribute( 'data-placeholder' ) ).to.be.false;
+			expect( viewRoot.getChild( 0 ).hasClass( 'ck-placeholder' ) ).to.be.false;
+		} );
+	} );
+
+	describe( 'showPlaceholder', () => {
+		it( 'should add the ck-placholder class if an element does not have it', () => {
+			setData( view, '<div></div>' );
+			const element = viewRoot.getChild( 0 );
+
+			const result = view.change( writer => showPlaceholder( writer, element ) );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
+			expect( result ).to.be.true;
+		} );
+
+		it( 'should do nothing if an element already has the ck-placeholder class', () => {
+			setData( view, '<div class="ck-placeholder"></div>' );
+			const element = viewRoot.getChild( 0 );
+			let spy;
+
+			const result = view.change( writer => {
+				spy = sinon.spy( writer, 'addClass' );
+
+				return showPlaceholder( writer, element );
+			} );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.true;
+			expect( result ).to.be.false;
+			sinon.assert.notCalled( spy );
+		} );
+	} );
+
+	describe( 'hidePlaceholder', () => {
+		it( 'should remove the ck-placholder class if an element has it', () => {
+			setData( view, '<div class="ck-placeholder"></div>' );
+			const element = viewRoot.getChild( 0 );
+
+			const result = view.change( writer => hidePlaceholder( writer, element ) );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( result ).to.be.true;
+		} );
+
+		it( 'should do nothing if an element has no ck-placeholder class', () => {
+			setData( view, '<div></div>' );
+			const element = viewRoot.getChild( 0 );
+			let spy;
+
+			const result = view.change( writer => {
+				spy = sinon.spy( writer, 'removeClass' );
+
+				return hidePlaceholder( writer, element );
+			} );
+
+			expect( element.hasClass( 'ck-placeholder' ) ).to.be.false;
+			expect( result ).to.be.false;
+			sinon.assert.notCalled( spy );
+		} );
+	} );
+
+	describe( 'needsPlaceholder', () => {
+		it( 'should return false if element was removed from the document', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+
+			view.change( writer => {
+				writer.remove( element );
+			} );
+
+			expect( needsPlaceholder( element ) ).to.be.false;
+		} );
+
+		it( 'should return true if element is empty and document is blurred', () => {
+			setData( view, '<p></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
+
+		it( 'should return true if element hosts UI elements only and document is blurred', () => {
+			setData( view, '<p><ui:span></ui:span></p>' );
+			viewDocument.isFocused = false;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
+
+		it( 'should return true when document is focused but selection anchored somewhere else', () => {
+			setData( view, '<p></p><p>{moo}</p>' );
+			viewDocument.isFocused = true;
+
+			const element = viewRoot.getChild( 0 );
+
+			expect( needsPlaceholder( element ) ).to.be.true;
+		} );
 	} );
 } );

+ 84 - 4
packages/ckeditor5-engine/tests/view/view/view.js

@@ -119,6 +119,19 @@ describe( 'view', () => {
 			expect( view._renderer.markedChildren.has( viewH1 ) ).to.be.true;
 		} );
 
+		it( 'should handle the "contenteditable" attribute management on #isReadOnly change', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			view.attachDomRoot( domDiv );
+
+			viewRoot.isReadOnly = false;
+			expect( viewRoot.getAttribute( 'contenteditable' ) ).to.equal( 'true' );
+
+			viewRoot.isReadOnly = true;
+			expect( viewRoot.getAttribute( 'contenteditable' ) ).to.equal( 'false' );
+		} );
+
 		it( 'should call observe on each observer', () => {
 			// The variable will be overwritten.
 			view.destroy();
@@ -144,6 +157,73 @@ describe( 'view', () => {
 		} );
 	} );
 
+	describe( 'detachDomRoot()', () => {
+		it( 'should remove DOM root and unbind DOM element', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			view.attachDomRoot( domDiv );
+			expect( count( view.domRoots ) ).to.equal( 1 );
+			expect( view.domConverter.mapViewToDom( viewRoot ) ).to.equal( domDiv );
+
+			view.detachDomRoot( 'main' );
+			expect( count( view.domRoots ) ).to.equal( 0 );
+			expect( view.domConverter.mapViewToDom( viewRoot ) ).to.be.undefined;
+
+			domDiv.remove();
+		} );
+
+		it( 'should restore the DOM root attributes to the state before attachDomRoot()', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			domDiv.setAttribute( 'foo', 'bar' );
+			domDiv.setAttribute( 'data-baz', 'qux' );
+			domDiv.classList.add( 'foo-class' );
+
+			view.attachDomRoot( domDiv );
+
+			view.change( writer => {
+				writer.addClass( 'addedClass', viewRoot );
+				writer.setAttribute( 'added-attribute', 'foo', viewRoot );
+				writer.setAttribute( 'foo', 'changed the value', viewRoot );
+			} );
+
+			view.detachDomRoot( 'main' );
+
+			const attributes = {};
+
+			for ( const attribute of domDiv.attributes ) {
+				attributes[ attribute.name ] = attribute.value;
+			}
+
+			expect( attributes ).to.deep.equal( {
+				foo: 'bar',
+				'data-baz': 'qux',
+				class: 'foo-class'
+			} );
+
+			domDiv.remove();
+		} );
+
+		it( 'should remove the "contenteditable" attribute from the DOM root', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			view.attachDomRoot( domDiv );
+			view.forceRender();
+
+			viewRoot.isReadOnly = false;
+			expect( domDiv.getAttribute( 'contenteditable' ) ).to.equal( 'true' );
+
+			view.detachDomRoot( 'main' );
+
+			expect( domDiv.hasAttribute( 'contenteditable' ) ).to.be.false;
+
+			domDiv.remove();
+		} );
+	} );
+
 	describe( 'addObserver()', () => {
 		beforeEach( () => {
 			// The variable will be overwritten.
@@ -260,6 +340,10 @@ describe( 'view', () => {
 
 			view.attachDomRoot( domRoot );
 
+			view.change( writer => {
+				writer.setSelection( range );
+			} );
+
 			// Make sure the window will have to scroll to the domRoot.
 			Object.assign( domRoot.style, {
 				position: 'absolute',
@@ -267,10 +351,6 @@ describe( 'view', () => {
 				left: '-1000px'
 			} );
 
-			view.change( writer => {
-				writer.setSelection( range );
-			} );
-
 			view.scrollToTheSelection();
 			sinon.assert.calledWithMatch( stub, sinon.match.number, sinon.match.number );
 		} );

+ 4 - 4
packages/ckeditor5-engine/theme/placeholder.css

@@ -6,9 +6,9 @@
 /* See ckeditor/ckeditor5#936. */
 .ck.ck-placeholder, .ck .ck-placeholder {
 	&::before {
-		 content: attr(data-placeholder);
+		content: attr(data-placeholder);
 
-		 /* See ckeditor/ckeditor5#469. */
-		 pointer-events: none;
-	 }
+		/* See ckeditor/ckeditor5#469. */
+		pointer-events: none;
+	}
 }