Merge pull request #1421 from ckeditor/t/1418

Feature: Introduced `ModelDocument#change:data` event. Closes #1418.

Author: Piotr Jasiun

src/model/differ.js

@@ -171,7 +171,7 @@ export default class Differ {
 				for ( const marker of this._markerCollection.getMarkersIntersectingRange( range ) ) {
 					const markerRange = marker.getRange();
 
-					this.bufferMarkerChange( marker.name, markerRange, markerRange );
+					this.bufferMarkerChange( marker.name, markerRange, markerRange, marker.affectsData );
 				}
 
 				break;
@@ -189,17 +189,20 @@ export default class Differ {
 	 * @param {module:engine/model/range~Range|null} oldRange Marker range before the change or `null` if the marker has just
 	 * been created.
 	 * @param {module:engine/model/range~Range|null} newRange Marker range after the change or `null` if the marker was removed.
+	 * @param {Boolean} affectsData Flag indicating whether marker affects the editor data.
 	 */
-	bufferMarkerChange( markerName, oldRange, newRange ) {
+	bufferMarkerChange( markerName, oldRange, newRange, affectsData ) {
 		const buffered = this._changedMarkers.get( markerName );
 
 		if ( !buffered ) {
 			this._changedMarkers.set( markerName, {
 				oldRange,
-				newRange
+				newRange,
+				affectsData
 			} );
 		} else {
 			buffered.newRange = newRange;
+			buffered.affectsData = affectsData;
 
 			if ( buffered.oldRange == null && buffered.newRange == null ) {
 				// The marker is going to be removed (`newRange == null`) but it did not exist before the first buffered change
@@ -243,6 +246,22 @@ export default class Differ {
 		return result;
 	}
 
+	/**
+	 * Checks whether some of buffered marker affects the editor data or whether some element changed.
+	 *
+	 * @returns {Boolean} `true` if buffered markers or changes in elements affects the editor data.
+	 */
+	hasDataChanges() {
+		for ( const [ , change ] of this._changedMarkers ) {
+			if ( change.affectsData ) {
+				return true;
+			}
+		}
+
+		// If markers do not affect the data, check whether there are some changes in elements.
+		return this._changesInElement.size > 0;
+	}
+
 	/**
 	 * Calculates the diff between the old model tree state (the state before the first buffered operations since the last {@link #reset}
 	 * call) and the new model tree state (actual one). It should be called after all buffered operations are executed.

src/model/document.js

@@ -147,11 +147,16 @@ export default class Document {
 		// Wait for `_change` event from model, which signalizes that outermost change block has finished.
 		// When this happens, check if there were any changes done on document, and if so, call post fixers,
 		// fire `change` event for features and conversion and then reset the differ.
+		// Fire `change:data` event when at least one operation or buffered marker changes the data.
 		this.listenTo( model, '_change', ( evt, writer ) => {
 			if ( !this.differ.isEmpty || hasSelectionChanged ) {
 				this._callPostFixers( writer );
 
-				this.fire( 'change', writer.batch );
+				if ( this.differ.hasDataChanges() ) {
+					this.fire( 'change:data', writer.batch );
+				} else {
+					this.fire( 'change', writer.batch );
+				}
 
 				this.differ.reset();
 				hasSelectionChanged = false;
@@ -163,12 +168,12 @@ export default class Document {
 		// are modified using `model.markers` collection, not through `MarkerOperation`).
 		this.listenTo( model.markers, 'update', ( evt, marker, oldRange, newRange ) => {
 			// Whenever marker is updated, buffer that change.
-			this.differ.bufferMarkerChange( marker.name, oldRange, newRange );
+			this.differ.bufferMarkerChange( marker.name, oldRange, newRange, marker.affectsData );
 
 			if ( oldRange === null ) {
 				// If this is a new marker, add a listener that will buffer change whenever marker changes.
 				marker.on( 'change', ( evt, oldRange ) => {
-					this.differ.bufferMarkerChange( marker.name, oldRange, marker.getRange() );
+					this.differ.bufferMarkerChange( marker.name, oldRange, marker.getRange(), marker.affectsData );
 				} );
 			}
 		} );
@@ -379,18 +384,35 @@ export default class Document {
 	 *			console.log( 'The Document has changed!' );
 	 *		} );
 	 *
-	 * If, however, you only want to be notified about structure changes, then check whether the
-	 * {@link module:engine/model/differ~Differ differ} contains any changes:
+	 * If, however, you only want to be notified about the data changes, then use the
+	 * {@link module:engine/model/document~Document#event:change:data change:data} event,
+	 * which fires for document structure changes and marker changes (which affects the data).
 	 *
-	 *		model.document.on( 'change', () => {
-	 *			if ( model.document.differ.getChanges().length > 0 ) {
-	 *				console.log( 'The Document has changed!' );
-	 *			}
+	 *		model.document.on( 'change:data', () => {
+	 *			console.log( 'The data has changed!' );
 	 *		} );
 	 *
 	 * @event change
 	 * @param {module:engine/model/batch~Batch} batch The batch that was used in the executed changes block.
 	 */
+
+	/**
+	 * Fired when the data changes, which includes:
+	 * * document structure changes,
+	 * * marker changes (which affects the data).
+	 *
+	 * If you want to be notified about the data changes, then listen to this event:
+	 *
+	 *		model.document.on( 'change:data', () => {
+	 *			console.log( 'The data has changed!' );
+	 *		} );
+	 *
+	 * If you would like to listen to all document's changes, then look at the
+	 * {@link module:engine/model/document~Document#event:change change} event.
+	 *
+	 * @event change:data
+	 * @param {module:engine/model/batch~Batch} batch The batch that was used in the executed changes block.
+	 */
 }
 
 mix( Document, EmitterMixin );

src/model/markercollection.js

@@ -88,9 +88,11 @@ export default class MarkerCollection {
 	 * @param {String|module:engine/model/markercollection~Marker} markerOrName Name of marker to set or marker instance to update.
 	 * @param {module:engine/model/range~Range} range Marker range.
 	 * @param {Boolean} [managedUsingOperations=false] Specifies whether the marker is managed using operations.
+	 * @param {Boolean} [affectsData=false] Specifies whether the marker affects the data produced by the data pipeline
+	 * (is persisted in the editor's data).
 	 * @returns {module:engine/model/markercollection~Marker} `Marker` instance which was added or updated.
 	 */
-	_set( markerOrName, range, managedUsingOperations = false ) {
+	_set( markerOrName, range, managedUsingOperations = false, affectsData = false ) {
 		const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName;
 		const oldMarker = this._markers.get( markerName );
 
@@ -108,6 +110,11 @@ export default class MarkerCollection {
 				hasChanged = true;
 			}
 
+			if ( typeof affectsData === 'boolean' && affectsData != oldMarker.affectsData ) {
+				oldMarker._affectsData = affectsData;
+				hasChanged = true;
+			}
+
 			if ( hasChanged ) {
 				this.fire( 'update:' + markerName, oldMarker, oldRange, range );
 			}
@@ -116,7 +123,7 @@ export default class MarkerCollection {
 		}
 
 		const liveRange = LiveRange.createFromRange( range );
-		const marker = new Marker( markerName, liveRange, managedUsingOperations );
+		const marker = new Marker( markerName, liveRange, managedUsingOperations, affectsData );
 
 		this._markers.set( markerName, marker );
 		this.fire( 'update:' + markerName, marker, null, range );
@@ -313,8 +320,10 @@ class Marker {
 	 * @param {String} name Marker name.
 	 * @param {module:engine/model/liverange~LiveRange} liveRange Range marked by the marker.
 	 * @param {Boolean} managedUsingOperations Specifies whether the marker is managed using operations.
+	 * @param {Boolean} affectsData Specifies whether the marker affects the data produced by the data pipeline
+	 * (is persisted in the editor's data).
 	 */
-	constructor( name, liveRange, managedUsingOperations ) {
+	constructor( name, liveRange, managedUsingOperations, affectsData ) {
 		/**
 		 * Marker's name.
 		 *
@@ -324,24 +333,33 @@ class Marker {
 		this.name = name;
 
 		/**
-		 * Flag indicates if the marker is managed using operations or not.
+		 * Range marked by the marker.
 		 *
 		 * @protected
+		 * @member {module:engine/model/liverange~LiveRange}
+		 */
+		this._liveRange = this._attachLiveRange( liveRange );
+
+		/**
+		 * Flag indicates if the marker is managed using operations or not.
+		 *
+		 * @private
 		 * @member {Boolean}
 		 */
 		this._managedUsingOperations = managedUsingOperations;
 
 		/**
-		 * Range marked by the marker.
+		 * Specifies whether the marker affects the data produced by the data pipeline
+		 * (is persisted in the editor's data).
 		 *
 		 * @private
-		 * @member {module:engine/model/liverange~LiveRange} #_liveRange
+		 * @member {Boolean}
 		 */
-		this._liveRange = this._attachLiveRange( liveRange );
+		this._affectsData = affectsData;
 	}
 
 	/**
-	 * Returns value of flag indicates if the marker is managed using operations or not.
+	 * A value indicating if the marker is managed using operations.
 	 * See {@link ~Marker marker class description} to learn more about marker types.
 	 * See {@link module:engine/model/writer~Writer#addMarker}.
 	 *
@@ -355,6 +373,19 @@ class Marker {
 		return this._managedUsingOperations;
 	}
 
+	/**
+	 * A value indicating if the marker changes the data.
+	 *
+	 * @returns {Boolean}
+	 */
+	get affectsData() {
+		if ( !this._liveRange ) {
+			throw new CKEditorError( 'marker-destroyed: Cannot use a destroyed marker instance.' );
+		}
+
+		return this._affectsData;
+	}
+
 	/**
 	 * Returns current marker start position.
 	 *
@@ -382,7 +413,7 @@ class Marker {
 	}
 
 	/**
-	 * Returns a range that represents current state of marker.
+	 * Returns a range that represents the current state of the marker.
 	 *
 	 * Keep in mind that returned value is a {@link module:engine/model/range~Range Range}, not a
 	 * {@link module:engine/model/liverange~LiveRange LiveRange}. This means that it is up-to-date and relevant only
@@ -402,7 +433,7 @@ class Marker {
 	}
 
 	/**
-	 * Binds new live range to marker and detach the old one if is attached.
+	 * Binds new live range to the marker and detach the old one if is attached.
 	 *
 	 * @protected
 	 * @param {module:engine/model/liverange~LiveRange} liveRange Live range to attach

src/model/operation/markeroperation.js

@@ -20,9 +20,11 @@ export default class MarkerOperation extends Operation {
 	 * @param {module:engine/model/range~Range} newRange Marker range after the change.
 	 * @param {module:engine/model/markercollection~MarkerCollection} markers Marker collection on which change should be executed.
 	 * @param {Number|null} baseVersion Document {@link module:engine/model/document~Document#version} on which operation
+	 * @param {Boolean} affectsData Specifies whether the marker operation affects the data produced by the data pipeline
+	 * (is persisted in the editor's data).
 	 * can be applied or `null` if the operation operates on detached (non-document) tree.
 	 */
-	constructor( name, oldRange, newRange, markers, baseVersion ) {
+	constructor( name, oldRange, newRange, markers, baseVersion, affectsData ) {
 		super( baseVersion );
 
 		/**
@@ -49,6 +51,15 @@ export default class MarkerOperation extends Operation {
 		 */
 		this.newRange = newRange ? Range.createFromRange( newRange ) : null;
 
+		/**
+		 * Specifies whether the marker operation affects the data produced by the data pipeline
+		 * (is persisted in the editor's data).
+		 *
+		 * @readonly
+		 * @member {Boolean}
+		 */
+		this.affectsData = affectsData;
+
 		/**
 		 * Marker collection on which change should be executed.
 		 *
@@ -71,7 +82,7 @@ export default class MarkerOperation extends Operation {
 	 * @returns {module:engine/model/operation/markeroperation~MarkerOperation} Clone of this operation.
 	 */
 	clone() {
-		return new MarkerOperation( this.name, this.oldRange, this.newRange, this._markers, this.baseVersion );
+		return new MarkerOperation( this.name, this.oldRange, this.newRange, this._markers, this.baseVersion, this.affectsData );
 	}
 
 	/**
@@ -80,7 +91,7 @@ export default class MarkerOperation extends Operation {
 	 * @returns {module:engine/model/operation/markeroperation~MarkerOperation}
 	 */
 	getReversed() {
-		return new MarkerOperation( this.name, this.newRange, this.oldRange, this._markers, this.baseVersion + 1 );
+		return new MarkerOperation( this.name, this.newRange, this.oldRange, this._markers, this.baseVersion + 1, this.affectsData );
 	}
 
 	/**
@@ -89,7 +100,7 @@ export default class MarkerOperation extends Operation {
 	_execute() {
 		const type = this.newRange ? '_set' : '_remove';
 
-		this._markers[ type ]( this.name, this.newRange, true );
+		this._markers[ type ]( this.name, this.newRange, true, this.affectsData );
 	}
 
 	/**
@@ -111,7 +122,7 @@ export default class MarkerOperation extends Operation {
 	}
 
 	/**
-	 * Creates `MarkerOperation` object from deserilized object, i.e. from parsed JSON string.
+	 * Creates `MarkerOperation` object from deserialized object, i.e. from parsed JSON string.
 	 *
 	 * @param {Object} json Deserialized JSON object.
 	 * @param {module:engine/model/document~Document} document Document on which this operation will be applied.
@@ -123,7 +134,8 @@ export default class MarkerOperation extends Operation {
 			json.oldRange ? Range.fromJSON( json.oldRange, document ) : null,
 			json.newRange ? Range.fromJSON( json.newRange, document ) : null,
 			document.model.markers,
-			json.baseVersion
+			json.baseVersion,
+			json.affectsData
 		);
 	}
 }

src/model/operation/operationfactory.js

@@ -37,7 +37,7 @@ operations[ RootAttributeOperation.className ] = RootAttributeOperation;
  */
 export default class OperationFactory {
 	/**
-	 * Creates concrete `Operation` object from deserilized object, i.e. from parsed JSON string.
+	 * Creates concrete `Operation` object from deserialized object, i.e. from parsed JSON string.
 	 *
 	 * @param {Object} json Deserialized JSON object.
 	 * @param {module:engine/model/document~Document} document Document on which this operation will be applied.