Merge pull request #1759 from ckeditor/t/1758

Other: Allowed for unbinding single elements from a marker name in `Mapper`. Closes #1758. BREAKING CHANGE: `Mapper#unbindElementsFromMarkerName( markerName )` was replaced by `Mapper#unbindElementFromMarkerName( element, markerName )`.
ckeditor · Jul 12, 2019 · 52e701d · 52e701d
2 parents 23a4c02 + b469be7
commit 52e701d
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 13 deletions.
diff --git a/src/conversion/downcasthelpers.js b/src/conversion/downcasthelpers.js
@@ -730,9 +730,8 @@ 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 );
 		}
 
@@ -1030,9 +1029,9 @@ function removeHighlight( highlightDescriptor ) {
 			return;
 		}
 
-		conversionApi.mapper.unbindElementsFromMarkerName( data.markerName );
-
 		for ( const element of elements ) {
+			conversionApi.mapper.unbindElementFromMarkerName( element, data.markerName );
+
 			if ( element.is( 'attributeElement' ) ) {
 				conversionApi.writer.unwrap( conversionApi.writer.createRangeOn( element ), viewHighlightElement );
 			} else {

diff --git a/src/conversion/mapper.js b/src/conversion/mapper.js
@@ -174,12 +174,21 @@ export default class Mapper {
 	}
 
 	/**
-	 * Unbinds all elements from given marker name.
+	 * Unbinds an element from given marker name.
 	 *
+	 * @param {module:engine/view/element~Element} element Element to unbind.
 	 * @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 );
+
+			if ( elements.size == 0 ) {
+				this._markerNameToElements.delete( name );
+			}
+		}
 	}
 
 	/**

diff --git a/tests/conversion/mapper.js b/tests/conversion/mapper.js
@@ -643,20 +643,25 @@ describe( 'Mapper', () => {
 			expect( elements ).to.deep.equal( [ viewA, viewB, viewC ] );
 		} );
 
-		it( 'should unbind all elements from a marker name', () => {
+		it( 'should unbind element 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' );
+			mapper.unbindElementFromMarkerName( viewA, 'marker' );
 
-			const elements = mapper.markerNameToElements( 'marker' );
+			expect( Array.from( mapper.markerNameToElements( 'marker' ) ) ).to.deep.equal( [ viewB ] );
+
+			mapper.unbindElementFromMarkerName( viewB, 'marker' );
+
+			expect( mapper.markerNameToElements( 'marker' ) ).to.be.null;
+
+			// Removing an element from non-existing group or non-bound element should not cause a crash.
+			mapper.unbindElementFromMarkerName( viewB, 'marker' );
 
-			expect( elements ).to.be.null;
+			expect( mapper.markerNameToElements( 'marker' ) ).to.be.null;
 		} );
 	} );