Bladeren bron

Merge branch 'master' into t/661

Piotrek Koszuliński 9 jaren geleden
bovenliggende
commit
09083cc1e1

+ 1 - 1
packages/ckeditor5-engine/package.json

@@ -1,6 +1,6 @@
 {
   "name": "ckeditor5-engine",
-  "version": "0.3.0",
+  "version": "0.4.0",
   "description": "CKEditor 5 Editing Engine",
   "keywords": [
     "CKEditor"

+ 1 - 1
packages/ckeditor5-engine/src/controller/insert.js

@@ -291,7 +291,7 @@ class Insertion {
 
 			// OK:  <p>xx[]</p> + <p>yy</p> => <p>xx[]yy</p> (when sticks to previous)
 			// NOK: <p>xx[]</p> + <p>yy</p> => <p>xxyy[]</p> (when sticks to next)
-			position = new LivePosition( this.position.root, this.position.path, 'STICKS_TO_PREVIOUS' );
+			position = new LivePosition( this.position.root, this.position.path, 'sticksToPrevious' );
 
 			this.batch.merge( mergePosRight );
 

+ 24 - 10
packages/ckeditor5-engine/src/model/liveposition.js

@@ -30,7 +30,7 @@ export default class LivePosition extends Position {
 	 * @see engine.model.Position
 	 * @param {engine.model.RootElement} root
 	 * @param {Array.<Number>} path
-	 * @param {engine.model.PositionStickiness} [stickiness] Defaults to `'STICKS_TO_NEXT'`.
+	 * @param {engine.model.PositionStickiness} [stickiness] Defaults to `'sticksToNext'`.
 	 * See {@link engine.model.LivePosition#stickiness}.
 	 */
 	constructor( root, path, stickiness ) {
@@ -65,7 +65,7 @@ export default class LivePosition extends Position {
 		 *
 		 * @member {engine.model.PositionStickiness} engine.model.LivePosition#stickiness
 		 */
-		this.stickiness = stickiness || 'STICKS_TO_NEXT';
+		this.stickiness = stickiness || 'sticksToNext';
 
 		bindWithDocument.call( this );
 	}
@@ -111,6 +111,13 @@ export default class LivePosition extends Position {
 	 * @param {engine.model.Position} position
 	 * @returns {engine.model.LivePosition}
 	 */
+
+	/**
+	 * Fired when `LivePosition` instance is changed due to changes on {@link engine.model.Document}.
+	 *
+	 * @event engine.model.LivePosition#change
+	 * @param {engine.model.Position} oldPosition Position equal to this live position before it got changed.
+	 */
 }
 
 /**
@@ -133,7 +140,8 @@ function bindWithDocument() {
 			if ( supportedTypes.has( type ) ) {
 				transform.call( this, type, changes.range, changes.sourcePosition );
 			}
-		}
+		},
+		{ priority: 'high' }
 	);
 }
 
@@ -155,7 +163,7 @@ function transform( type, range, position ) {
 
 	switch ( type ) {
 		case 'insert':
-			let insertBefore = this.stickiness == 'STICKS_TO_NEXT';
+			let insertBefore = this.stickiness == 'sticksToNext';
 			transformed = this._getTransformedByInsertion( range.start, howMany, insertBefore );
 			break;
 
@@ -165,28 +173,34 @@ function transform( type, range, position ) {
 			let originalRange = Range.createFromPositionAndShift( position, howMany );
 
 			let gotMoved = originalRange.containsPosition( this ) ||
-				( originalRange.start.isEqual( this ) && this.stickiness == 'STICKS_TO_NEXT' ) ||
-				( originalRange.end.isEqual( this ) && this.stickiness == 'STICKS_TO_PREVIOUS' );
+				( originalRange.start.isEqual( this ) && this.stickiness == 'sticksToNext' ) ||
+				( originalRange.end.isEqual( this ) && this.stickiness == 'sticksToPrevious' );
 
 			// We can't use ._getTransformedByMove() because we have a different if-condition.
 			if ( gotMoved ) {
 				transformed = this._getCombined( position, range.start );
 			} else {
-				let insertBefore = this.stickiness == 'STICKS_TO_NEXT';
+				let insertBefore = this.stickiness == 'sticksToNext';
 				transformed = this._getTransformedByMove( position, range.start, howMany, insertBefore );
 			}
 			break;
 	}
 
-	this.path = transformed.path;
-	this.root = transformed.root;
+	if ( !this.isEqual( transformed ) ) {
+		const oldPosition = Position.createFromPosition( this );
+
+		this.path = transformed.path;
+		this.root = transformed.root;
+
+		this.fire( 'change', oldPosition );
+	}
 }
 
 mix( LivePosition, EmitterMixin );
 
 /**
  * Enum representing how position is "sticking" with their neighbour nodes.
- * Possible values: `'STICKS_TO_NEXT'`, `'STICKS_TO_PREVIOUS'`.
+ * Possible values: `'sticksToNext'`, `'sticksToPrevious'`.
  *
  * @typedef {String} engine.model.PositionStickiness
  */

+ 7 - 2
packages/ckeditor5-engine/src/model/liverange.js

@@ -77,6 +77,8 @@ export default class LiveRange extends Range {
 	 * Fired when `LiveRange` instance is changed due to changes on {@link engine.model.Document}.
 	 *
 	 * @event engine.model.LiveRange#change
+	 * @param {engine.model.Range} oldRange Range with start and end position equal to start and end position of this live range
+	 * before it got changed.
 	 */
 }
 
@@ -99,7 +101,8 @@ function bindWithDocument() {
 			if ( supportedTypes.has( type ) ) {
 				transform.call( this, type, changes.range, changes.sourcePosition );
 			}
-		}
+		},
+		{ priority: 'high' }
 	);
 }
 
@@ -158,10 +161,12 @@ function transform( type, range, position ) {
 
 	// If anything changed, update the range and fire an event.
 	if ( !updated.start.isEqual( this.start ) || !updated.end.isEqual( this.end ) ) {
+		const oldRange = Range.createFromRange( this );
+
 		this.start = updated.start;
 		this.end = updated.end;
 
-		this.fire( 'change' );
+		this.fire( 'change', oldRange );
 	}
 }
 

+ 95 - 11
packages/ckeditor5-engine/src/model/liveselection.js

@@ -3,12 +3,14 @@
  * For licensing, see LICENSE.md.
  */
 
+import Position from './position.js';
 import Range from './range.js';
 import LiveRange from './liverange.js';
 import Text from './text.js';
 import TextProxy from './textproxy.js';
 import toMap from '../../utils/tomap.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
+import log from '../../utils/log.js';
 
 import Selection from './selection.js';
 
@@ -63,14 +65,6 @@ export default class LiveSelection extends Selection {
 		 */
 		this._attributePriority = new Map();
 
-		// Whenever selection range changes, if the change comes directly from selection API (direct user change).
-		this.on( 'change:range', ( evt, data ) => {
-			if ( data.directChange ) {
-				// Reset attributes on selection (clear attributes and priorities) and get attributes from surrounding nodes.
-				this._updateAttributes( true );
-			}
-		}, { priority: 'high' } );
-
 		// Whenever attribute operation is performed on document, update attributes. This is not the most efficient
 		// way to update selection attributes, but should be okay for now. `_updateAttributes` will be fired too often,
 		// but it won't change attributes or fire `change:attribute` event if not needed.
@@ -150,6 +144,30 @@ export default class LiveSelection extends Selection {
 	/**
 	 * @inheritDoc
 	 */
+	addRange( range, isBackward = false ) {
+		super.addRange( range, isBackward );
+		this.refreshAttributes();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	removeAllRanges() {
+		super.removeAllRanges();
+		this.refreshAttributes();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
+	setRanges( newRanges, isLastBackward = false ) {
+		super.setRanges( newRanges, isLastBackward );
+		this.refreshAttributes();
+	}
+
+	/**
+	 * @inheritDoc
+	 */
 	setAttribute( key, value ) {
 		// Store attribute in parent element if the selection is collapsed in an empty node.
 		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
@@ -206,6 +224,13 @@ export default class LiveSelection extends Selection {
 	}
 
 	/**
+	 * Removes all attributes from the selection and sets attributes according to the surrounding nodes.
+	 */
+	refreshAttributes() {
+		this._updateAttributes( true );
+	}
+
+	/**
 	 * Creates and returns an instance of `LiveSelection` that is a clone of given selection, meaning that it has same
 	 * ranges and same direction as this selection.
 	 *
@@ -230,22 +255,58 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	_pushRange( range ) {
+		const liveRange = this._prepareRange( range );
+
+		// `undefined` is returned when given `range` is in graveyard root.
+		if ( liveRange ) {
+			this._ranges.push( liveRange );
+		}
+	}
+
+	/**
+	 * Prepares given range to be added to selection. Checks if it is correct, converts it to {@link engine.model.LiveRange LiveRange}
+	 * and sets listeners listening to the range's change event.
+	 *
+	 * @private
+	 * @param {engine.model.Range} range
+	 */
+	_prepareRange( range ) {
 		if ( !( range instanceof Range ) ) {
+			/**
+			 * Trying to add an object that is not an instance of Range.
+			 *
+			 * @error model-selection-added-not-range
+			 */
 			throw new CKEditorError( 'model-selection-added-not-range: Trying to add an object that is not an instance of Range.' );
 		}
 
+		if ( range.root == this._document.graveyard ) {
+			/**
+			 * Trying to add a Range that is in the graveyard root. Range rejected.
+			 *
+			 * @warning model-selection-range-in-graveyard
+			 */
+			log.warn( 'model-selection-range-in-graveyard: Trying to add a Range that is in the graveyard root. Range rejected.' );
+
+			return;
+		}
+
 		this._checkRange( range );
 
 		const liveRange = LiveRange.createFromRange( range );
-		this.listenTo( liveRange, 'change', () => {
+		this.listenTo( liveRange, 'change', ( evt, oldRange ) => {
+			if ( liveRange.root == this._document.graveyard ) {
+				this._fixGraveyardSelection( liveRange, oldRange );
+			}
+
 			this.fire( 'change:range', { directChange: false } );
 		} );
 
-		this._ranges.push( liveRange );
+		return liveRange;
 	}
 
 	/**
-	 * Updates this selection attributes according to it's ranges and the {@link engine.model.Document model document}.
+	 * Updates this selection attributes according to its ranges and the {@link engine.model.Document model document}.
 	 *
 	 * @protected
 	 * @param {Boolean} clearAll
@@ -544,6 +605,29 @@ export default class LiveSelection extends Selection {
 
 		return attrs;
 	}
+
+	/**
+	 * Fixes a selection range after it ends up in graveyard root.
+	 *
+	 * @private
+	 * @param {engine.model.LiveRange} gyRange The range added in selection, that ended up in graveyard root.
+	 * @param {engine.model.Range} oldRange The state of that range before it was added to graveyard root.
+	 */
+	_fixGraveyardSelection( gyRange, oldRange ) {
+		const gyPath = gyRange.start.path;
+
+		const newPathLength = oldRange.start.path.length - ( gyPath.length - 2 );
+		const newPath = oldRange.start.path.slice( 0, newPathLength );
+		newPath[ newPath.length - 1 ] -= gyPath[ 1 ];
+
+		const newPosition = new Position( oldRange.root, newPath );
+		const newRange = this._prepareRange( new Range( newPosition, newPosition ) );
+
+		const index = this._ranges.indexOf( gyRange );
+		this._ranges.splice( index, 1, newRange );
+
+		gyRange.detach();
+	}
 }
 
 // Helper function for {@link engine.model.LiveSelection#_updateAttributes}. It takes model item, checks whether

+ 1 - 1
packages/ckeditor5-engine/src/model/position.js

@@ -104,7 +104,7 @@ export default class Position {
 	}
 
 	/**
-	 * Offset at which this position is located in it's {@link engine.model.Position#parent parent}. It is equal
+	 * Offset at which this position is located in its {@link engine.model.Position#parent parent}. It is equal
 	 * to the last item in position {@link engine.model.Position#path path}.
 	 *
 	 * @type {Number}

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

@@ -486,7 +486,8 @@ export default class DomConverter {
 			} else {
 				const viewBefore = this.getCorrespondingView( domParent.childNodes[ domOffset - 1 ] );
 
-				if ( viewBefore ) {
+				// TODO #663
+				if ( viewBefore && viewBefore.parent ) {
 					return new ViewPosition( viewBefore.parent, viewBefore.index + 1 );
 				}
 			}

+ 119 - 46
packages/ckeditor5-engine/src/view/renderer.js

@@ -88,13 +88,12 @@ export default class Renderer {
 		this.selection = selection;
 
 		/**
-		 * Position of the inline {@link engine.view.filler filler}.
-		 * It should always be put before the text which contains filler.
+		 * The text node in which the inline filler was rendered.
 		 *
 		 * @private
-		 * @member {engine.view.Position} engine.view.Renderer#_inlineFillerPosition
+		 * @member {Text} engine.view.Renderer#_inlineFiller
 		 */
-		this._inlineFillerPosition = null;
+		this._inlineFiller = null;
 
 		/**
 		 * Indicates if view document is focused and selection can be rendered. Selection will not be rendered if
@@ -176,21 +175,30 @@ export default class Renderer {
 	 * removed as long selection is in the text node which needed it at first.
 	 */
 	render() {
-		if ( !this._isInlineFillerAtSelection() ) {
+		let inlineFillerPosition;
+
+		// There was inline filler rendered in the DOM but it's not
+		// at the selection position any more, so we can remove it
+		// (cause even if it's needed, it must be placed in another location).
+		if ( this._inlineFiller && !this._isSelectionInInlineFiller() ) {
 			this._removeInlineFiller();
+		}
 
-			if ( this._needAddInlineFiller() ) {
-				this._inlineFillerPosition = this.selection.getFirstPosition();
-				// Do not use `markToSync` so it will be added even if the parent is already added.
-				this.markedChildren.add( this._inlineFillerPosition.parent );
-			} else {
-				this._inlineFillerPosition = null;
-			}
+		// If we've got the filler, let's try to guess its position in the view.
+		if ( this._inlineFiller ) {
+			inlineFillerPosition = this._getInlineFillerPosition();
+		}
+		// Othewise, if it's needed, create it at the selection position.
+		else if ( this._needsInlineFillerAtSelection() ) {
+			inlineFillerPosition = this.selection.getFirstPosition();
+
+			// Do not use `markToSync` so it will be added even if the parent is already added.
+			this.markedChildren.add( inlineFillerPosition.parent );
 		}
 
 		for ( let node of this.markedTexts ) {
 			if ( !this.markedChildren.has( node.parent ) && this.domConverter.getCorrespondingDom( node.parent ) ) {
-				this._updateText( node );
+				this._updateText( node, { inlineFillerPosition } );
 			}
 		}
 
@@ -199,7 +207,7 @@ export default class Renderer {
 		}
 
 		for ( let element of this.markedChildren ) {
-			this._updateChildren( element );
+			this._updateChildren( element, { inlineFillerPosition } );
 		}
 
 		this._updateSelection();
@@ -208,63 +216,120 @@ export default class Renderer {
 		this.markedTexts.clear();
 		this.markedAttributes.clear();
 		this.markedChildren.clear();
+
+		// Remember the filler by its node.
+		this._inlineFiller = this._getInlineFillerNode( inlineFillerPosition );
+	}
+
+	/**
+	 * Gets the text node in which the inline filler is kept.
+	 *
+	 * @private
+	 * @param {engine.view.Position} fillerPosition The position on which the filler is needed in the view.
+	 * @returns {Text} The text node with the filler.
+	 */
+	_getInlineFillerNode( fillerPosition ) {
+		if ( !fillerPosition ) {
+			this._inlineFiller = null;
+
+			return;
+		}
+
+		const domPosition = this.domConverter.viewPositionToDom( fillerPosition );
+
+		/* istanbul ignore if */
+		if ( !domPosition || !startsWithFiller( domPosition.parent ) ) {
+			/**
+			 * Cannot find filler node by its position.
+			 *
+			 * @error view-renderer-cannot-find-filler
+			 */
+			throw new CKEditorError( 'view-renderer-cannot-find-filler: Cannot find filler node by its position.' );
+		}
+
+		return domPosition.parent;
+	}
+
+	/**
+	 * Gets the position of the inline filler based on the current selection.
+	 * Here, we assume that we know that the filler is needed and
+	 * {@link #_isSelectionInInlineFiller is at the selection position}, and, since it's needed,
+	 * it's somewhere at the selection postion.
+	 *
+	 * Note: we cannot restore the filler position based on the filler's DOM text node, because
+	 * when this method is called (before rendering) the bindings will often be broken. View to DOM
+	 * bindings are only dependable after rendering.
+	 *
+	 * @private
+	 * @returns {engine.view.Position}
+	 */
+	_getInlineFillerPosition() {
+		const firstPos = this.selection.getFirstPosition();
+
+		if ( firstPos.parent  instanceof ViewText ) {
+			return ViewPosition.createBefore( this.selection.getFirstPosition().parent );
+		} else {
+			return firstPos;
+		}
 	}
 
 	/**
-	 * Returns `true` if the inline filler and selection are in the same place.
-	 * If it is true it means filler had been added for a reason and selection does not
-	 * left text node, user can be in the middle of the composition so it should not be touched.
+	 * Returns `true` if the selection hasn't left the inline filler's text node.
+	 * If it is `true` it means that the filler had been added for a reason and the selection does not
+	 * left the filler's text node. E.g. the user can be in the middle of a composition so it should not be touched.
 	 *
 	 * @private
 	 * @returns {Boolean} True if the inline filler and selection are in the same place.
 	 */
-	_isInlineFillerAtSelection() {
+	_isSelectionInInlineFiller() {
 		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed ) {
 			return false;
 		}
 
+		// Note, we can't check if selection's position equals position of the
+		// this._inlineFiller node, because of #663. We may not be able to calculate
+		// the filler's position in the view at this stage.
+		// Instead, we check it the other way – whether selection is anchored in
+		// that text node or next to it.
+
+		// Possible options are:
+		// "FILLER{}"
+		// "FILLERadded-text{}"
+
 		const selectionPosition = this.selection.getFirstPosition();
-		const fillerPosition = this._inlineFillerPosition;
 
-		if ( !fillerPosition ) {
+		// If we cannot convert this position's parent it means that selection is in not yet rendered
+		// node, which means that the filler can't be there.
+		if ( !this.domConverter.getCorrespondingDom( selectionPosition.parent ) ) {
 			return false;
 		}
 
-		if ( fillerPosition.isEqual( selectionPosition )  ) {
-			return true;
-		}
+		const { parent: domParent } = this.domConverter.viewPositionToDom( selectionPosition );
 
-		if ( selectionPosition.parent instanceof ViewText ) {
-			if ( fillerPosition.isEqual( ViewPosition.createBefore( selectionPosition.parent ) ) ) {
-				return true;
-			}
+		if ( this.domConverter.isText( domParent ) && startsWithFiller( domParent ) ) {
+			return true;
 		}
 
 		return false;
 	}
 
 	/**
-	 * Removes inline filler.
+	 * Removes the inline filler.
 	 *
 	 * @private
 	 */
 	_removeInlineFiller() {
-		if ( !this._inlineFillerPosition ) {
-			// Nothing to remove.
-			return;
-		}
+		const domFillerNode = this._inlineFiller;
 
-		const domFillerPosition = this.domConverter.viewPositionToDom( this._inlineFillerPosition );
-		const domFillerNode = domFillerPosition.parent;
-
-		// If there is no filler viewPositionToDom will return parent node, so domFillerNode will be an element.
-		if ( !( this.domConverter.isText( domFillerNode ) ) || !startsWithFiller( domFillerNode ) ) {
+		// Something weird happened and the stored node doesn't contain the filler's text.
+		if ( !startsWithFiller( domFillerNode ) ) {
 			/**
-			 * No inline filler on expected position.
+			 * The inline filler node was lost. Most likely, something overwrote the filler text node
+			 * in the DOM.
 			 *
-			 * @error renderer-render-no-inline-filler.
+			 * @error view-renderer-filler-was-lost
 			 */
-			throw new CKEditorError( 'view-renderer-render-no-inline-filler: No inline filler on expected position.' );
+			throw new CKEditorError( 'view-renderer-filler-was-lost: The inline filler node was lost.' );
 		}
 
 		if ( isInlineFiller( domFillerNode ) ) {
@@ -272,15 +337,17 @@ export default class Renderer {
 		} else {
 			domFillerNode.data = domFillerNode.data.substr( INLINE_FILLER_LENGTH );
 		}
+
+		this._inlineFiller = null;
 	}
 
 	/**
-	 * Checks if the inline {@link engine.view.filler fillers} should be added.
+	 * Checks if the inline {@link engine.view.filler filler} should be added.
 	 *
 	 * @private
 	 * @returns {Boolean} True if the inline fillers should be added.
 	 */
-	_needAddInlineFiller() {
+	_needsInlineFillerAtSelection() {
 		if ( this.selection.rangeCount != 1 || !this.selection.isCollapsed ) {
 			return false;
 		}
@@ -318,15 +385,18 @@ export default class Renderer {
 	 *
 	 * @private
 	 * @param {engine.view.Text} viewText View text to update.
+	 * @param {Object} options
+	 * @param {engine.view.Position} options.inlineFillerPosition The position on which the inline
+	 * filler should be rendered.
 	 */
-	_updateText( viewText ) {
+	_updateText( viewText, options ) {
 		const domText = this.domConverter.getCorrespondingDom( viewText );
 		const newDomText = this.domConverter.viewToDom( viewText, domText.ownerDocument );
 
 		const actualText = domText.data;
 		let expectedText = newDomText.data;
 
-		const filler = this._inlineFillerPosition;
+		const filler = options.inlineFillerPosition;
 
 		if ( filler && filler.parent == viewText.parent && filler.offset == viewText.index ) {
 			expectedText = INLINE_FILLER + expectedText;
@@ -366,13 +436,16 @@ export default class Renderer {
 	 *
 	 * @private
 	 * @param {engine.view.Element} viewElement View element to update.
+	 * @param {Object} options
+	 * @param {engine.view.Position} options.inlineFillerPosition The position on which the inline
+	 * filler should be rendered.
 	 */
-	_updateChildren( viewElement ) {
+	_updateChildren( viewElement, options ) {
 		const domConverter = this.domConverter;
 		const domElement = domConverter.getCorrespondingDom( viewElement );
 		const domDocument = domElement.ownerDocument;
 
-		const filler = this._inlineFillerPosition;
+		const filler = options.inlineFillerPosition;
 
 		const actualDomChildren = domElement.childNodes;
 		const expectedDomChildren = Array.from( domConverter.viewChildrenToDom( viewElement, domDocument, { bind: true } ) );

+ 12 - 18
packages/ckeditor5-engine/tests/model/composer/deletecontents.js

@@ -93,9 +93,7 @@ describe( 'Delete utils', () => {
 			} );
 
 			it( 'deletes characters (first half has attrs)', () => {
-				setData( doc, '<$text bold="true">fo[o</$text>b]ar', { selectionAttributes: {
-					bold: true
-				} } );
+				setData( doc, '<$text bold="true">fo[o</$text>b]ar' );
 
 				deleteContents( doc.batch(), doc.selection );
 
@@ -104,9 +102,7 @@ describe( 'Delete utils', () => {
 			} );
 
 			it( 'deletes characters (2nd half has attrs)', () => {
-				setData( doc, 'fo[o<$text bold="true">b]ar</$text>', { selectionAttributes: {
-					bold: true
-				} } );
+				setData( doc, 'fo[o<$text bold="true">b]ar</$text>' );
 
 				deleteContents( doc.batch(), doc.selection );
 
@@ -115,15 +111,7 @@ describe( 'Delete utils', () => {
 			} );
 
 			it( 'clears selection attrs when emptied content', () => {
-				setData(
-					doc,
-					'<paragraph>x</paragraph><paragraph>[<$text bold="true">foo</$text>]</paragraph><paragraph>y</paragraph>',
-					{
-						selectionAttributes: {
-							bold: true
-						}
-					}
-				);
+				setData( doc, '<paragraph>x</paragraph><paragraph>[<$text bold="true">foo</$text>]</paragraph><paragraph>y</paragraph>' );
 
 				deleteContents( doc.batch(), doc.selection );
 
@@ -132,9 +120,15 @@ describe( 'Delete utils', () => {
 			} );
 
 			it( 'leaves selection attributes when text contains them', () => {
-				setData( doc, '<paragraph>x<$text bold="true">a[foo]b</$text>y</paragraph>', { selectionAttributes: {
-					bold: true
-				} } );
+				setData(
+					doc,
+					'<paragraph>x<$text bold="true">a[foo]b</$text>y</paragraph>',
+					{
+						selectionAttributes: {
+							bold: true
+						}
+					}
+				);
 
 				deleteContents( doc.batch(), doc.selection );
 

+ 0 - 8
packages/ckeditor5-engine/tests/model/document/document.js

@@ -225,14 +225,6 @@ describe( 'Document', () => {
 	} );
 
 	describe( 'selection', () => {
-		it( 'should get updated attributes whenever selection gets updated', () => {
-			sinon.spy( doc.selection, '_updateAttributes' );
-
-			doc.selection.fire( 'change:range', { directChange: true } );
-
-			expect( doc.selection._updateAttributes.called ).to.be.true;
-		} );
-
 		it( 'should get updated attributes whenever attribute operation is applied', () => {
 			sinon.spy( doc.selection, '_updateAttributes' );
 

+ 42 - 8
packages/ckeditor5-engine/tests/model/liveposition.js

@@ -89,10 +89,13 @@ describe( 'LivePosition', () => {
 	} );
 
 	describe( 'should get transformed if', () => {
-		let live;
+		let live, spy;
 
 		beforeEach( () => {
 			live = new LivePosition( root, [ 1, 4, 6 ] );
+
+			spy = sinon.spy();
+			live.on( 'change', spy );
 		} );
 
 		afterEach( () => {
@@ -106,6 +109,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 4, 9 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at the same position and live position is sticking to right side', () => {
@@ -114,6 +118,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 4, 9 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is before a node from the live position path', () => {
@@ -122,6 +127,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 6, 6 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 		} );
 
@@ -137,6 +143,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 4, 9 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at the same position and live position is sticking to right side', () => {
@@ -150,6 +157,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 4, 9 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at a position before a node from the live position path', () => {
@@ -163,6 +171,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 6, 6 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is from the same parent and closer offset', () => {
@@ -176,6 +185,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 4, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is from a position before a node from the live position path', () => {
@@ -189,6 +199,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 1, 0, 6 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'contains live position (same level)', () => {
@@ -202,6 +213,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 2, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'contains live position (deep)', () => {
@@ -215,12 +227,13 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( [ 2, 1, 6 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 		} );
 	} );
 
 	describe( 'should not get transformed if', () => {
-		let path, otherRoot;
+		let path, otherRoot, spy;
 
 		before( () => {
 			path = [ 1, 4, 6 ];
@@ -231,6 +244,9 @@ describe( 'LivePosition', () => {
 
 		beforeEach( () => {
 			live = new LivePosition( root, path );
+
+			spy = sinon.spy();
+			live.on( 'change', spy );
 		} );
 
 		afterEach( () => {
@@ -244,17 +260,22 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is at the same position and live position is sticking to left side', () => {
-				let live = new LivePosition( root, path, 'STICKS_TO_PREVIOUS' );
+				let newLive = new LivePosition( root, path, 'sticksToPrevious' );
+				spy = sinon.spy();
+				newLive.on( 'change', spy );
+
 				let insertRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) );
 
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
-				expect( live.path ).to.deep.equal( path );
+				expect( newLive.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 
-				live.detach();
+				newLive.detach();
 			} );
 
 			it( 'is after a node from the position path', () => {
@@ -263,6 +284,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is in different root', () => {
@@ -271,6 +293,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
@@ -286,10 +309,14 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is at the same position and live position is sticking to left side', () => {
-				let live = new LivePosition( root, path, 'STICKS_TO_PREVIOUS' );
+				let newLive = new LivePosition( root, path, 'sticksToPrevious' );
+				spy = sinon.spy();
+				newLive.on( 'change', spy );
+
 				let moveSource = new Position( root, [ 2 ] );
 				let moveRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) );
 
@@ -299,9 +326,10 @@ describe( 'LivePosition', () => {
 				};
 				doc.fire( 'change', 'move', changes, null );
 
-				expect( live.path ).to.deep.equal( path );
+				expect( newLive.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 
-				live.detach();
+				newLive.detach();
 			} );
 
 			it( 'is at a position after a node from the live position path', () => {
@@ -315,6 +343,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from the same parent and further offset', () => {
@@ -328,6 +357,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from a position after a node from the live position path', () => {
@@ -341,6 +371,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is to different root', () => {
@@ -354,6 +385,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from different root', () => {
@@ -367,6 +399,7 @@ describe( 'LivePosition', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.path ).to.deep.equal( path );
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
@@ -381,6 +414,7 @@ describe( 'LivePosition', () => {
 			doc.fire( 'change', 'setAttribute', changes, null );
 
 			expect( live.path ).to.deep.equal( path );
+			expect( spy.called ).to.be.false;
 		} );
 	} );
 } );

+ 56 - 18
packages/ckeditor5-engine/tests/model/liverange.js

@@ -94,10 +94,13 @@ describe( 'LiveRange', () => {
 	// structure is representing what is _after_ operation is executed. So live LiveRange properties are describing
 	// virtual tree that is not existing anymore and event ranges are operating on the tree generated above.
 	describe( 'should get transformed if', () => {
-		let live;
+		let live, spy;
 
 		beforeEach( () => {
 			live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
+
+			spy = sinon.spy();
+			live.on( 'change', spy );
 		} );
 
 		afterEach( () => {
@@ -112,6 +115,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is in the same parent as range end and before it', () => {
@@ -121,6 +125,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 5 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at a position before a node from range start path', () => {
@@ -130,6 +135,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 3, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 4, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at a position before a node from range end path', () => {
@@ -139,6 +145,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is at the live range start position and live range is collapsed', () => {
@@ -150,6 +157,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 1, 8 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 		} );
 
@@ -166,6 +174,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is to the same parent as range end and before it', () => {
@@ -180,6 +189,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 6 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is to a position before a node from range start path', () => {
@@ -194,6 +204,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 3, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 4, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is to a position before a node from range end path', () => {
@@ -208,6 +219,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is from the same parent as range start and before it', () => {
@@ -222,6 +234,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 1 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is from the same parent as range end and before it', () => {
@@ -236,6 +249,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 0 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is from a position before a node from range start path', () => {
@@ -250,6 +264,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 0, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 1, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'intersects on live range left side', () => {
@@ -264,6 +279,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 2 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'intersects on live range right side', () => {
@@ -278,6 +294,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'intersects on live range left side and live range new start is touching moved range end', () => {
@@ -292,6 +309,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 5 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 7, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'intersects on live range right side and live range new end is touching moved range start', () => {
@@ -308,6 +326,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is equal to live range', () => {
@@ -324,6 +343,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 3, 0 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 3, 3 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'contains live range', () => {
@@ -340,22 +360,7 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 3, 1 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 3, 4 ] );
-			} );
-
-			it( 'is inside live range and points to live range', () => {
-				live.end.path = [ 0, 1, 12 ];
-
-				let moveSource = new Position( root, [ 0, 1, 6 ] );
-				let moveRange = new Range( new Position( root, [ 0, 1, 8 ] ), new Position( root, [ 0, 1, 10 ] ) );
-
-				let changes = {
-					range: moveRange,
-					sourcePosition: moveSource
-				};
-				doc.fire( 'change', 'move', changes, null );
-
-				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
-				expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 
 			it( 'is intersecting with live range and points to live range', () => {
@@ -372,12 +377,13 @@ describe( 'LiveRange', () => {
 
 				expect( live.start.path ).to.deep.equal( [ 0, 1, 2 ] );
 				expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] );
+				expect( spy.calledOnce ).to.be.true;
 			} );
 		} );
 	} );
 
 	describe( 'should not get transformed if', () => {
-		let otherRoot;
+		let otherRoot, spy;
 
 		before( () => {
 			otherRoot = doc.createRoot( '$root', 'otherRoot' );
@@ -388,6 +394,9 @@ describe( 'LiveRange', () => {
 		beforeEach( () => {
 			live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) );
 			clone = Range.createFromRange( live );
+
+			spy = sinon.spy();
+			live.on( 'change', spy );
 		} );
 
 		afterEach( () => {
@@ -401,6 +410,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is in the same parent as range end and after it', () => {
@@ -409,6 +419,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is to a position after a node from range end path', () => {
@@ -417,6 +428,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is in different root', () => {
@@ -425,6 +437,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'insert', { range: insertRange }, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
@@ -440,6 +453,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is to the same parent as range end and after it', () => {
@@ -453,6 +467,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is to a position after a node from range end path', () => {
@@ -466,6 +481,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from the same parent as range start and after it', () => {
@@ -479,6 +495,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from the same parent as range end and after it', () => {
@@ -492,6 +509,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from a position after a node from range end path', () => {
@@ -505,6 +523,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is to different root', () => {
@@ -518,6 +537,7 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 
 			it( 'is from different root', () => {
@@ -531,6 +551,24 @@ describe( 'LiveRange', () => {
 				doc.fire( 'change', 'move', changes, null );
 
 				expect( live.isEqual( clone ) ).to.be.true;
+				expect( spy.called ).to.be.false;
+			} );
+
+			it( 'is inside live range and points to live range', () => {
+				live.end.path = [ 0, 1, 12 ];
+
+				let moveSource = new Position( root, [ 0, 1, 6 ] );
+				let moveRange = new Range( new Position( root, [ 0, 1, 8 ] ), new Position( root, [ 0, 1, 10 ] ) );
+
+				let changes = {
+					range: moveRange,
+					sourcePosition: moveSource
+				};
+				doc.fire( 'change', 'move', changes, null );
+
+				expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] );
+				expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] );
+				expect( spy.calledOnce ).to.be.false;
 			} );
 		} );
 	} );

+ 77 - 3
packages/ckeditor5-engine/tests/model/liveselection.js

@@ -14,6 +14,7 @@ import LiveRange from '/ckeditor5/engine/model/liverange.js';
 import LiveSelection from '/ckeditor5/engine/model/liveselection.js';
 import InsertOperation from '/ckeditor5/engine/model/operation/insertoperation.js';
 import MoveOperation from '/ckeditor5/engine/model/operation/moveoperation.js';
+import RemoveOperation from '/ckeditor5/engine/model/operation/removeoperation.js';
 import AttributeOperation from '/ckeditor5/engine/model/operation/attributeoperation.js';
 import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 import count from '/ckeditor5/utils/count.js';
@@ -125,9 +126,7 @@ describe( 'LiveSelection', () => {
 		it( 'should convert added Range to LiveRange', () => {
 			selection.addRange( range );
 
-			const ranges = selection._ranges;
-
-			expect( ranges[ 0 ] ).to.be.instanceof( LiveRange );
+			expect( selection._ranges[ 0 ] ).to.be.instanceof( LiveRange );
 		} );
 
 		it( 'should throw an error when range is invalid', () => {
@@ -135,6 +134,20 @@ describe( 'LiveSelection', () => {
 				selection.addRange( { invalid: 'range' } );
 			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
 		} );
+
+		it( 'should not add a range that is in graveyard', () => {
+			selection.addRange( Range.createIn( doc.graveyard ) );
+
+			expect( selection._ranges.length ).to.equal( 0 );
+		} );
+
+		it( 'should refresh attributes', () => {
+			const spy = sinon.spy( selection, '_updateAttributes' );
+
+			selection.addRange( range );
+
+			expect( spy.called ).to.be.true;
+		} );
 	} );
 
 	describe( 'collapse', () => {
@@ -227,6 +240,14 @@ describe( 'LiveSelection', () => {
 			expect( ranges[ 0 ].detach.called ).to.be.true;
 			expect( ranges[ 1 ].detach.called ).to.be.true;
 		} );
+
+		it( 'should refresh attributes', () => {
+			const spy = sinon.spy( selection, '_updateAttributes' );
+
+			selection.removeAllRanges();
+
+			expect( spy.called ).to.be.true;
+		} );
 	} );
 
 	describe( 'setRanges', () => {
@@ -250,6 +271,14 @@ describe( 'LiveSelection', () => {
 			expect( oldRanges[ 0 ].detach.called ).to.be.true;
 			expect( oldRanges[ 1 ].detach.called ).to.be.true;
 		} );
+
+		it( 'should refresh attributes', () => {
+			const spy = sinon.spy( selection, '_updateAttributes' );
+
+			selection.setRanges( [ range ] );
+
+			expect( spy.called ).to.be.true;
+		} );
 	} );
 
 	describe( 'getFirstRange', () => {
@@ -284,6 +313,7 @@ describe( 'LiveSelection', () => {
 		let spyRange;
 
 		beforeEach( () => {
+			root.removeChildren( 0, root.childCount );
 			root.insertChildren( 0, [
 				new Element( 'ul', [], new Text( 'abcdef' ) ),
 				new Element( 'p', [], new Text( 'foobar' ) ),
@@ -518,6 +548,50 @@ describe( 'LiveSelection', () => {
 				expect( spyAttribute.called ).to.be.false;
 			} );
 		} );
+
+		describe( 'RemoveOperation', () => {
+			it( 'fix selection range if it ends up in graveyard #1', () => {
+				selection.collapse( new Position( root, [ 1, 3 ] ) );
+
+				doc.applyOperation( wrapInDelta(
+					new RemoveOperation(
+						new Position( root, [ 1, 2 ] ),
+						2,
+						doc.version
+					)
+				) );
+
+				expect( selection.getFirstPosition().path ).to.deep.equal( [ 1, 2 ] );
+			} );
+
+			it( 'fix selection range if it ends up in graveyard #2', () => {
+				selection.setRanges( [ new Range( new Position( root, [ 1, 2 ] ), new Position( root, [ 1, 4 ] ) ) ] );
+
+				doc.applyOperation( wrapInDelta(
+					new RemoveOperation(
+						new Position( root, [ 1, 2 ] ),
+						2,
+						doc.version
+					)
+				) );
+
+				expect( selection.getFirstPosition().path ).to.deep.equal( [ 1, 2 ] );
+			} );
+
+			it( 'fix selection range if it ends up in graveyard #3', () => {
+				selection.setRanges( [ new Range( new Position( root, [ 1, 2 ] ), new Position( root, [ 1, 4 ] ) ) ] );
+
+				doc.applyOperation( wrapInDelta(
+					new RemoveOperation(
+						new Position( root, [ 0 ] ),
+						3,
+						doc.version
+					)
+				) );
+
+				expect( selection.getFirstPosition().path ).to.deep.equal( [ 0 ] );
+			} );
+		} );
 	} );
 
 	describe( 'attributes interface', () => {

+ 50 - 32
packages/ckeditor5-engine/tests/view/document/jumpoverinlinefiller.js

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md.
  */
 
-/* globals document */
+/* globals document, Range */
 /* bender-tags: view, browser-only */
 
 import ViewRange from '/ckeditor5/engine/view/range.js';
@@ -18,9 +18,8 @@ import { parse, setData } from '/ckeditor5/engine/dev-utils/view.js';
 describe( 'Document', () => {
 	let viewDocument, domRoot;
 
-	before( () => {
+	beforeEach( () => {
 		domRoot = createElement( document, 'div', {
-			id: 'editor',
 			contenteditable: 'true'
 		} );
 		document.body.appendChild( domRoot );
@@ -33,7 +32,10 @@ describe( 'Document', () => {
 		viewDocument.isFocused = true;
 	} );
 
-	after( () => {
+	afterEach( () => {
+		// There's no destroy() method yet, so let's at least kill the observers.
+		viewDocument.disableObservers();
+
 		domRoot.parentElement.removeChild( domRoot );
 	} );
 
@@ -44,10 +46,11 @@ describe( 'Document', () => {
 
 			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
 
-			const domRange = document.getSelection().getRangeAt( 0 );
-			expect( isInlineFiller( domRange.startContainer ) ).to.be.true;
-			expect( domRange.startOffset ).to.equal( 0 );
-			expect( domRange.collapsed ).to.be.true;
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.data ).to.equal( 'foo' );
+			expect( domSelection.anchorOffset ).to.equal( 3 );
+			expect( domSelection.isCollapsed ).to.be.true;
 		} );
 
 		it( 'should do nothing when another key is pressed', () => {
@@ -56,10 +59,11 @@ describe( 'Document', () => {
 
 			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowright, domTarget: viewDocument.domRoots.get( 'main' ) } );
 
-			const domRange = document.getSelection().getRangeAt( 0 );
-			expect( isInlineFiller( domRange.startContainer ) ).to.be.true;
-			expect( domRange.startOffset ).to.equal( INLINE_FILLER_LENGTH );
-			expect( domRange.collapsed ).to.be.true;
+			const domSelection = document.getSelection();
+
+			expect( isInlineFiller( domSelection.anchorNode ) ).to.be.true;
+			expect( domSelection.anchorOffset ).to.equal( INLINE_FILLER_LENGTH );
+			expect( domSelection.isCollapsed ).to.be.true;
 		} );
 
 		it( 'should do nothing if range is not collapsed', () => {
@@ -68,43 +72,57 @@ describe( 'Document', () => {
 
 			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
 
-			const domRange = document.getSelection().getRangeAt( 0 );
-			expect( domRange.startContainer.data ).to.equal( 'x' );
-			expect( domRange.startOffset ).to.equal( 0 );
-			expect( domRange.endContainer.data ).to.equal( 'x' );
-			expect( domRange.endOffset ).to.equal( 1 );
+			const domSelection = document.getSelection();
+
+			expect( domSelection.anchorNode.data ).to.equal( 'x' );
+			expect( domSelection.anchorOffset ).to.equal( 0 );
+			expect( domSelection.focusNode.data ).to.equal( 'x' );
+			expect( domSelection.focusOffset ).to.equal( 1 );
 		} );
 
-		it( 'should do nothing if node does not start with filler', () => {
-			setData( viewDocument, '<container:p>foo<attribute:b>{}x</attribute:b>bar</container:p>' );
-			viewDocument.render();
+		// See #664
+		// it( 'should do nothing if node does not start with the filler', () => {
+		// 	setData( viewDocument, '<container:p>foo<attribute:b>{}x</attribute:b>bar</container:p>' );
+		// 	viewDocument.render();
 
-			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
+		// 	viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
 
-			const domRange = document.getSelection().getRangeAt( 0 );
-			expect( domRange.startContainer.data ).to.equal( 'x' );
-			expect( domRange.startOffset ).to.equal( 0 );
-			expect( domRange.collapsed ).to.be.true;
-		} );
+		// 	const domSelection = document.getSelection();
+
+		// 	expect( domSelection.anchorNode.data ).to.equal( 'x' );
+		// 	expect( domSelection.anchorOffset ).to.equal( INLINE_FILLER_LENGTH );
+		// 	expect( domSelection.isCollapsed ).to.be.true;
+		// } );
 
-		it( 'should do nothing if caret is not directly before filler', () => {
+		it( 'should do nothing if caret is not directly before the filler', () => {
 			setData( viewDocument, '<container:p>foo<attribute:b>[]</attribute:b>bar</container:p>' );
 			viewDocument.render();
 
-			// '<container:p>foo<attribute:b>x{}</attribute:b>bar</container:p>'
+			// Insert a letter to the <b>: '<container:p>foo<attribute:b>x{}</attribute:b>bar</container:p>'
+			// Do this both in the view and in the DOM to simulate typing and to avoid rendering (which would remove the filler).
 			const viewB = viewDocument.selection.getFirstPosition().parent;
 			const viewTextX = parse( 'x' );
 			viewB.appendChildren( viewTextX );
 			viewDocument.selection.removeAllRanges();
 			viewDocument.selection.addRange( ViewRange.createFromParentsAndOffsets( viewTextX, 1, viewTextX, 1 ) );
+
+			const domB = viewDocument.getDomRoot( 'main' ).querySelector( 'b' );
+			const domSelection = document.getSelection();
+			domB.childNodes[ 0 ].data += 'x';
+
+			const domRange = new Range();
+			domSelection.removeAllRanges();
+			domRange.setStart( domB.childNodes[ 0 ], INLINE_FILLER_LENGTH + 1 );
+			domRange.collapse( true );
+			domSelection.addRange( domRange );
+
 			viewDocument.render();
 
 			viewDocument.fire( 'keydown', { keyCode: keyCodes.arrowleft, domTarget: viewDocument.domRoots.get( 'main' ) } );
 
-			const domRange = document.getSelection().getRangeAt( 0 );
-			expect( startsWithFiller( domRange.startContainer ) ).to.be.true;
-			expect( domRange.startOffset ).to.equal( INLINE_FILLER_LENGTH + 1 );
-			expect( domRange.collapsed ).to.be.true;
+			expect( startsWithFiller( domSelection.anchorNode ) ).to.be.true;
+			expect( domSelection.anchorOffset ).to.equal( INLINE_FILLER_LENGTH + 1 );
+			expect( domSelection.isCollapsed ).to.be.true;
 		} );
 	} );
 } );

+ 127 - 4
packages/ckeditor5-engine/tests/view/renderer.js

@@ -577,6 +577,112 @@ describe( 'Renderer', () => {
 			expect( domP.childNodes[ 1 ].childNodes[ 0 ].data ).to.equal( 'x' );
 		} );
 
+		// #659
+		// The element before the filler is removed so the position of the filler
+		// cannot be remembered as parent+offset.
+		it( 'should remove filler from a modified DOM in case <p>bar<b>foo</b>[]</p>', () => {
+			// Step 1: <p>bar<b>foo</b>"FILLER{}"</p>
+			const { view: viewP, selection: newSelection } = parse( '<container:p>bar<attribute:b>foo</attribute:b>[]</container:p>' );
+			viewRoot.appendChildren( viewP );
+			selection.setTo( newSelection );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			const domP = domRoot.childNodes[ 0 ];
+			expect( domP.childNodes.length ).to.equal( 3 );
+			expect( domP.childNodes[ 2 ].data ).to.equal( INLINE_FILLER );
+
+			// Step 2: Remove the <b> and update the selection (<p>bar[]</p>).
+			viewP.removeChildren( 1 );
+
+			selection.removeAllRanges();
+			selection.addRange( ViewRange.createFromParentsAndOffsets( viewP, 1, viewP, 1 ) );
+
+			renderer.markToSync( 'children', viewP );
+			renderer.render();
+
+			// Step 3: Check whether there's no filler in the DOM.
+			expect( domP.childNodes.length ).to.equal( 1 );
+			expect( domP.childNodes[ 0 ].data ).to.equal( 'bar' );
+		} );
+
+		// #659
+		it( 'should remove filler from a modified DOM when children moved', () => {
+			// Step 1: <p><b>foo</b>"FILLER{}"<b>bar</b></p><p></p>
+			const { view: viewFragment, selection: newSelection }
+				= parse( '<container:p><attribute:b>foo</attribute:b>[]<attribute:b>bar</attribute:b></container:p><container:p></container:p>' );
+			viewRoot.appendChildren( viewFragment );
+			selection.setTo( newSelection );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			expect( domRoot.childNodes.length ).to.equal( 2 );
+
+			const domP = domRoot.childNodes[ 0 ];
+			const domP2 = domRoot.childNodes[ 1 ];
+			expect( domP.childNodes.length ).to.equal( 3 );
+			expect( domP.childNodes[ 1 ].data ).to.equal( INLINE_FILLER );
+
+			// Step 2: Move <b>foo</b><b>bar</b> to the second paragraph and leave collapsed selection in the first one.
+			// <p>[]</p><p><b>foo</b><b>bar</b></p>
+			const viewP = viewRoot.getChild( 0 );
+			const viewP2 = viewRoot.getChild( 1 );
+			const removedChildren = viewP.removeChildren( 0, 2 );
+
+			viewP2.appendChildren( removedChildren );
+
+			selection.removeAllRanges();
+			selection.addRange( ViewRange.createFromParentsAndOffsets( viewP, 0, viewP, 0 ) );
+
+			renderer.markToSync( 'children', viewP );
+			renderer.markToSync( 'children', viewP2 );
+			renderer.render();
+
+			// Step 3: Check whether in the first paragraph there's a <br> filler and that
+			// in the second one there are two <b> tags.
+			expect( domP.childNodes.length ).to.equal( 1 );
+			expect( isBlockFiller( domP.childNodes[ 0 ], BR_FILLER ) ).to.be.true;
+
+			expect( domP2.childNodes.length ).to.equal( 2 );
+			expect( domP2.childNodes[ 0 ].tagName.toLowerCase() ).to.equal( 'b' );
+			expect( domP2.childNodes[ 1 ].tagName.toLowerCase() ).to.equal( 'b' );
+		} );
+
+		// Test for an edge case in the _isSelectionInInlineFiller which can be triggered like
+		// in one of ckeditor/ckeditor5-typing#59 automated tests.
+		it( 'should not break when selection is moved to a new element, when filler exists', () => {
+			// Step 1: <p>bar<b>"FILLER{}"</b></p>
+			const { view: viewP, selection: newSelection } = parse( '<container:p>bar<attribute:b>[]</attribute:b></container:p>' );
+			viewRoot.appendChildren( viewP );
+			selection.setTo( newSelection );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			const domP = domRoot.childNodes[ 0 ];
+			expect( domP.childNodes.length ).to.equal( 2 );
+			expect( domP.childNodes[ 1 ].childNodes[ 0 ].data ).to.equal( INLINE_FILLER );
+
+			// Step 2: Move selection to a new attribute element and remove the previous one
+			viewP.removeChildren( 1 ); // Remove <b>.
+
+			const viewI = parse( '<attribute:i></attribute:i>' );
+			viewP.appendChildren( viewI );
+
+			selection.removeAllRanges();
+			selection.addRange( ViewRange.createFromParentsAndOffsets( viewI, 0, viewI, 0 ) );
+
+			renderer.markToSync( 'children', viewP );
+			renderer.render();
+
+			// Step 3: Check whether new filler was created in the <i> element.
+			expect( domP.childNodes.length ).to.equal( 2 );
+			expect( domP.childNodes[ 1 ].tagName.toLowerCase() ).to.equal( 'i' );
+			expect( domP.childNodes[ 1 ].childNodes[ 0 ].data ).to.equal( INLINE_FILLER );
+		} );
+
 		it( 'should handle typing in empty block, do nothing if changes are already applied', () => {
 			const domSelection = document.getSelection();
 
@@ -699,6 +805,8 @@ describe( 'Renderer', () => {
 		it( 'should handle typing in empty attribute, do nothing if changes are already applied', () => {
 			const domSelection = document.getSelection();
 
+			// 1. Render <p><b>FILLER{}</b>foo</p>.
+
 			const { view: viewP, selection: newSelection } = parse(
 				'<container:p><attribute:b>[]</attribute:b>foo</container:p>' );
 
@@ -708,6 +816,8 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewRoot );
 			renderer.render();
 
+			// 2. Check the DOM.
+
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 2 );
@@ -725,7 +835,8 @@ describe( 'Renderer', () => {
 			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
 			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
 
-			// Add text node to both DOM and View <p><b>x</b>foo</p>
+			// 3. Add text node to both the DOM and the view: <p><b>FILLERx</b>foo</p>.
+
 			domB.childNodes[ 0 ].data += 'x';
 
 			domSelection.removeAllRanges();
@@ -746,6 +857,8 @@ describe( 'Renderer', () => {
 		it( 'should handle typing in empty attribute as a children change, render if needed', () => {
 			const domSelection = document.getSelection();
 
+			// 1. Render <p><b>FILLER{}</b>foo</p>.
+
 			const { view: viewP, selection: newSelection } = parse(
 				'<container:p><attribute:b>[]</attribute:b>foo</container:p>' );
 
@@ -755,6 +868,8 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewRoot );
 			renderer.render();
 
+			// 2. Check the DOM.
+
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 2 );
@@ -772,7 +887,8 @@ describe( 'Renderer', () => {
 			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
 			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
 
-			// Add text node only to View <p><b>x</b>foo</p>
+			// 3. Add text node only to the view: <p><b>x{}</b>foo</p>.
+
 			const viewText = new ViewText( 'x' );
 			viewB.appendChildren( viewText );
 			selection.removeAllRanges();
@@ -793,6 +909,8 @@ describe( 'Renderer', () => {
 		it( 'should handle typing in empty attribute as a text change, render if needed', () => {
 			const domSelection = document.getSelection();
 
+			// 1. Render <p><b>FILLER{}</b>foo</p>.
+
 			const { view: viewP, selection: newSelection } = parse(
 				'<container:p><attribute:b>[]</attribute:b>foo</container:p>' );
 
@@ -802,6 +920,8 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewRoot );
 			renderer.render();
 
+			// 2. Check the DOM.
+
 			const domP = domRoot.childNodes[ 0 ];
 
 			expect( domP.childNodes.length ).to.equal( 2 );
@@ -819,7 +939,8 @@ describe( 'Renderer', () => {
 			expect( domSelection.getRangeAt( 0 ).startOffset ).to.equal( INLINE_FILLER_LENGTH );
 			expect( domSelection.getRangeAt( 0 ).collapsed ).to.be.true;
 
-			// Add text node only to View <p><b>x</b>foo</p>
+			// 3. Add text node only to the view: <p><b>x{}</b>foo</p>.
+
 			const viewText = new ViewText( 'x' );
 			viewB.appendChildren( viewText );
 			selection.removeAllRanges();
@@ -828,6 +949,8 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'text', viewText );
 			renderer.render();
 
+			// 4. Check the DOM.
+
 			expect( domB.childNodes.length ).to.equal( 1 );
 			expect( domB.childNodes[ 0 ].data ).to.equal( INLINE_FILLER + 'x' );
 
@@ -958,7 +1081,7 @@ describe( 'Renderer', () => {
 
 			expect( () => {
 				renderer.render();
-			} ).to.throw();
+			} ).to.throw( CKEditorError, /^view-renderer-filler-was-lost/ );
 		} );
 
 		it( 'should handle focusing element', () => {