8
0
Quellcode durchsuchen

Fix: Markers were not refreshed when a model element was renamed.

Szymon Cofalik vor 7 Jahren
Ursprung
Commit
3c6dc4fdaa

+ 22 - 1
packages/ckeditor5-engine/src/model/differ.js

@@ -19,7 +19,20 @@ import Range from './range';
  * elements and new ones and returns a change set.
  */
 export default class Differ {
-	constructor() {
+	/**
+	 * Creates a `Differ` instance.
+	 *
+	 * @param {module:engine/model/markercollection~MarkerCollection} markerCollection Model's marker collection.
+	 */
+	constructor( markerCollection ) {
+		/**
+		 * Reference to the model's marker collection.
+		 *
+		 * @private
+		 * @type {module:engine/model/markercollection~MarkerCollection}
+		 */
+		this._markerCollection = markerCollection;
+
 		/**
 		 * A map that stores changes that happened in a given element.
 		 *
@@ -153,6 +166,14 @@ export default class Differ {
 				this._markRemove( operation.position.parent, operation.position.offset, 1 );
 				this._markInsert( operation.position.parent, operation.position.offset, 1 );
 
+				const range = Range.createFromPositionAndShift( operation.position, 1 );
+
+				for ( const marker of this._markerCollection.getMarkersIntersectingRange( range ) ) {
+					const markerRange = marker.getRange();
+
+					this.bufferMarkerChange( marker.name, markerRange, markerRange );
+				}
+
 				break;
 			}
 		}

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

@@ -87,7 +87,7 @@ export default class Document {
 		 * @readonly
 		 * @member {module:engine/model/differ~Differ}
 		 */
-		this.differ = new Differ();
+		this.differ = new Differ( model.markers );
 
 		/**
 		 * Post-fixer callbacks registered to the model document.

+ 14 - 0
packages/ckeditor5-engine/src/model/markercollection.js

@@ -163,6 +163,20 @@ export default class MarkerCollection {
 	}
 
 	/**
+	 * Returns iterator that iterates over all markers, which intersects with given {@link module:engine/model/range~Range range}.
+	 *
+	 * @param {module:engine/model/range~Range} range
+	 * @returns {Iterable.<module:engine/model/markercollection~Marker>}
+	 */
+	* getMarkersIntersectingRange( range ) {
+		for ( const marker of this ) {
+			if ( marker.getRange().getIntersection( range ) !== null ) {
+				yield marker;
+			}
+		}
+	}
+
+	/**
 	 * Destroys marker collection and all markers inside it.
 	 */
 	destroy() {

+ 36 - 0
packages/ckeditor5-engine/tests/model/differ.js

@@ -653,6 +653,42 @@ describe( 'Differ', () => {
 				] );
 			} );
 		} );
+
+		it( 'markers refreshing', () => {
+			// Since rename is "translated" to remove+insert pair, we need to refresh all the markers that
+			// intersect with the renamed element.
+
+			model.change( () => {
+				// Renamed element contains marker.
+				model.markers._set( 'markerA', new Range( new Position( root, [ 1, 1 ] ), new Position( root, [ 1, 2 ] ) ) );
+
+				// Marker contains renamed element.
+				model.markers._set( 'markerB', new Range( new Position( root, [ 0 ] ), new Position( root, [ 2 ] ) ) );
+
+				// Intersecting.
+				model.markers._set( 'markerC', new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 1, 2 ] ) ) );
+
+				// Not intersecting.
+				model.markers._set( 'markerD', new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 1 ] ) ) );
+			} );
+
+			const markersToRefresh = [ 'markerA', 'markerB', 'markerC' ];
+
+			model.change( () => {
+				rename( root.getChild( 1 ), 'listItem' );
+
+				expectChanges( [
+					{ type: 'remove', name: 'paragraph', length: 1, position: new Position( root, [ 1 ] ) },
+					{ type: 'insert', name: 'listItem', length: 1, position: new Position( root, [ 1 ] ) }
+				] );
+
+				const markersToRemove = differ.getMarkersToRemove().map( entry => entry.name );
+				const markersToAdd = differ.getMarkersToAdd().map( entry => entry.name );
+
+				expect( markersToRefresh ).to.deep.equal( markersToRemove );
+				expect( markersToRefresh ).to.deep.equal( markersToAdd );
+			} );
+		} );
 	} );
 
 	describe( 'attribute', () => {

+ 7 - 0
packages/ckeditor5-engine/tests/model/range.js

@@ -788,6 +788,13 @@ describe( 'Range', () => {
 			expect( common.start.path ).to.deep.equal( [ 3, 2 ] );
 			expect( common.end.path ).to.deep.equal( [ 4, 7 ] );
 		} );
+
+		it( 'should return a range equal to both ranges if both ranges are equal', () => {
+			const otherRange = Range.createFromRange( range );
+			const common = range.getIntersection( otherRange );
+
+			expect( common.isEqual( range ) ).to.be.true;
+		} );
 	} );
 
 	// Note: We don't create model element structure in these tests because this method