8
0
Quellcode durchsuchen

Fixed: Fixed default remove model-to-view converter for edge cases.

Szymon Cofalik vor 8 Jahren
Ursprung
Commit
3438089fb5

+ 11 - 1
packages/ckeditor5-engine/src/conversion/model-to-view-converters.js

@@ -6,6 +6,7 @@
 import ViewElement from '../view/element';
 import ViewText from '../view/text';
 import ViewRange from '../view/range';
+import ViewPosition from '../view/position';
 import ViewTreeWalker from '../view/treewalker';
 import viewWriter from '../view/writer';
 
@@ -441,13 +442,22 @@ export function remove() {
 		// end of that range is incorrect.
 		// Instead we will use `data.sourcePosition` as this is the last correct model position and
 		// it is a position before the removed item. Then, we will calculate view range to remove "manually".
-		const viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
+		let viewPosition = conversionApi.mapper.toViewPosition( data.sourcePosition );
 		let viewRange;
 
 		if ( data.item.is( 'element' ) ) {
 			// Note: in remove conversion we cannot use model-to-view element mapping because `data.item` may be
 			// already mapped to another element (this happens when move change is converted).
 			// In this case however, `viewPosition` is the position before view element that corresponds to removed model element.
+			//
+			// First, fix the position. Traverse the tree forward until the container element is found. The `viewPosition`
+			// may be before a ui element, before attribute element or at the end of text element.
+			viewPosition = viewPosition.getLastMatchingPosition( value => !value.item.is( 'containerElement' ) );
+
+			if ( viewPosition.parent.is( 'text' ) && viewPosition.isAtEnd ) {
+				viewPosition = ViewPosition.createAfter( viewPosition.parent );
+			}
+
 			viewRange = ViewRange.createOn( viewPosition.nodeAfter );
 		} else {
 			// If removed item is a text node, we need to traverse view tree to find the view range to remove.

+ 61 - 4
packages/ckeditor5-engine/tests/conversion/model-to-view-converters.js

@@ -1028,8 +1028,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 
 		it( 'should not unbind element that has not been moved to graveyard', () => {
-			const modelElement = new ModelElement( 'a' );
-			const viewElement = new ViewElement( 'a' );
+			const modelElement = new ModelElement( 'paragraph' );
+			const viewElement = new ViewContainerElement( 'p' );
 
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
@@ -1056,8 +1056,8 @@ describe( 'model-to-view-converters', () => {
 		} );
 
 		it( 'should unbind elements if model element was moved to graveyard', () => {
-			const modelElement = new ModelElement( 'a' );
-			const viewElement = new ViewElement( 'a' );
+			const modelElement = new ModelElement( 'paragraph' );
+			const viewElement = new ViewContainerElement( 'p' );
 
 			modelRoot.appendChildren( [ modelElement, new ModelText( 'b' ) ] );
 			viewRoot.appendChildren( [ viewElement, new ViewText( 'b' ) ] );
@@ -1125,5 +1125,62 @@ describe( 'model-to-view-converters', () => {
 			expect( mapper.toModelElement( viewWElement ) ).to.be.undefined;
 			expect( mapper.toViewElement( modelWElement ) ).to.be.undefined;
 		} );
+
+		it( 'should work correctly if container element after ui element is removed', () => {
+			const modelP1 = new ModelElement( 'paragraph' );
+			const modelP2 = new ModelElement( 'paragraph' );
+
+			const viewP1 = new ViewContainerElement( 'p' );
+			const viewUi1 = new ViewUIElement( 'span' );
+			const viewUi2 = new ViewUIElement( 'span' );
+			const viewP2 = new ViewContainerElement( 'p' );
+
+			modelRoot.appendChildren( [ modelP1, modelP2 ] );
+			viewRoot.appendChildren( [ viewP1, viewUi1, viewUi2, viewP2 ] );
+
+			mapper.bindElements( modelP1, viewP1 );
+			mapper.bindElements( modelP2, viewP2 );
+
+			dispatcher.on( 'remove', remove() );
+
+			modelWriter.move(
+				ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 2 ),
+				ModelPosition.createAt( modelDoc.graveyard, 'end' )
+			);
+
+			dispatcher.convertRemove(
+				ModelPosition.createFromParentAndOffset( modelRoot, 1 ),
+				ModelRange.createFromParentsAndOffsets( modelDoc.graveyard, 0, modelDoc.graveyard, 1 )
+			);
+
+			expect( viewToString( viewRoot ) ).to.equal( '<div><p></p><span></span><span></span></div>' );
+		} );
+
+		it( 'should work correctly if container element after text node is removed', () => {
+			const modelText = new ModelText( 'foo' );
+			const modelP = new ModelElement( 'paragraph' );
+
+			const viewText = new ViewText( 'foo' );
+			const viewP = new ViewContainerElement( 'p' );
+
+			modelRoot.appendChildren( [ modelText, modelP ] );
+			viewRoot.appendChildren( [ viewText, viewP ] );
+
+			mapper.bindElements( modelP, viewP );
+
+			dispatcher.on( 'remove', remove() );
+
+			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</div>' );
+		} );
 	} );
 } );