浏览代码

Changed: `Renderer` will unbind DOM elements from view elements when removing them from DOM.

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

+ 12 - 0
packages/ckeditor5-engine/src/view/domconverter.js

@@ -134,6 +134,18 @@ export default class DomConverter {
 	}
 	}
 
 
 	/**
 	/**
+	 * Unbinds given `domElement` from the view element it was bound to.
+	 *
+	 * @param {HTMLElement} domElement DOM element to unbind.
+	 */
+	unbindDomElement( domElement ) {
+		const viewElement = this._domToViewMapping.get( domElement );
+
+		this._domToViewMapping.delete( domElement );
+		this._viewToDomMapping.delete( viewElement );
+	}
+
+	/**
 	 * Binds DOM and View document fragments, so it will be possible to get corresponding document fragments using
 	 * Binds DOM and View document fragments, so it will be possible to get corresponding document fragments using
 	 * {@link module:engine/view/domconverter~DomConverter#getCorrespondingViewDocumentFragment getCorrespondingViewDocumentFragment} and
 	 * {@link module:engine/view/domconverter~DomConverter#getCorrespondingViewDocumentFragment getCorrespondingViewDocumentFragment} and
 	 * {@link module:engine/view/domconverter~DomConverter#getCorrespondingDomDocumentFragment getCorrespondingDomDocumentFragment}.
 	 * {@link module:engine/view/domconverter~DomConverter#getCorrespondingDomDocumentFragment getCorrespondingDomDocumentFragment}.

+ 9 - 0
packages/ckeditor5-engine/src/view/renderer.js

@@ -437,6 +437,13 @@ export default class Renderer {
 	_updateChildren( viewElement, options ) {
 	_updateChildren( viewElement, options ) {
 		const domConverter = this.domConverter;
 		const domConverter = this.domConverter;
 		const domElement = domConverter.getCorrespondingDom( viewElement );
 		const domElement = domConverter.getCorrespondingDom( viewElement );
+
+		if ( !domElement ) {
+			// If there is no `domElement` it means that it was already removed from DOM.
+			// There is no need to update it. It will be updated when re-inserted.
+			return;
+		}
+
 		const domDocument = domElement.ownerDocument;
 		const domDocument = domElement.ownerDocument;
 
 
 		const filler = options.inlineFillerPosition;
 		const filler = options.inlineFillerPosition;
@@ -463,6 +470,8 @@ export default class Renderer {
 				insertAt( domElement, i, expectedDomChildren[ i ] );
 				insertAt( domElement, i, expectedDomChildren[ i ] );
 				i++;
 				i++;
 			} else if ( action === 'delete' ) {
 			} else if ( action === 'delete' ) {
+				// Whenever element is removed from DOM, unbind it.
+				this.domConverter.unbindDomElement( actualDomChildren[ i ] );
 				remove( actualDomChildren[ i ] );
 				remove( actualDomChildren[ i ] );
 			} else { // 'equal'
 			} else { // 'equal'
 				i++;
 				i++;

+ 57 - 0
packages/ckeditor5-engine/tests/view/renderer.js

@@ -290,6 +290,63 @@ describe( 'Renderer', () => {
 			expect( domRoot.childNodes[ 0 ].tagName ).to.equal( 'P' );
 			expect( domRoot.childNodes[ 0 ].tagName ).to.equal( 'P' );
 		} );
 		} );
 
 
+		it( 'should update removed item when it is reinserted', () => {
+			const viewFoo = new ViewText( 'foo' );
+			const viewP = new ViewElement( 'p', null, viewFoo );
+			const viewDiv = new ViewElement( 'div', null, viewP );
+
+			viewRoot.appendChildren( viewDiv );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			viewDiv.removeChildren( 0, 1 );
+			renderer.markToSync( 'children', viewDiv );
+			renderer.render();
+
+			viewP.removeChildren( 0, 1 );
+
+			viewDiv.appendChildren( viewP );
+			renderer.markToSync( 'children', viewDiv );
+			renderer.render();
+
+			expect( domRoot.childNodes.length ).to.equal( 1 );
+
+			const domDiv = domRoot.childNodes[ 0 ];
+
+			expect( domDiv.tagName ).to.equal( 'DIV' );
+			expect( domDiv.childNodes.length ).to.equal( 1 );
+
+			const domP = domDiv.childNodes[ 0 ];
+
+			expect( domP.tagName ).to.equal( 'P' );
+			expect( domP.childNodes.length ).to.equal( 0 );
+		} );
+
+		it( 'should not throw when trying to update children of view element that got removed and lost its binding', () => {
+			const viewFoo = new ViewText( 'foo' );
+			const viewP = new ViewElement( 'p', null, viewFoo );
+			const viewDiv = new ViewElement( 'div', null, viewP );
+
+			viewRoot.appendChildren( viewDiv );
+
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			viewRoot.removeChildren( 0, 1 );
+			renderer.markToSync( 'children', viewRoot );
+
+			viewDiv.removeChildren( 0, 1 );
+			renderer.markToSync( 'children', viewDiv );
+
+			viewP.removeChildren( 0, 1 );
+			renderer.markToSync( 'children', viewP );
+
+			renderer.render();
+
+			expect( domRoot.childNodes.length ).to.equal( 0 );
+		} );
+
 		it( 'should not care about filler if there is no DOM', () => {
 		it( 'should not care about filler if there is no DOM', () => {
 			selectionEditable = null;
 			selectionEditable = null;