Merge pull request #901 from ckeditor/t/888-2

Fix: view.Renderer will deeply unbind DOM elements when they are removed from DOM. Closes #888.

src/view/domconverter.js

@@ -134,15 +134,22 @@ export default class DomConverter {
 	}
 
 	/**
-	 * Unbinds given `domElement` from the view element it was bound to.
+	 * Unbinds given `domElement` from the view element it was bound to. Unbinding is deep, meaning that all children of
+	 * `domElement` will be unbound too.
 	 *
 	 * @param {HTMLElement} domElement DOM element to unbind.
 	 */
 	unbindDomElement( domElement ) {
 		const viewElement = this._domToViewMapping.get( domElement );
 
-		this._domToViewMapping.delete( domElement );
-		this._viewToDomMapping.delete( viewElement );
+		if ( viewElement ) {
+			this._domToViewMapping.delete( domElement );
+			this._viewToDomMapping.delete( viewElement );
+
+			for ( let child of domElement.childNodes ) {
+				this.unbindDomElement( child );
+			}
+		}
 	}
 
 	/**

src/view/renderer.js

@@ -472,7 +472,7 @@ export default class Renderer {
 				insertAt( domElement, i, expectedDomChildren[ i ] );
 				i++;
 			} else if ( action === 'delete' ) {
-				// Whenever element is removed from DOM, unbind it.
+				// Whenever element is removed from DOM, unbind it and all of its children.
 				this.domConverter.unbindDomElement( actualDomChildren[ i ] );
 				remove( actualDomChildren[ i ] );
 			} else { // 'equal'

tests/view/renderer.js

@@ -325,6 +325,45 @@ describe( 'Renderer', () => {
 			expect( domP.childNodes.length ).to.equal( 0 );
 		} );
 
+		it( 'should update removed item when it is reinserted #2', () => {
+			// Prepare view: root -> div "outer" -> div "inner" -> p.
+			const viewP = new ViewElement( 'p' );
+			const viewDivInner = new ViewElement( 'div', null, viewP );
+			const viewDivOuter = new ViewElement( 'div', null, viewDivInner );
+			viewRoot.appendChildren( viewDivOuter );
+
+			// Render view tree to DOM.
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			// Remove div "outer" from root and render it.
+			viewDivOuter.remove();
+			renderer.markToSync( 'children', viewRoot );
+			renderer.render();
+
+			// Remove p from div "child" -- div "inner" won't be marked because it is in document fragment not view root.
+			viewP.remove();
+			// Add div "outer" back to root.
+			viewRoot.appendChildren( viewDivOuter );
+			renderer.markToSync( 'children', viewRoot );
+
+			// Render changes, view is: root -> div "outer" -> div "inner".
+			renderer.render();
+
+			// Same is expected in DOM.
+			expect( domRoot.childNodes.length ).to.equal( 1 );
+
+			const domDivOuter = domRoot.childNodes[ 0 ];
+			expect( renderer.domConverter.viewToDom( viewDivOuter, domRoot.document ) ).to.equal( domDivOuter );
+			expect( domDivOuter.tagName ).to.equal( 'DIV' );
+			expect( domDivOuter.childNodes.length ).to.equal( 1 );
+
+			const domDivInner = domDivOuter.childNodes[ 0 ];
+			expect( renderer.domConverter.viewToDom( viewDivInner, domRoot.document ) ).to.equal( domDivInner );
+			expect( domDivInner.tagName ).to.equal( 'DIV' );
+			expect( domDivInner.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 );