diff --git a/docs/framework/guides/deep-dive/schema.md b/docs/framework/guides/deep-dive/schema.md index f3e06c6e0..8222e2391 100644 --- a/docs/framework/guides/deep-dive/schema.md +++ b/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 } ); ``` diff --git a/src/conversion/mapper.js b/src/conversion/mapper.js index 59be44666..3eb7c38c6 100644 --- a/src/conversion/mapper.js +++ b/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. * diff --git a/src/model/model.js b/src/model/model.js index 3c970ab29..500de84ae 100644 --- a/src/model/model.js +++ b/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', diff --git a/src/model/schema.js b/src/model/schema.js index a4a4f8b40..f1fdc4ad5 100644 --- a/src/model/schema.js +++ b/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` (`
`), 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 diff --git a/src/view/selection.js b/src/view/selection.js index 5a1e7fb45..8488029df 100644 --- a/src/view/selection.js +++ b/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. + // + //

Foo{}bar

vs

Foo[]bar

+ // + // 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; } diff --git a/src/view/view.js b/src/view/view.js index 255ee5e69..ad3600865 100644 --- a/src/view/view.js +++ b/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; } /** diff --git a/tests/conversion/mapper.js b/tests/conversion/mapper.js index e3f6fa0ef..e84cb2c88 100644 --- a/tests/conversion/mapper.js +++ b/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 ); + + //

f{}oo

+ + const viewPosition = new ViewPosition( viewText, 1 ); + + const viewMappedAncestor = mapper.findMappedViewAncestor( viewPosition ); + + expect( viewMappedAncestor ).to.equal( viewP ); + } ); + } ); } ); diff --git a/tests/model/schema.js b/tests/model/schema.js index 2b7b53eb3..a01a6e54b 100644 --- a/tests/model/schema.js +++ b/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; } ); } ); diff --git a/tests/model/utils/selection-post-fixer.js b/tests/model/utils/selection-post-fixer.js index 328978ade..9ce86a5a2 100644 --- a/tests/model/utils/selection-post-fixer.js +++ b/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, 'aaa[]bbb' ); + + expect( getModelData( model ) ).to.equal( 'aaa[]bbb' ); + } ); + + it( 'should fix selection that starts in inline element', () => { + setModelData( model, 'aaa[]bbb' ); + + expect( getModelData( model ) ).to.equal( 'aaa[]bbb' ); + } ); + + it( 'should fix selection that ends in inline element that is also an object', () => { + model.schema.extend( 'placeholder', { + isObject: true + } ); + + setModelData( model, 'aaa[]bbb' ); + + expect( getModelData( model ) ).to.equal( 'aaa[]bbb' ); + } ); + + it( 'should fix selection that starts in inline element that is also an object', () => { + model.schema.extend( 'placeholder', { + isObject: true + } ); + + setModelData( model, 'aaa[]bbb' ); + + expect( getModelData( model ) ).to.equal( 'aaa[]bbb' ); + } ); + } ); + describe( 'collapsed selection', () => { beforeEach( () => { setModelData( model, diff --git a/tests/view/selection.js b/tests/view/selection.js index 6e08762a2..207fb866b 100644 --- a/tests/view/selection.js +++ b/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 {bar} 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 [bar] [baz]' ); const selection = new Selection( docSelection ); diff --git a/tests/view/view/view.js b/tests/view/view/view.js index 3fe6a09b1..cceabbe3e 100644 --- a/tests/view/view/view.js +++ b/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 );