WIP

ckeditor · Jul 8, 2019 · e4baf37 · e4baf37
1 parent b84cf9b
commit e4baf37
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 50 deletions.
diff --git a/src/conversion/downcasthelpers.js b/src/conversion/downcasthelpers.js
@@ -730,14 +730,11 @@ function removeUIElement() {
 			return;
 		}
 
-		conversionApi.mapper.unbindElementsFromMarkerName( data.markerName );
-
 		for ( const element of elements ) {
+			conversionApi.mapper.unbindElementFromMarkerName( element, data.markerName );
 			conversionApi.writer.clear( conversionApi.writer.createRangeOn( element ), element );
 		}
 
-		conversionApi.writer.clearClonedElementsGroup( data.markerName );
-
 		evt.stop();
 	};
 }
@@ -1030,9 +1027,10 @@ function removeHighlight( highlightDescriptor ) {
 			return;
 		}
 
-		conversionApi.mapper.unbindElementsFromMarkerName( data.markerName );
-
 		for ( const element of elements ) {
+			conversionApi.mapper.unbindElementFromMarkerName( element, data.markerName );
+			// conversionApi.writer._removeFromClonedElementsGroup( element );
+
 			if ( element.is( 'attributeElement' ) ) {
 				conversionApi.writer.unwrap( conversionApi.writer.createRangeOn( element ), viewHighlightElement );
 			} else {
@@ -1041,8 +1039,6 @@ function removeHighlight( highlightDescriptor ) {
 			}
 		}
 
-		conversionApi.writer.clearClonedElementsGroup( data.markerName );
-
 		evt.stop();
 	};
 }

diff --git a/src/conversion/mapper.js b/src/conversion/mapper.js
@@ -173,13 +173,12 @@ export default class Mapper {
 		this._markerNameToElements.set( name, elements );
 	}
 
-	/**
-	 * Unbinds all elements from given marker name.
-	 *
-	 * @param {String} name Marker name.
-	 */
-	unbindElementsFromMarkerName( name ) {
-		this._markerNameToElements.delete( name );
+	unbindElementFromMarkerName( element, name ) {
+		const elements = this._markerNameToElements.get( name );
+
+		if ( elements ) {
+			elements.delete( element );
+		}
 	}
 
 	/**

diff --git a/src/view/downcastwriter.js b/src/view/downcastwriter.js
@@ -907,24 +907,6 @@ export default class DowncastWriter {
 		return newElement;
 	}
 
-	/**
-	 * Cleans up memory by removing obsolete cloned elements group from the writer.
-	 *
-	 * Should be used whenever all {@link module:engine/view/attributeelement~AttributeElement attribute elements}
-	 * with the same {@link module:engine/view/attributeelement~AttributeElement#id id} are going to be removed from the view and
-	 * the group will no longer be needed.
-	 *
-	 * Cloned elements group are not removed automatically in case if the group is still needed after all its elements
-	 * were removed from the view.
-	 *
-	 * Keep in mind that group names are equal to the `id` property of the attribute element.
-	 *
-	 * @param {String} groupName Name of the group to clear.
-	 */
-	clearClonedElementsGroup( groupName ) {
-		this._cloneGroups.delete( groupName );
-	}
-
 	/**
 	 * Creates position at the given location. The location can be specified as:
 	 *
@@ -1662,7 +1644,6 @@ export default class DowncastWriter {
 	 *
 	 * Does nothing if the element has no {@link module:engine/view/attributeelement~AttributeElement#id id}.
 	 *
-	 * @private
 	 * @param {module:engine/view/attributeelement~AttributeElement} element Attribute element to remove.
 	 */
 	_removeFromClonedElementsGroup( element ) {

diff --git a/tests/conversion/downcasthelpers.js b/tests/conversion/downcasthelpers.js
@@ -1177,7 +1177,7 @@ describe( 'DowncastHelpers', () => {
 					expect( viewToString( viewRoot ) ).to.equal( '<div><p>foo</p><p>bar</p></div>' );
 				} );
 
-				it( 'should correctly wrap and unwrap multiple, intersecting markers', () => {
+				it.only( 'should correctly wrap and unwrap multiple, intersecting markers', () => {
 					downcastHelpers.markerToHighlight( {
 						model: 'markerFoo',
 						view: { classes: 'foo' }

diff --git a/tests/conversion/mapper.js b/tests/conversion/mapper.js
@@ -643,21 +643,21 @@ describe( 'Mapper', () => {
 			expect( elements ).to.deep.equal( [ viewA, viewB, viewC ] );
 		} );
 
-		it( 'should unbind all elements from a marker name', () => {
-			const viewA = new ViewElement( 'a' );
-			const viewB = new ViewElement( 'b' );
-			const viewC = new ViewElement( 'c' );
-
-			mapper.bindElementToMarker( viewA, 'marker' );
-			mapper.bindElementToMarker( viewB, 'marker' );
-			mapper.bindElementToMarker( viewC, 'marker' );
-
-			mapper.unbindElementsFromMarkerName( 'marker' );
-
-			const elements = mapper.markerNameToElements( 'marker' );
-
-			expect( elements ).to.be.null;
-		} );
+		// it( 'should unbind all elements from a marker name', () => {
+		// 	const viewA = new ViewElement( 'a' );
+		// 	const viewB = new ViewElement( 'b' );
+		// 	const viewC = new ViewElement( 'c' );
+		//
+		// 	mapper.bindElementToMarker( viewA, 'marker' );
+		// 	mapper.bindElementToMarker( viewB, 'marker' );
+		// 	mapper.bindElementToMarker( viewC, 'marker' );
+		//
+		// 	mapper.unbindElementsFromMarkerName( 'marker' );
+		//
+		// 	const elements = mapper.markerNameToElements( 'marker' );
+		//
+		// 	expect( elements ).to.be.null;
+		// } );
 	} );
 
 	it( 'should pass isPhantom flag to model-to-view position mapping callback', () => {