Browse Source

Merge pull request #855 from ckeditor/t/854

Fix: Default `remove()` converter no longer removes `view.UIElements` that are next to removed nodes. Closes #854.

NOTE: It is advised to use either `Range#getTrimmed` or `Range#getEnlarged` before operating on a range returned from Mapper.
Piotr Jasiun 8 years ago
parent
commit
1f77ba3a0b

+ 2 - 3
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -424,7 +424,7 @@ export function remove() {
 		const modelRange = ModelRange.createFromPositionAndShift( data.sourcePosition, data.item.offsetSize );
 		const viewRange = conversionApi.mapper.toViewRange( modelRange );
 
-		viewWriter.remove( viewRange );
+		viewWriter.remove( viewRange.getTrimmed() );
 		conversionApi.mapper.unbindModelElement( data.item );
 	};
 }
@@ -452,9 +452,8 @@ export function removeUIElement( elementCreator ) {
 		}
 
 		const viewRange = conversionApi.mapper.toViewRange( data.range );
-		const enlargedViewRange = viewRange.getEnlarged();
 
-		viewWriter.clear( enlargedViewRange, viewElement );
+		viewWriter.clear( viewRange.getEnlarged(), viewElement );
 	};
 }
 

+ 24 - 0
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -848,5 +848,29 @@ describe( 'model-to-view-converters', () => {
 
 			expect( viewToString( viewRoot ) ).to.equal( '<div><div>கு</div></div>' );
 		} );
+
+		it( 'should not remove view ui elements that are placed next to removed content', () => {
+			modelRoot.appendChildren( new ModelText( 'foobar' ) );
+			viewRoot.appendChildren( [
+				new ViewText( 'foo' ),
+				new ViewUIElement( 'span' ),
+				new ViewText( 'bar' )
+			] );
+
+			dispatcher.on( 'remove', remove() );
+
+			// Remove 'b'.
+			modelWriter.move(
+				ModelRange.createFromParentsAndOffsets( modelRoot, 3, modelRoot, 4 ),
+				ModelPosition.createAt( modelDoc.graveyard, 'end' )
+			);
+
+			dispatcher.convertRemove(
+				ModelPosition.createFromParentAndOffset( modelRoot, 3 ),
+				ModelRange.createFromParentsAndOffsets( modelDoc.graveyard, 0, modelDoc.graveyard, 1 )
+			);
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div>foo<span></span>ar</div>' );
+		} );
 	} );
 } );