Procházet zdrojové kódy

Changed: default remove converter should not try to map model positions that do not exist in model.

Szymon Cofalik před 8 roky
rodič
revize
0ae837fc52

+ 37 - 4
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -3,10 +3,10 @@
  * For licensing, see LICENSE.md.
  */
 
-import ModelRange from '../model/range';
-
 import ViewElement from '../view/element';
 import ViewText from '../view/text';
+import ViewRange from '../view/range';
+import ViewTreeWalker from '../view/treewalker';
 import viewWriter from '../view/writer';
 
 /**
@@ -436,8 +436,12 @@ export function remove() {
 			return;
 		}
 
-		const modelRange = ModelRange.createFromPositionAndShift( data.sourcePosition, data.item.offsetSize );
-		const viewRange = conversionApi.mapper.toViewRange( modelRange );
+		// Note: at this moment we should avoid mapping non existing model positions.
+		// TODO: try to simplify this if `Mapper` ever changes.
+		const viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
+		const viewRange = data.item.is( 'element' ) ?
+			ViewRange.createOn( viewPosition.nodeAfter ) :
+			_expandViewPositionOnText( viewPosition, data.item.offsetSize );
 
 		viewWriter.remove( viewRange.getTrimmed() );
 
@@ -460,6 +464,35 @@ export function remove() {
 	};
 }
 
+// Helper function that seeks the end of a view range that starts at `position` and contains `howMany` many letters.
+// Because in view there might be empty view ui elements that split text nodes, we cannot just use `ViewRange#createFromPositionAndShift()`
+function _expandViewPositionOnText( position, howMany ) {
+	const walker = new ViewTreeWalker( { startPosition: position } );
+	let currentOffset = 0;
+
+	for ( let value of walker ) {
+		if ( value.type == 'text' ) {
+			let length = value.item.data.length;
+
+			if ( currentOffset + length >= howMany ) {
+				let item = value.item;
+
+				if ( item.is( 'textProxy' ) ) {
+					item = item.textNode;
+					currentOffset = currentOffset - value.item.offsetInText;
+				}
+
+				return ViewRange.createFromParentsAndOffsets(
+					position.parent, position.offset,
+					item, howMany - currentOffset
+				);
+			}
+
+			currentOffset += length;
+		}
+	}
+}
+
 /**
  * Function factory, creates a default model-to-view converter for removing {@link module:engine/view/uielement~UIElement ui element}
  * basing on marker remove change.

+ 25 - 1
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -904,7 +904,7 @@ describe( 'model-to-view-converters', () => {
 	} );
 
 	describe( 'remove', () => {
-		it( 'should remove items from view accordingly to changes in model', () => {
+		it( 'should remove items from view accordingly to changes in model #1', () => {
 			const modelDiv = new ModelElement( 'div', null, [
 				new ModelText( 'foo' ),
 				new ModelElement( 'image' ),
@@ -1003,6 +1003,30 @@ describe( 'model-to-view-converters', () => {
 			expect( viewToString( viewRoot ) ).to.equal( '<div>foo<span></span>ar</div>' );
 		} );
 
+		it( 'should remove correct amount of text when it is split by view ui element', () => {
+			modelRoot.appendChildren( new ModelText( 'foobar' ) );
+			viewRoot.appendChildren( [
+				new ViewText( 'foo' ),
+				new ViewUIElement( 'span' ),
+				new ViewText( 'bar' )
+			] );
+
+			dispatcher.on( 'remove', remove() );
+
+			// Remove 'o<span></span>b'.
+			modelWriter.move(
+				ModelRange.createFromParentsAndOffsets( modelRoot, 2, modelRoot, 4 ),
+				ModelPosition.createAt( modelDoc.graveyard, 'end' )
+			);
+
+			dispatcher.convertRemove(
+				ModelPosition.createFromParentAndOffset( modelRoot, 2 ),
+				ModelRange.createFromParentsAndOffsets( modelDoc.graveyard, 0, modelDoc.graveyard, 2 )
+			);
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div>foar</div>' );
+		} );
+
 		it( 'should not unbind element that has not been moved to graveyard', () => {
 			const modelElement = new ModelElement( 'a' );
 			const viewElement = new ViewElement( 'a' );