Kaynağa Gözat

Merge pull request #634 from ckeditor/t/259

Handling attributes, ranges and their changes in (Live)Selection.
Piotrek Koszuliński 9 yıl önce
ebeveyn
işleme
66a398b329

+ 4 - 4
packages/ckeditor5-engine/src/conversion/view-selection-to-model-converters.js

@@ -28,13 +28,13 @@ export function convertSelectionChange( modelDocument, mapper ) {
 	return ( evt, data ) => {
 		modelDocument.enqueueChanges( () => {
 			const viewSelection = data.newSelection;
-
-			modelDocument.selection.removeAllRanges();
+			const ranges = [];
 
 			for ( let viewRange of viewSelection.getRanges() ) {
-				const modelRange = mapper.toModelRange( viewRange );
-				modelDocument.selection.addRange( modelRange, viewSelection.isBackward );
+				ranges.push( mapper.toModelRange( viewRange ) );
 			}
+
+			modelDocument.selection.setRanges( ranges, viewSelection.isBackward );
 		} );
 	};
 }

+ 4 - 1
packages/ckeditor5-engine/src/dev-utils/model.js

@@ -148,7 +148,10 @@ export function setData( document, data, options = {} ) {
 			}
 
 			document.selection.setRanges( ranges, selection.isBackward );
-			document.selection.setAttributesTo( selection.getAttributes() );
+
+			if ( options.selectionAttributes ) {
+				document.selection.setAttributesTo( selection.getAttributes() );
+			}
 		}
 	} );
 }

+ 24 - 9
packages/ckeditor5-engine/src/model/document.js

@@ -7,6 +7,8 @@
 import deltas from './delta/basic-deltas.js'; // jshint ignore:line
 import transformations from './delta/basic-transformations.js'; // jshint ignore:line
 
+import Range from './range.js';
+import Position from './position.js';
 import RootElement from './rootelement.js';
 import Batch from './batch.js';
 import History from './history.js';
@@ -103,15 +105,6 @@ export default class Document {
 		 */
 		this._roots = new Map();
 
-		// Add events that will update selection attributes.
-		this.selection.on( 'change:range', () => {
-			this.selection._updateAttributes();
-		} );
-
-		this.on( 'changesDone', () => {
-			this.selection._updateAttributes();
-		} );
-
 		// Add events that will ensure selection correctness.
 		this.selection.on( 'change:range', () => {
 			for ( let range of this.selection.getRanges() ) {
@@ -324,6 +317,28 @@ export default class Document {
 	}
 
 	/**
+	 * Returns a default range for this selection. The default range is a collapsed range that starts and ends
+	 * at the beginning of this selection's document's {@link engine.model.Document#_getDefaultRoot default root}.
+	 *
+	 * @protected
+	 * @returns {engine.model.Range}
+	 */
+	_getDefaultRange() {
+		const defaultRoot = this._getDefaultRoot();
+
+		// Find the first position where the selection can be put.
+		for ( let position of Range.createIn( defaultRoot ).getPositions() ) {
+			if ( this.schema.check( { name: '$text', inside: position } ) ) {
+				return new Range( position, position );
+			}
+		}
+
+		const position = new Position( defaultRoot, [ 0 ] );
+
+		return new Range( position, position );
+	}
+
+	/**
 	 * Checks whether given {@link engine.model.Range range} is a valid range for
 	 * {@link engine.model.Document#selection document's selection}.
 	 *

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

@@ -72,6 +72,12 @@ export default class LiveRange extends Range {
 	 * @param {engine.model.Range} range
 	 * @returns {engine.model.LiveRange}
 	 */
+
+	/**
+	 * Fired when `LiveRange` instance is changed due to changes on {@link engine.model.Document}.
+	 *
+	 * @event engine.model.LiveRange#change
+	 */
 }
 
 /**
@@ -150,8 +156,13 @@ function transform( type, range, position ) {
 			break;
 	}
 
-	this.start = updated.start;
-	this.end = updated.end;
+	// If anything changed, update the range and fire an event.
+	if ( !updated.start.isEqual( this.start ) || !updated.end.isEqual( this.end ) ) {
+		this.start = updated.start;
+		this.end = updated.end;
+
+		this.fire( 'change' );
+	}
 }
 
 mix( LiveRange, EmitterMixin );

+ 291 - 107
packages/ckeditor5-engine/src/model/liveselection.js

@@ -3,17 +3,21 @@
  * For licensing, see LICENSE.md.
  */
 
-import LiveRange from './liverange.js';
 import Range from './range.js';
-import Position from './position.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 Selection from './selection.js';
 
 const storePrefix = 'selection:';
 
+const attrOpTypes = new Set(
+	[ 'addAttribute', 'removeAttribute', 'changeAttribute', 'addRootAttribute', 'removeRootAttribute', 'changeRootAttribute' ]
+);
+
 /**
  * `LiveSelection` is a type of {@link engine.model.Selection selection} that listens to changes on a
  * {@link engine.model.Document document} and has it ranges updated accordingly. Internal implementation of this
@@ -42,10 +46,39 @@ export default class LiveSelection extends Selection {
 		/**
 		 * Document which owns this selection.
 		 *
-		 * @private
-		 * @member {engine.model.Document} engine.model.Selection#_document
+		 * @protected
+		 * @member {engine.model.Document} engine.model.LiveSelection#_document
 		 */
 		this._document = document;
+
+		/**
+		 * Keeps mapping of attribute name to priority with which the attribute got modified (added/changed/removed)
+		 * last time. Possible values of priority are: `'low'` and `'normal'`.
+		 *
+		 * Priorities are used by internal `LiveSelection` mechanisms. All attributes set using `LiveSelection`
+		 * attributes API are set with `'normal'` priority.
+		 *
+		 * @private
+		 * @member {Map} engine.model.LiveSelection#_attributePriority
+		 */
+		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.
+		this.listenTo( this._document, 'change', ( evt, type ) => {
+			if ( attrOpTypes.has( type ) ) {
+				this._updateAttributes( false );
+			}
+		} );
 	}
 
 	/**
@@ -61,14 +94,14 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	get anchor() {
-		return super.anchor || this._getDefaultRange().start;
+		return super.anchor || this._document._getDefaultRange().start;
 	}
 
 	/**
 	 * @inheritDoc
 	 */
 	get focus() {
-		return super.focus || this._getDefaultRange().start;
+		return super.focus || this._document._getDefaultRange().start;
 	}
 
 	/**
@@ -85,6 +118,8 @@ export default class LiveSelection extends Selection {
 		for ( let i = 0; i < this._ranges.length; i++ ) {
 			this._ranges[ i ].detach();
 		}
+
+		this.stopListening();
 	}
 
 	/**
@@ -94,7 +129,7 @@ export default class LiveSelection extends Selection {
 		if ( this._ranges.length ) {
 			yield *super.getRanges();
 		} else {
-			yield this._getDefaultRange();
+			yield this._document._getDefaultRange();
 		}
 	}
 
@@ -102,62 +137,86 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	getFirstRange() {
-		return super.getFirstRange() || this._getDefaultRange();
+		return super.getFirstRange() || this._document._getDefaultRange();
 	}
 
 	/**
 	 * @inheritDoc
 	 */
 	getLastRange() {
-		return super.getLastRange() || this._getDefaultRange();
+		return super.getLastRange() || this._document._getDefaultRange();
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	removeAllRanges() {
-		this.destroy();
-		super.removeAllRanges();
-	}
+	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 ) {
+			this._storeAttribute( key, value );
+		}
 
-	/**
-	 * @inheritDoc
-	 */
-	setRanges( newRanges, isLastBackward ) {
-		this.destroy();
-		super.setRanges( newRanges, isLastBackward );
+		if ( this._setAttribute( key, value ) ) {
+			// Fire event with exact data.
+			const attributeKeys = [ key ];
+			this.fire( 'change:attribute', { attributeKeys, directChange: true } );
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	clearAttributes() {
-		this._setStoredAttributesTo( new Map() );
-		super.clearAttributes();
+	removeAttribute( key ) {
+		// Remove stored attribute from parent element if the selection is collapsed in an empty node.
+		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
+			this._removeStoredAttribute( key );
+		}
+
+		if ( this._removeAttribute( key ) ) {
+			// Fire event with exact data.
+			const attributeKeys = [ key ];
+			this.fire( 'change:attribute', { attributeKeys, directChange: true } );
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	removeAttribute( key ) {
-		this._removeStoredAttribute( key );
-		super.removeAttribute( key );
+	setAttributesTo( attrs ) {
+		attrs = toMap( attrs );
+
+		if ( this.isCollapsed && this.anchor.parent.childCount === 0 ) {
+			this._setStoredAttributesTo( attrs );
+		}
+
+		const changed = this._setAttributesTo( attrs );
+
+		if ( changed.size > 0 ) {
+			// Fire event with exact data (fire only if anything changed).
+			const attributeKeys = Array.from( changed );
+			this.fire( 'change:attribute', { attributeKeys, directChange: true } );
+		}
 	}
 
 	/**
 	 * @inheritDoc
 	 */
-	setAttribute( key, value ) {
-		this._storeAttribute( key, value );
-		super.setAttribute( key, value );
+	clearAttributes() {
+		this.setAttributesTo( [] );
 	}
 
 	/**
-	 * @inheritDoc
+	 * 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.
+	 *
+	 * @params {engine.model.Selection} otherSelection Selection to be cloned.
+	 * @returns {engine.model.LiveSelection} `Selection` instance that is a clone of given selection.
 	 */
-	setAttributesTo( attrs ) {
-		this._setStoredAttributesTo( toMap( attrs ) );
-		super.setAttributesTo( attrs );
+	static createFromSelection( otherSelection ) {
+		const selection = new this( otherSelection._document );
+		selection.setTo( otherSelection );
+
+		return selection;
 	}
 
 	/**
@@ -171,37 +230,189 @@ export default class LiveSelection extends Selection {
 	 * @inheritDoc
 	 */
 	_pushRange( range ) {
+		if ( !( range instanceof Range ) ) {
+			throw new CKEditorError( 'model-selection-added-not-range: Trying to add an object that is not an instance of Range.' );
+		}
+
 		this._checkRange( range );
-		this._ranges.push( LiveRange.createFromRange( range ) );
+
+		const liveRange = LiveRange.createFromRange( range );
+		this.listenTo( liveRange, 'change', () => {
+			this.fire( 'change:range', { directChange: false } );
+		} );
+
+		this._ranges.push( liveRange );
+	}
+
+	/**
+	 * Updates this selection attributes according to it's ranges and the {@link engine.model.Document model document}.
+	 *
+	 * @protected
+	 * @param {Boolean} clearAll
+	 * @fires engine.model.LiveSelection#change:attribute
+	 */
+	_updateAttributes( clearAll ) {
+		const newAttributes = toMap( this._getSurroundingAttributes() );
+		const oldAttributes = toMap( this.getAttributes() );
+
+		if ( clearAll ) {
+			// If `clearAll` remove all attributes and reset priorities.
+			this._attributePriority = new Map();
+			this._attrs = new Map();
+		} else {
+			// If not, remove only attributes added with `low` priority.
+			for ( let [ key, priority ] of this._attributePriority ) {
+				if ( priority == 'low' ) {
+					this._attrs.delete( key );
+					this._attributePriority.delete( key );
+				}
+			}
+		}
+
+		this._setAttributesTo( newAttributes, false );
+
+		// Let's evaluate which attributes really changed.
+		const changed = [];
+
+		// First, loop through all attributes that are set on selection right now.
+		// Check which of them are different than old attributes.
+		for ( let [ newKey, newValue ] of this.getAttributes() ) {
+			if ( !oldAttributes.has( newKey ) || oldAttributes.get( newKey ) !== newValue ) {
+				changed.push( newKey );
+			}
+		}
+
+		// Then, check which of old attributes got removed.
+		for ( let [ oldKey ] of oldAttributes ) {
+			if ( !this.hasAttribute( oldKey ) ) {
+				changed.push( oldKey );
+			}
+		}
+
+		// Fire event with exact data (fire only if anything changed).
+		if ( changed.length > 0 ) {
+			this.fire( 'change:attribute', { attributeKeys: changed, directChange: false } );
+		}
+	}
+
+	/**
+	 * Generates and returns an attribute key for selection attributes store, basing on original attribute key.
+	 *
+	 * @protected
+	 * @param {String} key Attribute key to convert.
+	 * @returns {String} Converted attribute key, applicable for selection store.
+	 */
+	static _getStoreAttributeKey( key ) {
+		return storePrefix + key;
 	}
 
 	/**
-	 * Returns a default range for this selection. The default range is a collapsed range that starts and ends
-	 * at the beginning of this selection's document's {@link engine.model.Document#_getDefaultRoot default root}.
-	 * This "artificial" range is important for algorithms that base on selection, so they won't break or need
-	 * special logic if there are no real ranges in the selection.
+	 * Internal method for setting `LiveSelection` attribute. Supports attribute priorities (through `directChange`
+	 * parameter).
 	 *
 	 * @private
-	 * @returns {engine.model.Range}
+	 * @param {String} key Attribute key.
+	 * @param {*} value Attribute value.
+	 * @param {Boolean} [directChange=true] `true` if the change is caused by `Selection` API, `false` if change
+	 * is caused by `Batch` API.
+	 * @returns {Boolean} Whether value has changed.
 	 */
-	_getDefaultRange() {
-		const defaultRoot = this._document._getDefaultRoot();
+	_setAttribute( key, value, directChange = true ) {
+		const priority = directChange ? 'normal' : 'low';
+
+		if ( priority == 'low' && this._attributePriority.get( key ) == 'normal' ) {
+			// Priority too low.
+			return false;
+		}
+
+		const oldValue = super.getAttribute( key );
+
+		// Don't do anything if value has not changed.
+		if ( oldValue === value ) {
+			return false;
+		}
+
+		this._attrs.set( key, value );
+
+		// Update priorities map.
+		this._attributePriority.set( key, priority );
+
+		return true;
+	}
+
+	/**
+	 * Internal method for removing `LiveSelection` attribute. Supports attribute priorities (through `directChange`
+	 * parameter).
+	 *
+	 * @private
+	 * @param {String} key Attribute key.
+	 * @param {Boolean} [directChange=true] `true` if the change is caused by `Selection` API, `false` if change
+	 * is caused by `Batch` API.
+	 * @returns {Boolean} Whether attribute was removed. May not be true if such attributes didn't exist or the
+	 * existing attribute had higher priority.
+	 */
+	_removeAttribute( key, directChange = true ) {
+		const priority = directChange ? 'normal' : 'low';
+
+		if ( priority == 'low' && this._attributePriority.get( key ) == 'normal' ) {
+			// Priority too low.
+			return false;
+		}
+
+		// Don't do anything if value has not changed.
+		if ( !super.hasAttribute( key ) ) {
+			return false;
+		}
 
-		// Find the first position where the selection can be put.
-		for ( let position of Range.createIn( defaultRoot ).getPositions() ) {
-			if ( this._document.schema.check( { name: '$text', inside: position } ) ) {
-				return new Range( position, position );
+		this._attrs.delete( key );
+
+		// Update priorities map.
+		this._attributePriority.set( key, priority );
+
+		return true;
+	}
+
+	/**
+	 * Internal method for setting multiple `LiveSelection` attributes. Supports attribute priorities (through
+	 * `directChange` parameter).
+	 *
+	 * @private
+	 * @param {Iterable|Object} attrs Iterable object containing attributes to be set.
+	 * @param {Boolean} [directChange=true] `true` if the change is caused by `Selection` API, `false` if change
+	 * is caused by `Batch` API.
+	 * @returns {Set.<String>} Changed attribute keys.
+	 */
+	_setAttributesTo( attrs, directChange = true ) {
+		const changed = new Set();
+
+		for ( let [ oldKey, oldValue ] of this.getAttributes() ) {
+			// Do not remove attribute if attribute with same key and value is about to be set.
+			if ( attrs.get( oldKey ) === oldValue ) {
+				continue;
+			}
+
+			// Attribute still might not get removed because of priorities.
+			if ( this._removeAttribute( oldKey, directChange ) ) {
+				changed.add( oldKey );
 			}
 		}
 
-		const position = new Position( defaultRoot, [ 0 ] );
+		for ( let [ key, value ] of attrs ) {
+			// Attribute may not be set because of attributes or because same key/value is already added.
+			const gotAdded = this._setAttribute( key, value, directChange );
 
-		return new Range( position, position );
+			if ( gotAdded ) {
+				changed.add( key );
+			}
+		}
+
+		return changed;
 	}
 
 	/**
-	 * Iterates through all attributes stored in current selection's parent.
+	 * Returns an iterator that iterates through all selection attributes stored in current selection's parent.
 	 *
+	 * @private
 	 * @returns {Iterable.<*>}
 	 */
 	*_getStoredAttributes() {
@@ -225,72 +436,56 @@ export default class LiveSelection extends Selection {
 	 * @param {String} key Key of attribute to remove.
 	 */
 	_removeStoredAttribute( key ) {
-		const selectionParent = this.getFirstPosition().parent;
-
-		if ( this.isCollapsed && selectionParent.childCount === 0 ) {
-			const storeKey = LiveSelection._getStoreAttributeKey( key );
+		const storeKey = LiveSelection._getStoreAttributeKey( key );
 
-			this._document.enqueueChanges( () => {
-				this._document.batch().removeAttribute( selectionParent, storeKey );
-			} );
-		}
+		this._document.batch().removeAttribute( this.anchor.parent, storeKey );
 	}
 
 	/**
-	 * Stores given attribute key and value in current selection's parent node if the selection is collapsed and
-	 * the parent node is empty.
+	 * Stores given attribute key and value in current selection's parent node.
 	 *
 	 * @private
 	 * @param {String} key Key of attribute to set.
 	 * @param {*} value Attribute value.
 	 */
 	_storeAttribute( key, value ) {
-		const selectionParent = this.getFirstPosition().parent;
-
-		if ( this.isCollapsed && selectionParent.childCount === 0 ) {
-			const storeKey = LiveSelection._getStoreAttributeKey( key );
+		const storeKey = LiveSelection._getStoreAttributeKey( key );
 
-			this._document.enqueueChanges( () => {
-				this._document.batch().setAttribute( selectionParent, storeKey, value );
-			} );
-		}
+		this._document.batch().setAttribute( this.anchor.parent, storeKey, value );
 	}
 
 	/**
 	 * Sets selection attributes stored in current selection's parent node to given set of attributes.
 	 *
-	 * @param {Iterable|Object} attrs Iterable object containing attributes to be set.
 	 * @private
+	 * @param {Iterable|Object} attrs Iterable object containing attributes to be set.
 	 */
 	_setStoredAttributesTo( attrs ) {
-		const selectionParent = this.getFirstPosition().parent;
-
-		if ( this.isCollapsed && selectionParent.childCount === 0 ) {
-			this._document.enqueueChanges( () => {
-				const batch = this._document.batch();
+		const selectionParent = this.anchor.parent;
+		const batch = this._document.batch();
 
-				for ( let attr of this._getStoredAttributes() ) {
-					const storeKey = LiveSelection._getStoreAttributeKey( attr[ 0 ] );
+		for ( let [ oldKey ] of this._getStoredAttributes() ) {
+			const storeKey = LiveSelection._getStoreAttributeKey( oldKey );
 
-					batch.removeAttribute( selectionParent, storeKey );
-				}
+			batch.removeAttribute( selectionParent, storeKey );
+		}
 
-				for ( let attr of attrs ) {
-					const storeKey = LiveSelection._getStoreAttributeKey( attr[ 0 ] );
+		for ( let [ key, value ] of attrs ) {
+			const storeKey = LiveSelection._getStoreAttributeKey( key );
 
-					batch.setAttribute( selectionParent, storeKey, attr[ 1 ] );
-				}
-			} );
+			batch.setAttribute( selectionParent, storeKey, value );
 		}
 	}
 
 	/**
-	 * Updates this selection attributes according to it's ranges and the document.
+	 * Checks model text nodes that are closest to the selection's first position and returns attributes of first
+	 * found element. If there are no text nodes in selection's first position parent, it returns selection
+	 * attributes stored in that parent.
 	 *
-	 * @fires engine.model.LiveSelection#change:attribute
-	 * @protected
+	 * @private
+	 * @returns {Iterable.<*>} Collection of attributes.
 	 */
-	_updateAttributes() {
+	_getSurroundingAttributes() {
 		const position = this.getFirstPosition();
 
 		let attrs = null;
@@ -347,30 +542,19 @@ export default class LiveSelection extends Selection {
 			}
 		}
 
-		if ( attrs ) {
-			this._attrs = new Map( attrs );
-		} else {
-			this.clearAttributes();
-		}
-
-		function getAttrsIfCharacter( node ) {
-			if ( node instanceof TextProxy || node instanceof Text ) {
-				return node.getAttributes();
-			}
-
-			return null;
-		}
-
-		this.fire( 'change:attribute' );
+		return attrs;
 	}
+}
 
-	/**
-	 * Generates and returns an attribute key for selection attributes store, basing on original attribute key.
-	 *
-	 * @param {String} key Attribute key to convert.
-	 * @returns {String} Converted attribute key, applicable for selection store.
-	 */
-	static _getStoreAttributeKey( key ) {
-		return storePrefix + key;
+// Helper function for {@link engine.model.LiveSelection#_updateAttributes}. It takes model item, checks whether
+// it is a text node (or text proxy) and if so, returns it's attributes. If not, returns `null`.
+//
+// @param {engine.model.Item}  node
+// @returns {Boolean}
+function getAttrsIfCharacter( node ) {
+	if ( node instanceof TextProxy || node instanceof Text ) {
+		return node.getAttributes();
 	}
+
+	return null;
 }

+ 5 - 0
packages/ckeditor5-engine/src/model/schema.js

@@ -178,6 +178,11 @@ export class SchemaItem {
 
 			// Now we have to check every item name from the path to check.
 			for ( let checkName of checkPath ) {
+				// Don't check items that are not registered in schema.
+				if ( !this._schema.hasItem( checkName ) ) {
+					continue;
+				}
+
 				// Every item name is expanded to all names of items that item is extending.
 				// So, if on item path, there is an item that is extended by item from checked path, it will
 				// also be treated as matching.

+ 133 - 63
packages/ckeditor5-engine/src/model/selection.js

@@ -9,6 +9,7 @@ import EmitterMixin from '../../utils/emittermixin.js';
 import CKEditorError from '../../utils/ckeditorerror.js';
 import mix from '../../utils/mix.js';
 import toMap from '../../utils/tomap.js';
+import mapsEqual from '../../utils/mapsequal.js';
 
 /**
  * `Selection` is a group of {@link engine.model.Range ranges} which has a direction specified by
@@ -244,7 +245,7 @@ export default class Selection {
 		this._pushRange( range );
 		this._lastRangeBackward = !!isBackward;
 
-		this.fire( 'change:range' );
+		this.fire( 'change:range', { directChange: true } );
 	}
 
 	/**
@@ -253,9 +254,10 @@ export default class Selection {
 	 * @fires engine.model.Selection#change:range
 	 */
 	removeAllRanges() {
-		this._ranges = [];
-
-		this.fire( 'change:range' );
+		if ( this._ranges.length > 0 ) {
+			this._removeAllRanges();
+			this.fire( 'change:range', { directChange: true } );
+		}
 	}
 
 	/**
@@ -269,8 +271,26 @@ export default class Selection {
 	 * @param {Boolean} [isLastBackward=false] Flag describing if last added range was selected forward - from start to end (`false`)
 	 * or backward - from end to start (`true`).
 	 */
-	setRanges( newRanges, isLastBackward ) {
-		this._ranges = [];
+	setRanges( newRanges, isLastBackward = false ) {
+		newRanges = Array.from( newRanges );
+
+		// Check whether there is any range in new ranges set that is different than all already added ranges.
+		const anyNewRange = newRanges.some( ( newRange ) => {
+			if ( !( newRange instanceof Range ) ) {
+				throw new CKEditorError( 'model-selection-added-not-range: Trying to add an object that is not an instance of Range.' );
+			}
+
+			return this._ranges.every( ( oldRange ) => {
+				return !oldRange.isEqual( newRange );
+			} );
+		} );
+
+		// Don't do anything if nothing changed.
+		if ( newRanges.length === this._ranges.length && !anyNewRange ) {
+			return;
+		}
+
+		this._removeAllRanges();
 
 		for ( let range of newRanges ) {
 			this._pushRange( range );
@@ -278,7 +298,7 @@ export default class Selection {
 
 		this._lastRangeBackward = !!isLastBackward;
 
-		this.fire( 'change:range' );
+		this.fire( 'change:range', { directChange: true } );
 	}
 
 	/**
@@ -338,6 +358,45 @@ export default class Selection {
 	}
 
 	/**
+	 * Sets {@link engine.model.Selection#focus} to the specified location.
+	 *
+	 * The location can be specified in the same form as {@link engine.model.Position.createAt} parameters.
+	 *
+	 * @fires engine.model.Selection#change:range
+	 * @param {engine.model.Item|engine.model.Position} itemOrPosition
+	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
+	 * first parameter is a {@link engine.model.Item model item}.
+	 */
+	setFocus( itemOrPosition, offset ) {
+		if ( this.anchor === null ) {
+			/**
+			 * Cannot set selection focus if there are no ranges in selection.
+			 *
+			 * @error model-selection-setFocus-no-ranges
+			 */
+			throw new CKEditorError( 'model-selection-setFocus-no-ranges: Cannot set selection focus if there are no ranges in selection.' );
+		}
+
+		const newFocus = Position.createAt( itemOrPosition, offset );
+
+		if ( newFocus.compareWith( this.focus ) == 'same' ) {
+			return;
+		}
+
+		const anchor = this.anchor;
+
+		if ( this._ranges.length ) {
+			this._popRange();
+		}
+
+		if ( newFocus.compareWith( anchor ) == 'before' ) {
+			this.addRange( new Range( newFocus, anchor ), true );
+		} else {
+			this.addRange( new Range( anchor, newFocus ) );
+		}
+	}
+
+	/**
 	 * Gets an attribute value for given key or `undefined` if that attribute is not set on the selection.
 	 *
 	 * @param {String} key Key of attribute to look for.
@@ -381,87 +440,81 @@ export default class Selection {
 	/**
 	 * Removes all attributes from the selection.
 	 *
+	 * If there were any attributes in selection, fires the {@link engine.model.Selection#change} event with
+	 * removed attributes' keys.
+	 *
 	 * @fires engine.model.Selection#change:attribute
 	 */
 	clearAttributes() {
-		this._attrs.clear();
+		if ( this._attrs.size > 0 ) {
+			const attributeKeys = Array.from( this._attrs.keys() );
+			this._attrs.clear();
 
-		this.fire( 'change:attribute' );
+			this.fire( 'change:attribute', { attributeKeys, directChange: true } );
+		}
 	}
 
 	/**
 	 * Removes an attribute with given key from the selection.
 	 *
+	 * If given attribute was set on the selection, fires the {@link engine.model.Selection#change} event with
+	 * removed attribute key.
+	 *
 	 * @fires engine.model.Selection#change:attribute
 	 * @param {String} key Key of attribute to remove.
 	 */
 	removeAttribute( key ) {
-		this._attrs.delete( key );
+		if ( this.hasAttribute( key ) ) {
+			this._attrs.delete( key );
 
-		this.fire( 'change:attribute' );
+			this.fire( 'change:attribute', { attributeKeys: [ key ], directChange: true } );
+		}
 	}
 
 	/**
 	 * Sets attribute on the selection. If attribute with the same key already is set, it's value is overwritten.
 	 *
+	 * If the attribute value has changed, fires the {@link engine.model.Selection#change} event with
+	 * the attribute key.
+	 *
 	 * @fires engine.model.Selection#change:attribute
 	 * @param {String} key Key of attribute to set.
 	 * @param {*} value Attribute value.
 	 */
 	setAttribute( key, value ) {
-		this._attrs.set( key, value );
+		if ( this.getAttribute( key ) !== value ) {
+			this._attrs.set( key, value );
 
-		this.fire( 'change:attribute' );
+			this.fire( 'change:attribute', { attributeKeys: [ key ], directChange: true } );
+		}
 	}
 
 	/**
 	 * Removes all attributes from the selection and sets given attributes.
 	 *
+	 * If given set of attributes is different than set of attributes already added to selection, fires
+	 * {@link engine.model.Selection#change change event} with keys of attributes that changed.
+	 *
 	 * @fires engine.model.Selection#change:attribute
 	 * @param {Iterable|Object} attrs Iterable object containing attributes to be set.
 	 */
 	setAttributesTo( attrs ) {
-		this._attrs = toMap( attrs );
+		attrs = toMap( attrs );
 
-		this.fire( 'change:attribute' );
-	}
+		if ( !mapsEqual( attrs, this._attrs ) ) {
+			// Create a set from keys of old and new attributes.
+			const changed = new Set( Array.from( attrs.keys() ).concat( Array.from( this._attrs.keys() ) ) );
 
-	/**
-	 * Sets {@link engine.model.Selection#focus} to the specified location.
-	 *
-	 * The location can be specified in the same form as {@link engine.model.Position.createAt} parameters.
-	 *
-	 * @fires engine.model.Selection#change:range
-	 * @param {engine.model.Item|engine.model.Position} itemOrPosition
-	 * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when
-	 * first parameter is a {@link engine.model.Item model item}.
-	 */
-	setFocus( itemOrPosition, offset ) {
-		if ( this.anchor === null ) {
-			/**
-			 * Cannot set selection focus if there are no ranges in selection.
-			 *
-			 * @error model-selection-setFocus-no-ranges
-			 */
-			throw new CKEditorError( 'model-selection-setFocus-no-ranges: Cannot set selection focus if there are no ranges in selection.' );
-		}
-
-		const newFocus = Position.createAt( itemOrPosition, offset );
-
-		if ( newFocus.compareWith( this.focus ) == 'same' ) {
-			return;
-		}
+			for ( let [ key, value ] of attrs ) {
+				// If the attribute remains unchanged, remove it from changed set.
+				if ( this._attrs.get( key ) === value ) {
+					changed.delete( key );
+				}
+			}
 
-		const anchor = this.anchor;
+			this._attrs = attrs;
 
-		if ( this._ranges.length ) {
-			this._popRange();
-		}
-
-		if ( newFocus.compareWith( anchor ) == 'before' ) {
-			this.addRange( new Range( newFocus, anchor ), true );
-		} else {
-			this.addRange( new Range( anchor, newFocus ) );
+			this.fire( 'change:attribute', { attributeKeys: Array.from( changed ), directChange: true } );
 		}
 	}
 
@@ -498,8 +551,8 @@ export default class Selection {
 	/**
 	 * Checks if given range intersects with ranges that are already in the selection. Throws an error if it does.
 	 *
-	 * @param {engine.model.Range} range Range to check.
 	 * @protected
+	 * @param {engine.model.Range} range Range to check.
 	 */
 	_checkRange( range ) {
 		for ( let i = 0; i < this._ranges.length; i++ ) {
@@ -527,18 +580,35 @@ export default class Selection {
 	_popRange() {
 		this._ranges.pop();
 	}
-}
 
-mix( Selection, EmitterMixin );
+	/**
+	 * Deletes ranges from internal range array. Uses {@link engine.model.Selection#_popRange _popRange} to
+	 * ensure proper ranges removal.
+	 *
+	 * @private
+	 */
+	_removeAllRanges() {
+		while ( this._ranges.length > 0 ) {
+			this._popRange();
+		}
+	}
 
-/**
- * Fired whenever selection ranges are changed through {@link engine.model.Selection Selection API}.
- *
- * @event engine.model.Selection#change:range
- */
+	/**
+	 * Fired whenever selection ranges are changed.
+	 *
+	 * @event engine.model.Selection#change:range
+	 * @param {Boolean} directChange Specifies whether the range change was caused by direct usage of `Selection` API (`true`)
+	 * or by changes done to {@link engine.model.Document model document} using {@link engine.model.Batch Batch} API (`false`).
+	 */
 
-/**
- * Fired whenever selection attributes are changed.
- *
- * @event engine.model.Selection#change:attribute
- */
+	/**
+	 * Fired whenever selection attributes are changed.
+	 *
+	 * @event engine.model.Selection#change:attribute
+	 * @param {Boolean} directChange Specifies whether the attributes changed by direct usage of the Selection API (`true`)
+	 * or by changes done to the {@link engine.model.Document model document} using the {@link engine.model.Batch Batch} API (`false`).
+	 * @param {Array.<String>} attributeKeys Array containing keys of attributes that changed.
+	 */
+}
+
+mix( Selection, EmitterMixin );

+ 3 - 3
packages/ckeditor5-engine/tests/model/document/document.js

@@ -228,15 +228,15 @@ describe( 'Document', () => {
 		it( 'should get updated attributes whenever selection gets updated', () => {
 			sinon.spy( doc.selection, '_updateAttributes' );
 
-			doc.selection.fire( 'change:range' );
+			doc.selection.fire( 'change:range', { directChange: true } );
 
 			expect( doc.selection._updateAttributes.called ).to.be.true;
 		} );
 
-		it( 'should get updated attributes whenever changes to the document are applied', () => {
+		it( 'should get updated attributes whenever attribute operation is applied', () => {
 			sinon.spy( doc.selection, '_updateAttributes' );
 
-			doc.fire( 'changesDone' );
+			doc.fire( 'change', 'addAttribute' );
 
 			expect( doc.selection._updateAttributes.called ).to.be.true;
 		} );

+ 181 - 40
packages/ckeditor5-engine/tests/model/liveselection.js

@@ -14,6 +14,8 @@ 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 AttributeOperation from '/ckeditor5/engine/model/operation/attributeoperation.js';
+import CKEditorError from '/ckeditor5/utils/ckeditorerror.js';
 import count from '/ckeditor5/utils/count.js';
 import testUtils from '/tests/core/_utils/utils.js';
 import { wrapInDelta } from '/tests/engine/model/_utils/utils.js';
@@ -127,6 +129,12 @@ describe( 'LiveSelection', () => {
 
 			expect( ranges[ 0 ] ).to.be.instanceof( LiveRange );
 		} );
+
+		it( 'should throw an error when range is invalid', () => {
+			expect( () => {
+				selection.addRange( { invalid: 'range' } );
+			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
+		} );
 	} );
 
 	describe( 'collapse', () => {
@@ -196,7 +204,7 @@ describe( 'LiveSelection', () => {
 			spy = sinon.spy();
 			selection.on( 'change:range', spy );
 
-			ranges = selection._ranges;
+			ranges = Array.from( selection._ranges );
 
 			sinon.spy( ranges[ 0 ], 'detach' );
 			sinon.spy( ranges[ 1 ], 'detach' );
@@ -222,35 +230,23 @@ describe( 'LiveSelection', () => {
 	} );
 
 	describe( 'setRanges', () => {
-		let newRanges, spy, oldRanges;
-
-		before( () => {
-			newRanges = [
-				new Range( new Position( root, [ 4 ] ), new Position( root, [ 5 ] ) ),
-				new Range( new Position( root, [ 5, 0 ] ), new Position( root, [ 6, 0 ] ) )
-			];
+		it( 'should throw an error when range is invalid', () => {
+			expect( () => {
+				selection.setRanges( [ { invalid: 'range' } ] );
+			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
 		} );
 
-		beforeEach( () => {
+		it( 'should detach removed ranges', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
 
-			spy = sinon.spy();
-			selection.on( 'change:range', spy );
-
-			oldRanges = selection._ranges;
+			const oldRanges = Array.from( selection._ranges );
 
 			sinon.spy( oldRanges[ 0 ], 'detach' );
 			sinon.spy( oldRanges[ 1 ], 'detach' );
-		} );
 
-		afterEach( () => {
-			oldRanges[ 0 ].detach.restore();
-			oldRanges[ 1 ].detach.restore();
-		} );
+			selection.setRanges( [] );
 
-		it( 'should detach removed ranges', () => {
-			selection.setRanges( newRanges );
 			expect( oldRanges[ 0 ].detach.called ).to.be.true;
 			expect( oldRanges[ 1 ].detach.called ).to.be.true;
 		} );
@@ -284,8 +280,8 @@ describe( 'LiveSelection', () => {
 
 	// LiveSelection uses LiveRanges so here are only simple test to see if integration is
 	// working well, without getting into complicated corner cases.
-	describe( 'after applying an operation should get updated and not fire update event', () => {
-		let spy;
+	describe( 'after applying an operation should get updated and fire events', () => {
+		let spyRange;
 
 		beforeEach( () => {
 			root.insertChildren( 0, [
@@ -296,12 +292,16 @@ describe( 'LiveSelection', () => {
 
 			selection.addRange( new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 1, 4 ] ) ) );
 
-			spy = sinon.spy();
-			selection.on( 'change:range', spy );
+			spyRange = sinon.spy();
+			selection.on( 'change:range', spyRange );
 		} );
 
 		describe( 'InsertOperation', () => {
 			it( 'before selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new InsertOperation(
 						new Position( root, [ 0, 1 ] ),
@@ -310,14 +310,18 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 5 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 4 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 
 			it( 'inside selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new InsertOperation(
 						new Position( root, [ 1, 0 ] ),
@@ -326,16 +330,20 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 2 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 7 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 		} );
 
 		describe( 'MoveOperation', () => {
 			it( 'move range from before a selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new MoveOperation(
 						new Position( root, [ 0, 0 ] ),
@@ -345,14 +353,18 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 0 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 4 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 
 			it( 'moved into before a selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new MoveOperation(
 						new Position( root, [ 2 ] ),
@@ -362,14 +374,18 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 4 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 4 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 
 			it( 'move range from inside of selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new MoveOperation(
 						new Position( root, [ 1, 0 ] ),
@@ -379,14 +395,18 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 2 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 2 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 
 			it( 'moved range intersects with selection', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new MoveOperation(
 						new Position( root, [ 1, 3 ] ),
@@ -396,14 +416,18 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 2 ] );
 				expect( range.end.path ).to.deep.equal( [ 1, 3 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
 			} );
 
 			it( 'split inside selection (do not break selection)', () => {
+				selection.on( 'change:range', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+				} );
+
 				doc.applyOperation( wrapInDelta(
 					new InsertOperation(
 						new Position( root, [ 2 ] ),
@@ -421,11 +445,77 @@ describe( 'LiveSelection', () => {
 					)
 				) );
 
-				let range = selection._ranges[ 0 ];
+				let range = selection.getFirstRange();
 
 				expect( range.start.path ).to.deep.equal( [ 0, 2 ] );
 				expect( range.end.path ).to.deep.equal( [ 2, 2 ] );
-				expect( spy.called ).to.be.false;
+				expect( spyRange.calledOnce ).to.be.true;
+			} );
+		} );
+
+		describe( 'AttributeOperation', () => {
+			it( 'changed range includes selection anchor', () => {
+				const spyAttribute = sinon.spy();
+				selection.on( 'change:attribute', spyAttribute );
+
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.false;
+					expect( data.attributeKeys ).to.deep.equal( [ 'foo' ] );
+				} );
+
+				doc.applyOperation( wrapInDelta(
+					new AttributeOperation(
+						new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 5 ] ) ),
+						'foo',
+						null,
+						'bar',
+						doc.version
+					)
+				) );
+
+				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
+				expect( spyAttribute.calledOnce ).to.be.true;
+			} );
+
+			it( 'should not overwrite previously set attributes', () => {
+				selection.setAttribute( 'foo', 'xyz' );
+
+				const spyAttribute = sinon.spy();
+				selection.on( 'change:attribute', spyAttribute );
+
+				doc.applyOperation( wrapInDelta(
+					new AttributeOperation(
+						new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 5 ] ) ),
+						'foo',
+						null,
+						'bar',
+						doc.version
+					)
+				) );
+
+				expect( selection.getAttribute( 'foo' ) ).to.equal( 'xyz' );
+				expect( spyAttribute.called ).to.be.false;
+			} );
+
+			it( 'should not overwrite previously removed attributes', () => {
+				selection.setAttribute( 'foo', 'xyz' );
+				selection.removeAttribute( 'foo' );
+
+				const spyAttribute = sinon.spy();
+				selection.on( 'change:attribute', spyAttribute );
+
+				doc.applyOperation( wrapInDelta(
+					new AttributeOperation(
+						new Range( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 5 ] ) ),
+						'foo',
+						null,
+						'bar',
+						doc.version
+					)
+				) );
+
+				expect( selection.hasAttribute( 'foo' ) ).to.be.false;
+				expect( spyAttribute.called ).to.be.false;
 			} );
 		} );
 	} );
@@ -458,6 +548,30 @@ describe( 'LiveSelection', () => {
 		} );
 
 		describe( 'setAttributesTo', () => {
+			it( 'should fire change:attribute event with correct parameters', ( done ) => {
+				selection.setAttributesTo( { foo: 'bar', abc: 'def' } );
+
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.true;
+					expect( data.attributeKeys ).to.deep.equal( [ 'abc', 'xxx' ] );
+
+					done();
+				} );
+
+				selection.setAttributesTo( { foo: 'bar', xxx: 'yyy' } );
+			} );
+
+			it( 'should not fire change:attribute event if same attributes are set', () => {
+				selection.setAttributesTo( { foo: 'bar', abc: 'def' } );
+
+				const spy = sinon.spy();
+				selection.on( 'change:attribute', spy );
+
+				selection.setAttributesTo( { foo: 'bar', abc: 'def' } );
+
+				expect( spy.called ).to.be.false;
+			} );
+
 			it( 'should remove all stored attributes and store the given ones if the selection is in empty node', () => {
 				selection.setRanges( [ rangeInEmptyP ] );
 				selection.setAttribute( 'abc', 'xyz' );
@@ -510,7 +624,7 @@ describe( 'LiveSelection', () => {
 		} );
 	} );
 
-	describe( '_updateAttributes', () => {
+	describe( 'update attributes on direct range change', () => {
 		beforeEach( () => {
 			root.insertChildren( 0, [
 				new Element( 'p', { p: true } ),
@@ -562,13 +676,40 @@ describe( 'LiveSelection', () => {
 			expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [] );
 		} );
 
+		it( 'should overwrite any previously set attributes', () => {
+			selection.collapse( new Position( root, [ 5, 0 ] ) );
+
+			selection.setAttribute( 'x', true );
+			selection.setAttribute( 'y', true );
+
+			expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [ [ 'd', true ], [ 'x', true ], [ 'y', true ] ] );
+
+			selection.collapse( new Position( root, [ 1 ] ) );
+
+			expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [ [ 'a', true ] ] );
+		} );
+
 		it( 'should fire change:attribute event', () => {
 			let spy = sinon.spy();
 			selection.on( 'change:attribute', spy );
 
 			selection.setRanges( [ new Range( new Position( root, [ 2 ] ), new Position( root, [ 5 ] ) ) ] );
 
-			expect( spy.called ).to.be.true;
+			expect( spy.calledOnce ).to.be.true;
+		} );
+
+		it( 'should not fire change:attribute event if attributes did not change', () => {
+			selection.collapse( new Position( root, [ 5, 0 ] ) );
+
+			expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [ [ 'd', true ] ] );
+
+			let spy = sinon.spy();
+			selection.on( 'change:attribute', spy );
+
+			selection.collapse( new Position( root, [ 5, 1 ] ) );
+
+			expect( Array.from( selection.getAttributes() ) ).to.deep.equal( [ [ 'd', true ] ] );
+			expect( spy.called ).to.be.false;
 		} );
 	} );
 

+ 4 - 0
packages/ckeditor5-engine/tests/model/schema/schema.js

@@ -208,6 +208,10 @@ describe( 'check', () => {
 			// Even if image has src and alt, it can't have attributes that weren't allowed
 			expect( schema.check( { name: 'img', inside: '$block', attributes: [ 'alt', 'src', 'attr' ] } ) ).to.be.false;
 		} );
+
+		it( 'should omit path elements that are added to schema', () => {
+			expect( schema.check( { name: '$inline', inside: '$block new $block' } ) ).to.be.true;
+		} );
 	} );
 
 	describe( 'array of elements as inside', () => {

+ 151 - 34
packages/ckeditor5-engine/tests/model/selection.js

@@ -106,12 +106,6 @@ describe( 'Selection', () => {
 	} );
 
 	describe( 'addRange', () => {
-		it( 'should throw an error when range is invalid', () => {
-			expect( () => {
-				selection.addRange( { invalid: 'Range' } );
-			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
-		} );
-
 		it( 'should copy added ranges and store multiple ranges', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
@@ -169,6 +163,12 @@ describe( 'Selection', () => {
 			expect( spy.called ).to.be.true;
 		} );
 
+		it( 'should throw an error when range is invalid', () => {
+			expect( () => {
+				selection.addRange( { invalid: 'range' } );
+			} ).to.throw( CKEditorError, /model-selection-added-not-range/ );
+		} );
+
 		it( 'should throw an error if added range intersects with already stored range', () => {
 			selection.addRange( liveRange );
 
@@ -457,40 +457,48 @@ describe( 'Selection', () => {
 	} );
 
 	describe( 'removeAllRanges', () => {
-		let spy, ranges;
+		let spy;
 
-		beforeEach( () => {
+		it( 'should remove all stored ranges', () => {
+			selection.addRange( liveRange );
+			selection.addRange( range );
+
+			selection.removeAllRanges();
+
+			expect( Array.from( selection.getRanges() ).length ).to.equal( 0 );
+		} );
+
+		it( 'should fire exactly one change:range event', () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
 
 			spy = sinon.spy();
 			selection.on( 'change:range', spy );
 
-			ranges = selection._ranges;
-
 			selection.removeAllRanges();
-		} );
 
-		it( 'should remove all stored ranges', () => {
-			expect( Array.from( selection.getRanges() ).length ).to.equal( 0 );
+			expect( spy.calledOnce ).to.be.true;
 		} );
 
-		it( 'should fire exactly one update event', () => {
-			expect( spy.calledOnce ).to.be.true;
+		it( 'should not fire change:range event if there were no ranges', () => {
+			spy = sinon.spy();
+			selection.on( 'change:range', spy );
+
+			selection.removeAllRanges();
+
+			expect( spy.called ).to.be.false;
 		} );
 	} );
 
 	describe( 'setRanges', () => {
 		let newRanges, spy, oldRanges;
 
-		before( () => {
+		beforeEach( () => {
 			newRanges = [
 				new Range( new Position( root, [ 4 ] ), new Position( root, [ 5 ] ) ),
 				new Range( new Position( root, [ 5, 0 ] ), new Position( root, [ 6, 0 ] ) )
 			];
-		} );
 
-		beforeEach( () => {
 			selection.addRange( liveRange );
 			selection.addRange( range );
 
@@ -509,11 +517,8 @@ describe( 'Selection', () => {
 		it( 'should remove all ranges and add given ranges', () => {
 			selection.setRanges( newRanges );
 
-			let ranges = selection._ranges;
-
-			expect( ranges.length ).to.equal( 2 );
-			expect( ranges[ 0 ].isEqual( newRanges[ 0 ] ) ).to.be.true;
-			expect( ranges[ 1 ].isEqual( newRanges[ 1 ] ) ).to.be.true;
+			let ranges = Array.from( selection.getRanges() );
+			expect( ranges ).to.deep.equal( newRanges );
 		} );
 
 		it( 'should use last range from given array to get anchor and focus position', () => {
@@ -528,15 +533,28 @@ describe( 'Selection', () => {
 			expect( selection.focus.path ).to.deep.equal( [ 5, 0 ] );
 		} );
 
-		it( 'should fire exactly one update event', () => {
+		it( 'should fire exactly one change:range event', () => {
 			selection.setRanges( newRanges );
 			expect( spy.calledOnce ).to.be.true;
 		} );
+
+		it( 'should fire change:range event with correct parameters', () => {
+			selection.on( 'change:range', ( evt, data ) => {
+				expect( data.directChange ).to.be.true;
+			} );
+
+			selection.setRanges( newRanges );
+		} );
+
+		it( 'should not fire change:range event if given ranges are the same', () => {
+			selection.setRanges( [ liveRange, range ] );
+			expect( spy.calledOnce ).to.be.false;
+		} );
 	} );
 
 	describe( 'setTo', () => {
 		it( 'should set selection to be same as given selection, using setRanges method', () => {
-			sinon.spy( selection, 'setRanges' );
+			const spy = sinon.spy( selection, 'setRanges' );
 
 			const otherSelection = new Selection();
 			otherSelection.addRange( range1 );
@@ -547,6 +565,7 @@ describe( 'Selection', () => {
 			expect( Array.from( selection.getRanges() ) ).to.deep.equal( [ range1, range2 ] );
 			expect( selection.isBackward ).to.be.true;
 			expect( selection.setRanges.calledOnce ).to.be.true;
+			spy.restore();
 		} );
 	} );
 
@@ -679,11 +698,27 @@ describe( 'Selection', () => {
 	describe( 'collapseToStart', () => {
 		it( 'should collapse to start position and fire change event', () => {
 			selection.setRanges( [ range2, range1, range3 ] );
+
+			const spy = sinon.spy();
+			selection.on( 'change:range', spy );
+
 			selection.collapseToStart();
 
 			expect( selection.rangeCount ).to.equal( 1 );
 			expect( selection.isCollapsed ).to.be.true;
 			expect( selection.getFirstPosition().isEqual( range1.start ) ).to.be.true;
+			expect( spy.calledOnce ).to.be.true;
+		} );
+
+		it( 'should do nothing if selection was already collapsed', () => {
+			selection.collapse( range1.start );
+
+			const spy = sinon.spy( selection, 'fire' );
+
+			selection.collapseToStart();
+
+			expect( spy.notCalled ).to.be.true;
+			spy.restore();
 		} );
 
 		it( 'should do nothing if no ranges present', () => {
@@ -697,22 +732,38 @@ describe( 'Selection', () => {
 	} );
 
 	describe( 'collapseToEnd', () => {
-		it( 'should collapse to start position and fire change event', () => {
+		it( 'should collapse to start position and fire change:range event', () => {
 			selection.setRanges( [ range2, range3, range1 ] );
+
+			const spy = sinon.spy();
+			selection.on( 'change:range', spy );
+
 			selection.collapseToEnd();
 
 			expect( selection.rangeCount ).to.equal( 1 );
 			expect( selection.isCollapsed ).to.be.true;
 			expect( selection.getLastPosition().isEqual( range3.end ) ).to.be.true;
+			expect( spy.calledOnce ).to.be.true;
 		} );
 
-		it( 'should do nothing if no ranges present', () => {
+		it( 'should do nothing if selection was already collapsed', () => {
+			selection.collapse( range1.start );
+
 			const spy = sinon.spy( selection, 'fire' );
 
 			selection.collapseToEnd();
 
+			expect( spy.notCalled ).to.be.true;
 			spy.restore();
+		} );
+
+		it( 'should do nothing if selection has no ranges', () => {
+			const spy = sinon.spy( selection, 'fire' );
+
+			selection.collapseToEnd();
+
 			expect( spy.notCalled ).to.be.true;
+			spy.restore();
 		} );
 	} );
 
@@ -756,13 +807,24 @@ describe( 'Selection', () => {
 				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
 			} );
 
-			it( 'should fire change:attribute event', () => {
+			it( 'should fire change:attribute event with correct parameters', () => {
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.true;
+					expect( data.attributeKeys ).to.deep.equal( [ 'foo' ] );
+				} );
+
+				selection.setAttribute( 'foo', 'bar' );
+			} );
+
+			it( 'should not fire change:attribute event if attribute with same key and value was already set', () => {
+				selection.setAttribute( 'foo', 'bar' );
+
 				let spy = sinon.spy();
 				selection.on( 'change:attribute', spy );
 
 				selection.setAttribute( 'foo', 'bar' );
 
-				expect( spy.called ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
@@ -821,13 +883,24 @@ describe( 'Selection', () => {
 				expect( selection.getAttribute( 'abc' ) ).to.be.undefined;
 			} );
 
-			it( 'should fire change:attribute event', () => {
+			it( 'should fire change:attribute event with correct parameters', () => {
+				selection.setAttribute( 'foo', 'bar' );
+
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.true;
+					expect( data.attributeKeys ).to.deep.equal( [ 'foo' ] );
+				} );
+
+				selection.clearAttributes();
+			} );
+
+			it( 'should not fire change:attribute event if there were no attributes', () => {
 				let spy = sinon.spy();
 				selection.on( 'change:attribute', spy );
 
 				selection.clearAttributes();
 
-				expect( spy.called ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
@@ -840,25 +913,69 @@ describe( 'Selection', () => {
 				expect( selection.getAttribute( 'foo' ) ).to.be.undefined;
 			} );
 
-			it( 'should fire change:attribute event', () => {
+			it( 'should fire change:attribute event with correct parameters', () => {
+				selection.setAttribute( 'foo', 'bar' );
+
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.true;
+					expect( data.attributeKeys ).to.deep.equal( [ 'foo' ] );
+				} );
+
+				selection.removeAttribute( 'foo' );
+			} );
+
+			it( 'should not fire change:attribute event if such attribute did not exist', () => {
 				let spy = sinon.spy();
 				selection.on( 'change:attribute', spy );
 
 				selection.removeAttribute( 'foo' );
 
-				expect( spy.called ).to.be.true;
+				expect( spy.called ).to.be.false;
 			} );
 		} );
 
 		describe( 'setAttributesTo', () => {
 			it( 'should remove all attributes set on element and set the given ones', () => {
-				selection.setRanges( [ rangeInFullP ] );
 				selection.setAttribute( 'abc', 'xyz' );
 				selection.setAttributesTo( { foo: 'bar' } );
 
 				expect( selection.getAttribute( 'foo' ) ).to.equal( 'bar' );
 				expect( selection.getAttribute( 'abc' ) ).to.be.undefined;
 			} );
+
+			it( 'should fire only one change:attribute event', () => {
+				selection.setAttributesTo( { foo: 'bar', xxx: 'yyy' } );
+
+				let spy = sinon.spy();
+				selection.on( 'change:attribute', spy );
+
+				selection.setAttributesTo( { foo: 'bar', abc: 'def' } );
+
+				expect( spy.calledOnce ).to.be.true;
+			} );
+
+			it( 'should fire change:attribute event with correct parameters', () => {
+				selection.setAttributesTo( { foo: 'bar', xxx: 'yyy' } );
+
+				selection.on( 'change:attribute', ( evt, data ) => {
+					expect( data.directChange ).to.be.true;
+					expect( data.attributeKeys ).to.deep.equal( [ 'abc', 'xxx' ] );
+				} );
+
+				selection.setAttributesTo( { foo: 'bar', abc: 'def' } );
+			} );
+
+			it( 'should not fire change:attribute event if attributes had not changed', () => {
+				selection.setRanges( [ rangeInFullP ] );
+				selection.setAttributesTo( { foo: 'bar', xxx: 'yyy' } );
+
+				let spy = sinon.spy();
+				selection.on( 'change:attribute', spy );
+
+				selection.setAttributesTo( { xxx: 'yyy', foo: 'bar' } );
+
+				expect( spy.called ).to.be.false;
+			} );
 		} );
 	} );
 } );

+ 7 - 0
packages/ckeditor5-engine/tests/tickets/462/1.html

@@ -0,0 +1,7 @@
+<head>
+	<link rel="stylesheet" href="%APPS_DIR%ckeditor/build/modules/amd/theme/ckeditor.css">
+</head>
+
+<div id="editor">
+	<p>Lorem <strong>ipsum.</strong></p>
+</div>

+ 28 - 0
packages/ckeditor5-engine/tests/tickets/462/1.js

@@ -0,0 +1,28 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals console, window, document, setInterval */
+
+import ClassicEditor from '/ckeditor5/editor-classic/classic.js';
+
+ClassicEditor.create( document.querySelector( '#editor' ), {
+	features: [ 'enter', 'typing', 'paragraph', 'basic-styles/bold', 'basic-styles/italic' ],
+	toolbar: [ 'bold', 'italic' ]
+} )
+.then( editor => {
+	window.editor = editor;
+
+	setInterval( () => {
+		console.clear();
+
+		console.log( editor.editing.view.getDomRoot().innerHTML.replace( /\u200b/g, '@' ) );
+		console.log( 'selection.hasAttribute( italic )', editor.document.selection.hasAttribute( 'italic' ) );
+		console.log( 'selection.hasAttribute( bold )', editor.document.selection.hasAttribute( 'bold' ) );
+		console.log( 'selection anchor\'s parentNode', document.getSelection().anchorNode.parentNode );
+	}, 2000 );
+} )
+.catch( err => {
+	console.error( err.stack );
+} );

+ 27 - 0
packages/ckeditor5-engine/tests/tickets/462/1.md

@@ -0,0 +1,27 @@
+@bender-ui: collapsed
+@bender-tags: ticket, 462, iteration4
+
+### Selection attributes conversion test [#462](https://github.com/ckeditor/ckeditor5-engine/issues/462)
+
+#### Test 1
+
+1. Put caret inside unstyled word.
+2. Press <kbd>Ctrl+I</kbd>.
+
+Expected results:
+
+* `<em>@@@@@@@</em>` was inserted into editable.
+* selection has the `'italic'` attribute.
+* selection anchor's parent node is `<em>`.
+
+#### Test 2
+
+1. Put caret inside bold word.
+2. Press <kbd>Ctrl+B</kbd>.
+
+Expected results:
+
+* `<em>` element from previous test got removed.
+* `<strong>` element got broken.
+* selection doesn't have the `'bold'` attribute.
+* selection anchor's parent node is `<p>`.

+ 8 - 0
packages/ckeditor5-engine/tests/tickets/603/1.html

@@ -0,0 +1,8 @@
+<head>
+	<link rel="stylesheet" href="%APPS_DIR%ckeditor/build/modules/amd/theme/ckeditor.css">
+</head>
+
+<div id="editor">
+	<h2>Foo <em>bar</em></h2>
+	<p>Lorem <strong><em>ips</em>um</strong> dolor <strong>sit</strong> amet.</p>
+</div>

+ 26 - 0
packages/ckeditor5-engine/tests/tickets/603/1.js

@@ -0,0 +1,26 @@
+/**
+ * @license Copyright (c) 2003-2016, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+/* globals console, window, document */
+
+import ClassicEditor from '/ckeditor5/editor-classic/classic.js';
+
+ClassicEditor.create( document.querySelector( '#editor' ), {
+	features: [ 'enter', 'typing', 'paragraph', 'heading', 'basic-styles/bold', 'basic-styles/italic' ],
+	toolbar: [ 'headings', 'bold', 'italic' ]
+} )
+.then( editor => {
+	window.editor = editor;
+
+	const sel = editor.document.selection;
+
+	sel.on( 'change', ( evt, data ) => {
+		const date = new Date();
+		console.log( `${ date.getSeconds() }s${ String( date.getMilliseconds() ).slice( 0, 2 ) }ms`, evt.name, data );
+	} );
+} )
+.catch( err => {
+	console.error( err.stack );
+} );

+ 17 - 0
packages/ckeditor5-engine/tests/tickets/603/1.md

@@ -0,0 +1,17 @@
+@bender-ui: collapsed
+@bender-tags: ticket, 603, iteration4
+
+### Selection events test [#603](https://github.com/ckeditor/ckeditor5-engine/issues/603)
+
+Test that each selection change fires:
+
+* only one `change:range` for position change.
+* only one `change:attribute` for attribute(s) change (if anything has changed).
+
+Test following ways of changing selection:
+
+* clicking in various positions,
+* arrow keys,
+* applying styles,
+* enter and backspace keys,
+* typing.