浏览代码

Changed: Nodes are removed from their old parent when they are appended to new parent.

Szymon Cofalik 8 年之前
父节点
当前提交
d83732f969

+ 2 - 16
packages/ckeditor5-engine/src/dev-utils/model.js

@@ -126,22 +126,8 @@ export function setData( document, data, options = {} ) {
 			const ranges = [];
 
 			for ( const range of selection.getRanges() ) {
-				let start, end;
-
-				// Each range returned from `parse()` method has its root placed in DocumentFragment.
-				// Here we convert each range to have its root re-calculated properly and be placed inside
-				// model document root.
-				if ( range.start.parent.is( 'documentFragment' ) ) {
-					start = ModelPosition.createFromParentAndOffset( modelRoot, range.start.offset );
-				} else {
-					start = ModelPosition.createFromParentAndOffset( range.start.parent, range.start.offset );
-				}
-
-				if ( range.end.parent.is( 'documentFragment' ) ) {
-					end = ModelPosition.createFromParentAndOffset( modelRoot, range.end.offset );
-				} else {
-					end = ModelPosition.createFromParentAndOffset( range.end.parent, range.end.offset );
-				}
+				const start = new ModelPosition( modelRoot, range.start.path );
+				const end = new ModelPosition( modelRoot, range.end.path );
 
 				ranges.push( new ModelRange( start, end ) );
 			}

+ 1 - 1
packages/ckeditor5-engine/src/dev-utils/view.js

@@ -852,7 +852,7 @@ function _convertViewElements( rootNode ) {
 		const convertedElement = rootNode.is( 'documentFragment' ) ? new ViewDocumentFragment() : _convertElement( rootNode );
 
 		// Convert all child nodes.
-		for ( const child of rootNode.getChildren() ) {
+		for ( const child of [ ...rootNode.getChildren() ] ) {
 			if ( convertedElement.is( 'emptyElement' ) ) {
 				throw new Error( 'Parse error - cannot parse inside EmptyElement.' );
 			}

+ 5 - 0
packages/ckeditor5-engine/src/model/documentfragment.js

@@ -235,6 +235,11 @@ export default class DocumentFragment {
 		nodes = normalize( nodes );
 
 		for ( const node of nodes ) {
+			// If node that is being added to this element is already inside another element, first remove it from the old parent.
+			if ( node.parent !== null ) {
+				node.remove();
+			}
+
 			node.parent = this;
 		}
 

+ 5 - 0
packages/ckeditor5-engine/src/model/element.js

@@ -202,6 +202,11 @@ export default class Element extends Node {
 		nodes = normalize( nodes );
 
 		for ( const node of nodes ) {
+			// If node that is being added to this element is already inside another element, first remove it from the old parent.
+			if ( node.parent !== null ) {
+				node.remove();
+			}
+
 			node.parent = this;
 		}
 

+ 5 - 0
packages/ckeditor5-engine/src/view/documentfragment.js

@@ -150,6 +150,11 @@ export default class DocumentFragment {
 		nodes = normalize( nodes );
 
 		for ( const node of nodes ) {
+			// If node that is being added to this element is already inside another element, first remove it from the old parent.
+			if ( node.parent !== null ) {
+				node.remove();
+			}
+
 			node.parent = this;
 
 			this._children.splice( index, 0, node );

+ 5 - 0
packages/ckeditor5-engine/src/view/element.js

@@ -355,6 +355,11 @@ export default class Element extends Node {
 		nodes = normalize( nodes );
 
 		for ( const node of nodes ) {
+			// If node that is being added to this element is already inside another element, first remove it from the old parent.
+			if ( node.parent !== null ) {
+				node.remove();
+			}
+
 			node.parent = this;
 
 			this._children.splice( index, 0, node );

+ 0 - 6
packages/ckeditor5-engine/tests/conversion/advanced-converters.js

@@ -287,8 +287,6 @@ describe( 'advanced-converters', () => {
 		it( 'should convert view image to model', () => {
 			const viewElement = new ViewContainerElement( 'img', { src: 'bar.jpg', title: 'bar' } );
 			const modelElement = viewDispatcher.convert( viewElement );
-			// Attaching to tree so tree walker works fine in `modelToString`.
-			modelRoot.appendChildren( modelElement );
 
 			expect( modelToString( modelElement ) ).to.equal( '<image src="bar.jpg" title="bar"></image>' );
 		} );
@@ -303,8 +301,6 @@ describe( 'advanced-converters', () => {
 				]
 			);
 			const modelElement = viewDispatcher.convert( viewElement );
-			// Attaching to tree so tree walker works fine in `modelToString`.
-			modelRoot.appendChildren( modelElement );
 
 			expect( modelToString( modelElement ) ).to.equal( '<image src="bar.jpg" title="bar"><caption>foobar</caption></image>' );
 		} );
@@ -595,7 +591,6 @@ describe( 'advanced-converters', () => {
 			);
 
 			const modelElement = viewDispatcher.convert( viewElement );
-			modelRoot.appendChildren( modelElement );
 
 			expect( modelToString( modelElement ) ).to.equal( '<quote linkHref="foo.html" linkTitle="Foo source">foo</quote>' );
 		} );
@@ -761,7 +756,6 @@ describe( 'advanced-converters', () => {
 			] );
 
 			const modelElement = viewDispatcher.convert( viewElement );
-			modelRoot.appendChildren( modelElement );
 
 			expect( modelToString( modelElement ) ).to.equal(
 				'<table cellpadding="5" cellspacing="5">' +

+ 6 - 24
packages/ckeditor5-engine/tests/conversion/buildviewconverter.js

@@ -7,7 +7,6 @@ import buildViewConverter from '../../src/conversion/buildviewconverter';
 
 import ModelSchema from '../../src/model/schema';
 import ModelDocumentFragment from '../../src/model/documentfragment';
-import ModelDocument from '../../src/model/document';
 import ModelElement from '../../src/model/element';
 import ModelTextProxy from '../../src/model/textproxy';
 import ModelRange from '../../src/model/range';
@@ -64,7 +63,7 @@ const textAttributes = [ undefined, 'linkHref', 'linkTitle', 'bold', 'italic', '
 const pAttributes = [ undefined, 'class', 'important', 'theme', 'decorated', 'size' ];
 
 describe( 'View converter builder', () => {
-	let dispatcher, modelDoc, modelRoot, schema, objWithContext;
+	let dispatcher, schema, objWithContext;
 
 	beforeEach( () => {
 		// `additionalData` parameter for `.convert` calls.
@@ -91,16 +90,12 @@ describe( 'View converter builder', () => {
 
 		dispatcher = new ViewConversionDispatcher( { schema } );
 		dispatcher.on( 'text', convertText() );
-
-		modelDoc = new ModelDocument();
-		modelRoot = modelDoc.createRoot();
 	} );
 
 	it( 'should convert from view element to model element', () => {
 		buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' );
 
 		const conversionResult = dispatcher.convert( new ViewContainerElement( 'p', null, new ViewText( 'foo' ) ), objWithContext );
-		modelRoot.appendChildren( conversionResult );
 
 		expect( modelToString( conversionResult ) ).to.equal( '<paragraph>foo</paragraph>' );
 	} );
@@ -111,7 +106,6 @@ describe( 'View converter builder', () => {
 			.toElement( viewElement => new ModelElement( 'image', { src: viewElement.getAttribute( 'src' ) } ) );
 
 		const conversionResult = dispatcher.convert( new ViewContainerElement( 'img', { src: 'foo.jpg' } ), objWithContext );
-		modelRoot.appendChildren( conversionResult );
 
 		expect( modelToString( conversionResult ) ).to.equal( '<image src="foo.jpg"></image>' );
 	} );
@@ -122,10 +116,9 @@ describe( 'View converter builder', () => {
 		const conversionResult = dispatcher.convert(
 			new ViewAttributeElement( 'strong', null, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( conversionResult );
 
 		// Have to check root because result is a ModelText.
-		expect( modelToString( modelRoot ) ).to.equal( '<$root><$text bold="true">foo</$text></$root>' );
+		expect( modelToString( conversionResult ) ).to.equal( '<$text bold="true">foo</$text>' );
 	} );
 
 	it( 'should convert from view element to model attributes using creator function', () => {
@@ -136,10 +129,9 @@ describe( 'View converter builder', () => {
 		const conversionResult = dispatcher.convert(
 			new ViewAttributeElement( 'a', { href: 'foo.html' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( conversionResult );
 
 		// Have to check root because result is a ModelText.
-		expect( modelToString( modelRoot ) ).to.equal( '<$root><$text linkHref="foo.html">foo</$text></$root>' );
+		expect( modelToString( conversionResult ) ).to.equal( '<$text linkHref="foo.html">foo</$text>' );
 	} );
 
 	it( 'should convert from view attribute to model attribute', () => {
@@ -152,7 +144,6 @@ describe( 'View converter builder', () => {
 		const conversionResult = dispatcher.convert(
 			new ViewContainerElement( 'p', { class: 'myClass' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( conversionResult );
 
 		expect( modelToString( conversionResult ) ).to.equal( '<paragraph class="myClass">foo</paragraph>' );
 	} );
@@ -200,7 +191,6 @@ describe( 'View converter builder', () => {
 		] );
 
 		const conversionResult = dispatcher.convert( viewElement, objWithContext );
-		modelRoot.appendChildren( conversionResult );
 
 		expect( modelToString( conversionResult ) ).to.equal( '<paragraph><$text bold="true">aaabbbcccddd</$text></paragraph>' );
 	} );
@@ -337,7 +327,7 @@ describe( 'View converter builder', () => {
 		result = dispatcher.convert(
 			new ViewContainerElement( 'span', { class: 'megatron' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( result );
+
 		expect( modelToString( result ) ).to.equal( '<span>foo</span>' );
 
 		// Almost a megatron. Missing a head.
@@ -346,7 +336,6 @@ describe( 'View converter builder', () => {
 			objWithContext
 		);
 
-		modelRoot.appendChildren( result );
 		expect( modelToString( result ) ).to.equal( '<span>foo</span>' );
 
 		// This would be a megatron but is a paragraph.
@@ -359,7 +348,6 @@ describe( 'View converter builder', () => {
 			objWithContext
 		);
 
-		modelRoot.appendChildren( result );
 		expect( modelToString( result ) ).to.equal( '<paragraph>foo</paragraph>' );
 
 		// At last we have a megatron!
@@ -372,7 +360,6 @@ describe( 'View converter builder', () => {
 			objWithContext
 		);
 
-		modelRoot.appendChildren( result );
 		expect( modelToString( result ) ).to.equal( '<MEGATRON>foo</MEGATRON>' );
 	} );
 
@@ -392,7 +379,6 @@ describe( 'View converter builder', () => {
 
 		const conversionResult = dispatcher.convert( viewElement, objWithContext );
 
-		modelRoot.appendChildren( conversionResult );
 		expect( modelToString( conversionResult ) ).to.equal( '<span transformer="megatron">foo</span>' );
 	} );
 
@@ -415,7 +401,6 @@ describe( 'View converter builder', () => {
 		const conversionResult = dispatcher.convert(
 			new ViewContainerElement( 'p', { class: 'myClass' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( conversionResult );
 
 		// Element converter was fired first even though attribute converter was added first.
 		expect( modelToString( conversionResult ) ).to.equal( '<paragraph class="myClass">foo</paragraph>' );
@@ -432,7 +417,7 @@ describe( 'View converter builder', () => {
 		result = dispatcher.convert(
 			new ViewContainerElement( 'p', { class: 'myClass' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( result );
+
 		expect( modelToString( result ) ).to.equal( '<paragraph class="myClass">foo</paragraph>' );
 
 		buildViewConverter().for( dispatcher )
@@ -442,7 +427,7 @@ describe( 'View converter builder', () => {
 		result = dispatcher.convert(
 			new ViewContainerElement( 'p', { class: 'myClass' }, new ViewText( 'foo' ) ), objWithContext
 		);
-		modelRoot.appendChildren( result );
+
 		expect( modelToString( result ) ).to.equal( '<customP>foo</customP>' );
 	} );
 
@@ -461,9 +446,7 @@ describe( 'View converter builder', () => {
 			.toAttribute( 'size', 'small' );
 
 		const viewElement = new ViewContainerElement( 'p', { class: 'decorated small' }, new ViewText( 'foo' ) );
-
 		const conversionResult = dispatcher.convert( viewElement, objWithContext );
-		modelRoot.appendChildren( conversionResult );
 
 		// P element and it's children got converted by the converter (1) and the converter (1) got fired
 		// because P name was not consumed in converter (2). Converter (3) could consume class="small" because
@@ -487,7 +470,6 @@ describe( 'View converter builder', () => {
 		] );
 
 		const conversionResult = dispatcher.convert( viewStructure, objWithContext );
-		modelRoot.appendChildren( conversionResult );
 
 		expect( modelToString( conversionResult ) ).to.equal( '<div class="myClass"><abcd>foo</abcd></div>' );
 	} );

+ 1 - 1
packages/ckeditor5-engine/tests/model/node.js

@@ -16,7 +16,7 @@ describe( 'Node', () => {
 		one, two, three,
 		textBA, textR, img;
 
-	before( () => {
+	beforeEach( () => {
 		node = new Node();
 
 		one = new Element( 'one' );