Merge pull request #890 from ckeditor/t/888

Fix: Renderer will unbind DOM elements from view elements when removing them from DOM. Closes #888.

src/view/domconverter.js

@@ -133,6 +133,18 @@ export default class DomConverter {
  this._viewToDomMapping.set( viewElement, domElement );
  }
 
+ /**
+ * 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
  * {@link module:engine/view/domconverter~DomConverter#getCorrespondingViewDocumentFragment getCorrespondingViewDocumentFragment} and

src/view/renderer.js

@@ -437,6 +437,13 @@ export default class Renderer {
  _updateChildren( viewElement, options ) {
  const domConverter = this.domConverter;
  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 filler = options.inlineFillerPosition;
@@ -463,6 +470,8 @@ export default class Renderer {
  insertAt( domElement, i, expectedDomChildren[ i ] );
  i++;
  } else if ( action === 'delete' ) {
+ // Whenever element is removed from DOM, unbind it.
+ this.domConverter.unbindDomElement( actualDomChildren[ i ] );
  remove( actualDomChildren[ i ] );
  } else { // 'equal'
  i++;

tests/view/renderer.js

@@ -290,6 +290,63 @@ describe( 'Renderer', () => {
  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', () => {
  selectionEditable = null;