8
0
Просмотр исходного кода

Merge branch 'master' into t/ckeditor5/1540

Piotrek Koszuliński 6 лет назад
Родитель
Сommit
0c3d34c66d

+ 2 - 1
packages/ckeditor5-engine/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
 } );
 ```
 

+ 19 - 8
packages/ckeditor5-engine/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 );
@@ -339,6 +333,23 @@ export default class Mapper {
 	}
 
 	/**
+	 * 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.
 	 *
 	 * Example:

+ 2 - 1
packages/ckeditor5-engine/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',

+ 25 - 5
packages/ckeditor5-engine/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
@@ -247,6 +247,24 @@ export default class Schema {
 	}
 
 	/**
+	 * 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.
 	 *
 	 *		schema.checkChild( model.document.getRoot(), paragraph ); // -> false
@@ -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

+ 1 - 2
packages/ckeditor5-engine/src/view/renderer.js

@@ -707,8 +707,7 @@ export default class Renderer {
 			container.appendChild( domDocument.createTextNode( '\u00A0' ) );
 		}
 
-		// Add fake container if not already added.
-		if ( !container.parentElement ) {
+		if ( !container.parentElement || container.parentElement != domRoot ) {
 			domRoot.appendChild( container );
 		}
 

+ 20 - 2
packages/ckeditor5-engine/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;
 	}

+ 17 - 11
packages/ckeditor5-engine/src/view/view.js

@@ -89,6 +89,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}.
 		 *
 		 * @protected
@@ -130,14 +138,6 @@ export default class View {
 		 * @private
 		 * @type {Boolean}
 		 */
-		this._renderingInProgress = false;
-
-		/**
-		 * Used to prevent calling {@link #forceRender} and {@link #change} during rendering view to the DOM.
-		 *
-		 * @private
-		 * @type {Boolean}
-		 */
 		this._postFixersInProgress = false;
 
 		/**
@@ -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;
 	}
 
 	/**

+ 25 - 0
packages/ckeditor5-engine/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 );
+		} );
+	} );
 } );

+ 52 - 2
packages/ckeditor5-engine/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;
 		} );
 	} );
 

+ 41 - 0
packages/ckeditor5-engine/tests/model/utils/selection-post-fixer.js

@@ -945,6 +945,47 @@ describe( 'Selection post-fixer', () => {
 			} );
 		} );
 
+		describe( 'non-collapsed selection - inline widget scenarios', () => {
+			beforeEach( () => {
+				model.schema.register( 'placeholder', {
+					allowWhere: '$text',
+					isInline: true
+				} );
+			} );
+
+			it( 'should fix selection that ends in inline element', () => {
+				setModelData( model, '<paragraph>aaa[<placeholder>]</placeholder>bbb</paragraph>' );
+
+				expect( getModelData( model ) ).to.equal( '<paragraph>aaa[]<placeholder></placeholder>bbb</paragraph>' );
+			} );
+
+			it( 'should fix selection that starts in inline element', () => {
+				setModelData( model, '<paragraph>aaa<placeholder>[</placeholder>]bbb</paragraph>' );
+
+				expect( getModelData( model ) ).to.equal( '<paragraph>aaa<placeholder></placeholder>[]bbb</paragraph>' );
+			} );
+
+			it( 'should fix selection that ends in inline element that is also an object', () => {
+				model.schema.extend( 'placeholder', {
+					isObject: true
+				} );
+
+				setModelData( model, '<paragraph>aaa[<placeholder>]</placeholder>bbb</paragraph>' );
+
+				expect( getModelData( model ) ).to.equal( '<paragraph>aaa[<placeholder></placeholder>]bbb</paragraph>' );
+			} );
+
+			it( 'should fix selection that starts in inline element that is also an object', () => {
+				model.schema.extend( 'placeholder', {
+					isObject: true
+				} );
+
+				setModelData( model, '<paragraph>aaa<placeholder>[</placeholder>]bbb</paragraph>' );
+
+				expect( getModelData( model ) ).to.equal( '<paragraph>aaa[<placeholder></placeholder>]bbb</paragraph>' );
+			} );
+		} );
+
 		describe( 'collapsed selection', () => {
 			beforeEach( () => {
 				setModelData( model,

+ 36 - 19
packages/ckeditor5-engine/tests/view/renderer.js

@@ -7,6 +7,7 @@
 
 import View from '../../src/view/view';
 import ViewElement from '../../src/view/element';
+import ViewEditableElement from '../../src/view/editableelement';
 import ViewContainerElement from '../../src/view/containerelement';
 import ViewAttributeElement from '../../src/view/attributeelement';
 import ViewText from '../../src/view/text';
@@ -112,10 +113,12 @@ describe( 'Renderer', () => {
 	} );
 
 	describe( 'render', () => {
-		let viewRoot, domRoot, selectionEditable;
+		let viewRoot, domRoot;
 
 		beforeEach( () => {
-			viewRoot = new ViewElement( 'div' );
+			viewRoot = new ViewEditableElement( 'div' );
+			viewRoot.getFillerOffset = () => null;
+
 			domRoot = document.createElement( 'div' );
 			document.body.appendChild( domRoot );
 
@@ -127,16 +130,7 @@ describe( 'Renderer', () => {
 
 			selection._setTo( null );
 
-			selectionEditable = viewRoot;
-
 			renderer.isFocused = true;
-
-			// Fake selection editable - it is needed to render selection properly.
-			Object.defineProperty( selection, 'editableElement', {
-				get() {
-					return selectionEditable;
-				}
-			} );
 		} );
 
 		afterEach( () => {
@@ -422,8 +416,6 @@ describe( 'Renderer', () => {
 		} );
 
 		it( 'should not care about filler if there is no DOM', () => {
-			selectionEditable = null;
-
 			const { view: viewP, selection: newSelection } = parse(
 				'<container:p>foo<attribute:b>[]</attribute:b>bar</container:p>' );
 
@@ -1202,11 +1194,10 @@ describe( 'Renderer', () => {
 			domSelection.removeAllRanges();
 			domSelection.collapse( domDiv, 0 );
 
-			selectionEditable = null;
-
+			const viewDiv = new ViewElement( 'div' );
 			const { view: viewP, selection: newSelection } = parse( '<container:p>fo{o}</container:p>' );
 
-			viewRoot._appendChild( viewP );
+			viewDiv._appendChild( viewP );
 			selection._setTo( newSelection );
 
 			renderer.render();
@@ -1353,13 +1344,13 @@ describe( 'Renderer', () => {
 		} );
 
 		it( 'should handle focusing element', () => {
+			selection._setTo( viewRoot, 0 );
+
 			const domFocusSpy = testUtils.sinon.spy( domRoot, 'focus' );
-			const editable = selection.editableElement;
 
 			renderer.render();
 
-			expect( editable ).to.equal( viewRoot );
-			expect( domFocusSpy.calledOnce ).to.be.true;
+			expect( domFocusSpy.called ).to.be.true;
 		} );
 
 		it( 'should not focus editable if isFocues is set to false', () => {
@@ -1928,6 +1919,32 @@ describe( 'Renderer', () => {
 				expect( container.style.left ).to.equal( '-9999px' );
 			} );
 
+			it( 'should move fake selection container between editables', () => {
+				const viewEditable = new ViewEditableElement( 'div' );
+				viewEditable._appendChild( parse( '<container:p>abc xyz</container:p>' ) );
+
+				const domEditable = document.createElement( 'div' );
+
+				document.body.appendChild( domEditable );
+
+				domConverter.bindElements( domEditable, viewEditable );
+
+				renderer.markToSync( 'children', viewEditable );
+				selection._setTo( selection.getRanges(), { fake: true, label: 'fake selection' } );
+				renderer.render();
+
+				let container = document.getSelection().anchorNode;
+
+				expect( domRoot.contains( container ) ).to.be.true;
+
+				selection._setTo( viewEditable, 'in', { fake: true, label: 'fake selection' } );
+				renderer.render();
+
+				container = document.getSelection().anchorNode;
+
+				expect( domEditable.contains( container ) ).to.be.true;
+			} );
+
 			it( 'should bind fake selection container to view selection', () => {
 				selection._setTo( selection.getRanges(), { fake: true, label: 'fake selection' } );
 				renderer.render();

+ 8 - 0
packages/ckeditor5-engine/tests/view/selection.js

@@ -976,6 +976,14 @@ describe( 'Selection', () => {
 			expect( selection.getSelectedElement() ).to.equal( b );
 		} );
 
+		it( 'should return selected element if the selection is anchored at the end/at the beginning of a text node', () => {
+			const { selection: docSelection, view } = parse( 'foo {<b>bar</b>} baz' );
+			const b = view.getChild( 1 );
+			const selection = new Selection( docSelection );
+
+			expect( selection.getSelectedElement() ).to.equal( b );
+		} );
+
 		it( 'should return null if there is more than one range', () => {
 			const { selection: docSelection } = parse( 'foo [<b>bar</b>] [<i>baz</i>]' );
 			const selection = new Selection( docSelection );

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

@@ -132,6 +132,19 @@ describe( 'view', () => {
 			expect( viewRoot.getAttribute( 'contenteditable' ) ).to.equal( 'false' );
 		} );
 
+		it( 'should handle the ".ck-read-only" class management on #isReadOnly change', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			view.attachDomRoot( domDiv );
+
+			viewRoot.isReadOnly = false;
+			expect( viewRoot.hasClass( 'ck-read-only' ) ).to.be.false;
+
+			viewRoot.isReadOnly = true;
+			expect( viewRoot.hasClass( 'ck-read-only' ) ).to.be.true;
+		} );
+
 		it( 'should call observe on each observer', () => {
 			// The variable will be overwritten.
 			view.destroy();
@@ -222,6 +235,23 @@ describe( 'view', () => {
 
 			domDiv.remove();
 		} );
+
+		it( 'should remove the ".ck-read-only" class from the DOM root', () => {
+			const domDiv = document.createElement( 'div' );
+			const viewRoot = createViewRoot( viewDocument, 'div', 'main' );
+
+			view.attachDomRoot( domDiv );
+			view.forceRender();
+
+			viewRoot.isReadOnly = true;
+			expect( domDiv.classList.contains( 'ck-read-only' ) ).to.be.true;
+
+			view.detachDomRoot( 'main' );
+
+			expect( domDiv.classList.contains( 'ck-read-only' ) ).to.be.false;
+
+			domDiv.remove();
+		} );
 	} );
 
 	describe( 'addObserver()', () => {
@@ -457,6 +487,22 @@ describe( 'view', () => {
 		} );
 	} );
 
+	describe( 'isRenderingInProgress', () => {
+		it( 'should be true while rendering is in progress', () => {
+			expect( view.isRenderingInProgress ).to.equal( false );
+
+			const spy = sinon.spy();
+
+			view.on( 'change:isRenderingInProgress', spy );
+
+			view.fire( 'render' );
+
+			sinon.assert.calledTwice( spy );
+			sinon.assert.calledWith( spy.firstCall, sinon.match.any, 'isRenderingInProgress', true );
+			sinon.assert.calledWith( spy.secondCall, sinon.match.any, 'isRenderingInProgress', false );
+		} );
+	} );
+
 	describe( 'forceRender()', () => {
 		it( 'disable observers, renders and enable observers', () => {
 			const observerMock = view.addObserver( ObserverMock );