8
0
Maciej Bukowski 7 лет назад
Родитель
Сommit
2d790252c0

+ 18 - 2
packages/ckeditor5-engine/src/model/differ.js

@@ -190,13 +190,14 @@ export default class Differ {
 	 * been created.
 	 * @param {module:engine/model/range~Range|null} newRange Marker range after the change or `null` if the marker was removed.
 	 */
-	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;
@@ -243,6 +244,21 @@ export default class Differ {
 		return result;
 	}
 
+	/**
+	 * Checks whether some of buffered marker can affect the data model.
+	 *
+	 * @returns {Boolean} `true` if buffered markers can change the data model.
+	 */
+	containsMarkersAffectingData() {
+		for ( const [ , change ] of this._changedMarkers ) {
+			if ( change.affectsData ) {
+				return true;
+			}
+		}
+
+		return false;
+	}
+
 	/**
 	 * 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.

+ 4 - 4
packages/ckeditor5-engine/src/model/document.js

@@ -152,7 +152,7 @@ export default class Document {
 			if ( !this.differ.isEmpty || hasSelectionChanged ) {
 				this._callPostFixers( writer );
 
-				if ( !this.differ.isEmpty && isBatchAffectingData( writer.batch ) ) {
+				if ( isBatchAffectingData( writer.batch ) || this.differ.containsMarkersAffectingData() ) {
 					this.fire( 'change:data', writer.batch );
 				} else {
 					this.fire( 'change', writer.batch );
@@ -166,14 +166,14 @@ export default class Document {
 		// Buffer marker changes.
 		// This is not covered in buffering operations because markers may change outside of them (when they
 		// are modified using `model.markers` collection, not through `MarkerOperation`).
-		this.listenTo( model.markers, 'update', ( evt, marker, oldRange, newRange ) => {
+		this.listenTo( model.markers, 'update', ( evt, marker, oldRange, newRange, affectsData ) => {
 			// Whenever marker is updated, buffer that change.
-			this.differ.bufferMarkerChange( marker.name, oldRange, newRange );
+			this.differ.bufferMarkerChange( marker.name, oldRange, newRange, 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(), affectsData );
 				} );
 			}
 		} );

+ 2 - 2
packages/ckeditor5-engine/src/model/markercollection.js

@@ -115,7 +115,7 @@ export default class MarkerCollection {
 			}
 
 			if ( hasChanged ) {
-				this.fire( 'update:' + markerName, oldMarker, oldRange, range );
+				this.fire( 'update:' + markerName, oldMarker, oldRange, range, affectsData );
 			}
 
 			return oldMarker;
@@ -125,7 +125,7 @@ export default class MarkerCollection {
 		const marker = new Marker( markerName, liveRange, managedUsingOperations, affectsData );
 
 		this._markers.set( markerName, marker );
-		this.fire( 'update:' + markerName, marker, null, range );
+		this.fire( 'update:' + markerName, marker, null, range, affectsData );
 
 		return marker;
 	}

+ 13 - 1
packages/ckeditor5-engine/tests/manual/markers.js

@@ -47,6 +47,18 @@ ClassicEditor
 			}
 		} ) );
 
+		editor.conversion.for( 'dataDowncast' ).add( downcastMarkerToHighlight( {
+			model: 'highlight',
+			view: data => {
+				const color = data.markerName.split( ':' )[ 1 ];
+
+				return {
+					classes: 'h-' + color,
+					priority: 1
+				};
+			}
+		} ) );
+
 		window.document.getElementById( 'add-yellow' ).addEventListener( 'mousedown', e => {
 			e.preventDefault();
 			addHighlight( 'yellow' );
@@ -83,7 +95,7 @@ ClassicEditor
 			const name = 'highlight:yellow:' + uid();
 
 			markerNames.push( name );
-			writer.addMarker( name, { range, usingOperation: false } );
+			writer.addMarker( name, { range, usingOperation: false, affectsData: true } );
 		} );
 	} )
 	.catch( err => {

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

@@ -437,6 +437,38 @@ describe( 'Document', () => {
 			sinon.assert.notCalled( spy );
 		} );
 
+		it( 'should be fired if the writer adds marker not managed by using operations', () => {
+			const root = doc.createRoot();
+			const spy = sinon.spy();
+
+			root._appendChild( new Text( 'foo' ) );
+
+			doc.on( 'change:data', spy );
+
+			model.change( writer => {
+				const range = Range.createFromParentsAndOffsets( root, 2, root, 4 );
+				writer.addMarker( 'name', { range, usingOperation: false } );
+			} );
+
+			sinon.assert.calledOnce( spy );
+		} );
+
+		it( 'should not be fired if the writer adds marker not managed by using operations with affectsData set to false', () => {
+			const root = doc.createRoot();
+			const spy = sinon.spy();
+
+			root._appendChild( new Text( 'foo' ) );
+
+			doc.on( 'change:data', spy );
+
+			model.change( writer => {
+				const range = Range.createFromParentsAndOffsets( root, 2, root, 4 );
+				writer.addMarker( 'name', { range, usingOperation: false, affectsData: false } );
+			} );
+
+			sinon.assert.notCalled( spy );
+		} );
+
 		it( 'should not be fired if writer was used on non-document tree', () => {
 			const spy = sinon.spy();
 

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

@@ -53,7 +53,7 @@ describe( 'MarkerCollection', () => {
 			expect( marker.managedUsingOperations ).to.false;
 			expect( marker.affectsData ).to.true;
 			expect( marker.getRange().isEqual( range ) ).to.be.true;
-			sinon.assert.calledWithExactly( markers.fire, 'update:name', result, null, range );
+			sinon.assert.calledWithExactly( markers.fire, 'update:name', result, null, range, true );
 		} );
 
 		it( 'should create a marker marked as managed by operations', () => {
@@ -80,7 +80,7 @@ describe( 'MarkerCollection', () => {
 			expect( result ).to.equal( marker );
 			expect( marker.getRange().isEqual( range2 ) ).to.be.true;
 
-			sinon.assert.calledWithExactly( markers.fire, 'update:name', marker, range, range2 );
+			sinon.assert.calledWithExactly( markers.fire, 'update:name', marker, range, range2, true );
 			sinon.assert.calledOnce( marker._detachLiveRange );
 			sinon.assert.calledOnce( marker._detachLiveRange );
 		} );
@@ -100,7 +100,7 @@ describe( 'MarkerCollection', () => {
 			expect( marker.managedUsingOperations ).to.true;
 			expect( marker.getRange().isEqual( range ) ).to.be.true;
 
-			sinon.assert.calledWithExactly( markers.fire, 'update:name', marker, range, range );
+			sinon.assert.calledWithExactly( markers.fire, 'update:name', marker, range, range, true );
 			sinon.assert.notCalled( marker._detachLiveRange );
 			sinon.assert.notCalled( marker._attachLiveRange );
 		} );