Merge branch 'master' into t/ckeditor5/1523

docs/framework/guides/deep-dive/schema.md

@@ -57,7 +57,8 @@ schema.register( '$block', {
 	isBlock: true
 } );
 schema.register( '$text', {
-	allowIn: '$block'
+	allowIn: '$block',
+	isInline: true
 } );
 ```
 

src/conversion/mapper.js

@@ -91,14 +91,8 @@ export default class Mapper {
 				return;
 			}
 
-			let viewBlock = data.viewPosition.parent;
-			let modelParent = this._viewToModelMapping.get( viewBlock );
-
-			while ( !modelParent ) {
-				viewBlock = viewBlock.parent;
-				modelParent = this._viewToModelMapping.get( viewBlock );
-			}
-
+			const viewBlock = this.findMappedViewAncestor( data.viewPosition );
+			const modelParent = this._viewToModelMapping.get( viewBlock );
 			const modelOffset = this._toModelOffset( data.viewPosition.parent, data.viewPosition.offset, viewBlock );
 
 			data.modelPosition = ModelPosition._createAt( modelParent, modelOffset );
@@ -338,6 +332,23 @@ export default class Mapper {
 		this._viewToModelLengthCallbacks.set( viewElementName, lengthCallback );
 	}
 
+	/**
+	 * For given `viewPosition`, finds and returns the closest ancestor of this position that has a mapping to
+	 * the model.
+	 *
+	 * @param {module:engine/view/position~Position} viewPosition Position for which mapped ancestor should be found.
+	 * @returns {module:engine/view/element~Element}
+	 */
+	findMappedViewAncestor( viewPosition ) {
+		let parent = viewPosition.parent;
+
+		while ( !this._viewToModelMapping.has( parent ) ) {
+			parent = parent.parent;
+		}
+
+		return parent;
+	}
+
 	/**
 	 * Calculates model offset based on the view position and the block element.
 	 *

src/model/model.js

@@ -94,7 +94,8 @@ export default class Model {
 			isBlock: true
 		} );
 		this.schema.register( '$text', {
-			allowIn: '$block'
+			allowIn: '$block',
+			isInline: true
 		} );
 		this.schema.register( '$clipboardHolder', {
 			allowContentOf: '$root',

src/model/schema.js

@@ -230,7 +230,7 @@ export default class Schema {
 
 	/**
 	 * Returns `true` if the given item is defined to be
-	 * a object element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isObject` property.
+	 * an object element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isObject` property.
 	 *
 	 *		schema.isObject( 'paragraph' ); // -> false
 	 *		schema.isObject( 'image' ); // -> true
@@ -246,6 +246,24 @@ export default class Schema {
 		return !!( def && def.isObject );
 	}
 
+	/**
+	 * Returns `true` if the given item is defined to be
+	 * an inline element by {@link module:engine/model/schema~SchemaItemDefinition}'s `isInline` property.
+	 *
+	 *		schema.isInline( 'paragraph' ); // -> false
+	 *		schema.isInline( 'softBreak' ); // -> true
+	 *
+	 *		const text = writer.createText('foo' );
+	 *		schema.isInline( text ); // -> true
+	 *
+	 * @param {module:engine/model/item~Item|module:engine/model/schema~SchemaContextItem|String} item
+	 */
+	isInline( item ) {
+		const def = this.getDefinition( item );
+
+		return !!( def && def.isInline );
+	}
+
 	/**
 	 * Checks whether the given node (`child`) can be a child of the given context.
 	 *
@@ -899,7 +917,7 @@ mix( Schema, ObservableMixin );
  * * `allowAttributesOf` – A string or an array of strings. Inherits attributes from other items.
  * * `inheritTypesFrom` – A string or an array of strings. Inherits `is*` properties of other items.
  * * `inheritAllFrom` – A string. A shorthand for `allowContentOf`, `allowWhere`, `allowAttributesOf`, `inheritTypesFrom`.
- * * Additionally, you can define the following `is*` properties: `isBlock`, `isLimit`, `isObject`. Read about them below.
+ * * Additionally, you can define the following `is*` properties: `isBlock`, `isLimit`, `isObject`, `isInline`. Read about them below.
  *
  * # The is* properties
  *
@@ -915,8 +933,9 @@ mix( Schema, ObservableMixin );
  * a limit element are limited to its content. **Note:** All objects (`isObject`) are treated as limit elements, too.
  * * `isObject` – Whether an item is "self-contained" and should be treated as a whole. Examples of object elements:
  * `image`, `table`, `video`, etc. **Note:** An object is also a limit, so
- * {@link module:engine/model/schema~Schema#isLimit `isLimit()`}
- * returns `true` for object elements automatically.
+ * {@link module:engine/model/schema~Schema#isLimit `isLimit()`} returns `true` for object elements automatically.
+ * * `isInline` – Whether an item is "text-like" and should be treated as an inline node. Examples of inline elements:
+ * `$text`, `softBreak` (`<br>`), etc.
  *
  * # Generic items
  *
@@ -931,7 +950,8 @@ mix( Schema, ObservableMixin );
  *			isBlock: true
  *		} );
  *		this.schema.register( '$text', {
- *			allowIn: '$block'
+ *			allowIn: '$block',
+ *			isInline: true
  *		} );
  *
  * They reflect typical editor content that is contained within one root, consists of several blocks

src/view/selection.js

@@ -414,8 +414,26 @@ export default class Selection {
 		}
 
 		const range = this.getFirstRange();
-		const nodeAfterStart = range.start.nodeAfter;
-		const nodeBeforeEnd = range.end.nodeBefore;
+
+		let nodeAfterStart = range.start.nodeAfter;
+		let nodeBeforeEnd = range.end.nodeBefore;
+
+		// Handle the situation when selection position is at the beginning / at the end of a text node.
+		// In such situation `.nodeAfter` and `.nodeBefore` are `null` but the selection still might be spanning
+		// over one element.
+		//
+		// <p>Foo{<span class="widget"></span>}bar</p> vs <p>Foo[<span class="widget"></span>]bar</p>
+		//
+		// These are basically the same selections, only the difference is if the selection position is at
+		// at the end/at the beginning of a text node or just before/just after the text node.
+		//
+		if ( range.start.parent.is( 'text' ) && range.start.isAtEnd && range.start.parent.nextSibling ) {
+			nodeAfterStart = range.start.parent.nextSibling;
+		}
+
+		if ( range.end.parent.is( 'text' ) && range.end.isAtStart && range.end.parent.previousSibling ) {
+			nodeBeforeEnd = range.end.parent.previousSibling;
+		}
 
 		return ( nodeAfterStart instanceof Element && nodeAfterStart == nodeBeforeEnd ) ? nodeAfterStart : null;
 	}

src/view/view.js

@@ -88,6 +88,14 @@ export default class View {
 		 */
 		this.domRoots = new Map();
 
+		/**
+		 * Used to prevent calling {@link #forceRender} and {@link #change} during rendering view to the DOM.
+		 *
+		 * @readonly
+		 * @member {Boolean} #isRenderingInProgress
+		 */
+		this.set( 'isRenderingInProgress', false );
+
 		/**
 		 * Instance of the {@link module:engine/view/renderer~Renderer renderer}.
 		 *
@@ -124,14 +132,6 @@ export default class View {
 		 */
 		this._ongoingChange = false;
 
-		/**
-		 * Used to prevent calling {@link #forceRender} and {@link #change} during rendering view to the DOM.
-		 *
-		 * @private
-		 * @type {Boolean}
-		 */
-		this._renderingInProgress = false;
-
 		/**
 		 * Used to prevent calling {@link #forceRender} and {@link #change} during rendering view to the DOM.
 		 *
@@ -240,6 +240,12 @@ export default class View {
 
 		const updateContenteditableAttribute = () => {
 			this._writer.setAttribute( 'contenteditable', !viewRoot.isReadOnly, viewRoot );
+
+			if ( viewRoot.isReadOnly ) {
+				this._writer.addClass( 'ck-read-only', viewRoot );
+			} else {
+				this._writer.removeClass( 'ck-read-only', viewRoot );
+			}
 		};
 
 		// Set initial value.
@@ -428,7 +434,7 @@ export default class View {
 	 * @returns {*} Value returned by the callback.
 	 */
 	change( callback ) {
-		if ( this._renderingInProgress || this._postFixersInProgress ) {
+		if ( this.isRenderingInProgress || this._postFixersInProgress ) {
 			/**
 			 * 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.
@@ -662,11 +668,11 @@ export default class View {
 	 * @private
 	 */
 	_render() {
-		this._renderingInProgress = true;
+		this.isRenderingInProgress = true;
 		this.disableObservers();
 		this._renderer.render();
 		this.enableObservers();
-		this._renderingInProgress = false;
+		this.isRenderingInProgress = false;
 	}
 
 	/**

tests/conversion/mapper.js

@@ -721,4 +721,29 @@ describe( 'Mapper', () => {
 			expect( mapper.getModelLength( viewDiv ) ).to.equal( 6 );
 		} );
 	} );
+
+	describe( 'findMappedViewAncestor()', () => {
+		it( 'should return for given view position the closest ancestor which is mapped to a model element', () => {
+			const mapper = new Mapper();
+
+			const modelP = new ModelElement( 'p' );
+			const modelDiv = new ModelElement( 'div' );
+
+			const viewText = new ViewText( 'foo' );
+			const viewSpan = new ViewElement( 'span', null, viewText );
+			const viewP = new ViewElement( 'p', null, viewSpan );
+			const viewDiv = new ViewElement( 'div', null, viewP );
+
+			mapper.bindElements( modelP, viewP );
+			mapper.bindElements( modelDiv, viewDiv );
+
+			// <div><p><span>f{}oo</span></p></div>
+
+			const viewPosition = new ViewPosition( viewText, 1 );
+
+			const viewMappedAncestor = mapper.findMappedViewAncestor( viewPosition );
+
+			expect( viewMappedAncestor ).to.equal( viewP );
+		} );
+	} );
 } );

tests/model/schema.js

@@ -376,6 +376,41 @@ describe( 'Schema', () => {
 		} );
 	} );
 
+	describe( 'isInline()', () => {
+		it( 'returns true if an item was registered as inline', () => {
+			schema.register( 'foo', {
+				isInline: true
+			} );
+
+			expect( schema.isInline( 'foo' ) ).to.be.true;
+		} );
+
+		it( 'returns false if an item was registered as a limit (because not all limits are objects)', () => {
+			schema.register( 'foo', {
+				isLimit: true
+			} );
+
+			expect( schema.isInline( 'foo' ) ).to.be.false;
+		} );
+
+		it( 'returns false if an item was not registered as an object', () => {
+			schema.register( 'foo' );
+
+			expect( schema.isInline( 'foo' ) ).to.be.false;
+		} );
+
+		it( 'returns false if an item was not registered at all', () => {
+			expect( schema.isInline( 'foo' ) ).to.be.false;
+		} );
+
+		it( 'uses getDefinition()\'s item to definition normalization', () => {
+			const stub = sinon.stub( schema, 'getDefinition' ).returns( { isInline: true } );
+
+			expect( schema.isInline( 'foo' ) ).to.be.true;
+			expect( stub.calledOnce ).to.be.true;
+		} );
+	} );
+
 	describe( 'checkChild()', () => {
 		beforeEach( () => {
 			schema.register( '$root' );
@@ -2468,7 +2503,8 @@ describe( 'Schema', () => {
 			},
 			() => {
 				schema.extend( '$text', {
-					allowAttributes: [ 'bold', 'italic' ]
+					allowAttributes: [ 'bold', 'italic' ],
+					isInline: true
 				} );
 
 				// Disallow bold in heading1.
@@ -2494,7 +2530,8 @@ describe( 'Schema', () => {
 				isBlock: true
 			} );
 			schema.register( '$text', {
-				allowIn: '$block'
+				allowIn: '$block',
+				isInline: true
 			} );
 
 			for ( const definition of definitions ) {
@@ -2738,40 +2775,53 @@ describe( 'Schema', () => {
 			expect( schema.checkAttribute( r1i, 'alignment' ) ).to.be.false;
 		} );
 
+		it( '$text is inline', () => {
+			expect( schema.isLimit( '$text' ) ).to.be.false;
+			expect( schema.isBlock( '$text' ) ).to.be.false;
+			expect( schema.isObject( '$text' ) ).to.be.false;
+			expect( schema.isInline( '$text' ) ).to.be.true;
+		} );
+
 		it( '$root is limit', () => {
 			expect( schema.isLimit( '$root' ) ).to.be.true;
 			expect( schema.isBlock( '$root' ) ).to.be.false;
 			expect( schema.isObject( '$root' ) ).to.be.false;
+			expect( schema.isInline( '$root' ) ).to.be.false;
 		} );
 
 		it( 'paragraph is block', () => {
 			expect( schema.isLimit( 'paragraph' ) ).to.be.false;
 			expect( schema.isBlock( 'paragraph' ) ).to.be.true;
 			expect( schema.isObject( 'paragraph' ) ).to.be.false;
+			expect( schema.isInline( 'paragraph' ) ).to.be.false;
 		} );
 
 		it( 'heading1 is block', () => {
 			expect( schema.isLimit( 'heading1' ) ).to.be.false;
 			expect( schema.isBlock( 'heading1' ) ).to.be.true;
 			expect( schema.isObject( 'heading1' ) ).to.be.false;
+			expect( schema.isInline( 'heading1' ) ).to.be.false;
 		} );
 
 		it( 'listItem is block', () => {
 			expect( schema.isLimit( 'listItem' ) ).to.be.false;
 			expect( schema.isBlock( 'listItem' ) ).to.be.true;
 			expect( schema.isObject( 'listItem' ) ).to.be.false;
+			expect( schema.isInline( 'lisItem' ) ).to.be.false;
 		} );
 
 		it( 'image is block object', () => {
 			expect( schema.isLimit( 'image' ) ).to.be.true;
 			expect( schema.isBlock( 'image' ) ).to.be.true;
 			expect( schema.isObject( 'image' ) ).to.be.true;
+			expect( schema.isInline( 'image' ) ).to.be.false;
 		} );
 
 		it( 'caption is limit', () => {
 			expect( schema.isLimit( 'caption' ) ).to.be.true;
 			expect( schema.isBlock( 'caption' ) ).to.be.false;
 			expect( schema.isObject( 'caption' ) ).to.be.false;
+			expect( schema.isInline( 'caption' ) ).to.be.false;
 		} );
 	} );