diff --git a/packages/ckeditor5-engine/src/view/renderer.js b/packages/ckeditor5-engine/src/view/renderer.js index 0981852df84..ee47822c217 100644 --- a/packages/ckeditor5-engine/src/view/renderer.js +++ b/packages/ckeditor5-engine/src/view/renderer.js @@ -930,6 +930,7 @@ function addInlineFiller( domDocument, domParentOrArray, offset ) { function areSimilar( node1, node2 ) { return isNode( node1 ) && isNode( node2 ) && !isText( node1 ) && !isText( node2 ) && + node1.nodeType !== 8 && node2.nodeType !== 8 && node1.tagName.toLowerCase() === node2.tagName.toLowerCase(); } diff --git a/packages/ckeditor5-engine/tests/view/renderer.js b/packages/ckeditor5-engine/tests/view/renderer.js index 6062b663f80..f769cb03115 100644 --- a/packages/ckeditor5-engine/tests/view/renderer.js +++ b/packages/ckeditor5-engine/tests/view/renderer.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license */ -/* globals document, window, NodeFilter, MutationObserver */ +/* globals document, window, NodeFilter, MutationObserver, HTMLImageElement */ import View from '../../src/view/view'; import ViewElement from '../../src/view/element'; @@ -305,6 +305,33 @@ describe( 'Renderer', () => { expect( domRoot.childNodes[ 0 ] ).to.equal( domImg ); } ); + it( 'should remove any comment from the DOM element', () => { + // This comment should be cleared during render. + const domComment = document.createComment( 'some comment from the user-land' ); + domRoot.appendChild( domComment ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( domRoot.childNodes.length ).to.equal( 0 ); + } ); + + // https://github.com/ckeditor/ckeditor5/issues/5734 + it( 'should replace a comment with the added element', () => { + const viewImg = new ViewElement( viewDocument, 'img' ); + viewRoot._appendChild( viewImg ); + + // This comment should be cleared during render. + const domComment = document.createComment( 'some comment from the user-land' ); + domRoot.appendChild( domComment ); + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + expect( domRoot.childNodes.length ).to.equal( 1 ); + expect( domRoot.childNodes[ 0 ] ).to.be.an.instanceof( HTMLImageElement ); + } ); + it( 'should change element if it is different', () => { const viewImg = new ViewElement( viewDocument, 'img' ); viewRoot._appendChild( viewImg ); @@ -2706,6 +2733,78 @@ describe( 'Renderer', () => { expect( mappings.get( domQULB ) ).to.equal( viewQ.getChild( 0 ).getChild( 0 ).getChild( 1 ) ); } ); + // https://github.com/ckeditor/ckeditor5/issues/5734 + it( 'should not rerender DOM when view replaced with the same structure without a comment', () => { + const domContent = '' + + '

Heading 1

' + + '

Ph Bold' + + 'Link' + + '

' + + '' + + '
'; + const content = '' + + 'He' + + 'ading 1' + + '' + + 'Ph ' + + 'Bold' + + '' + + 'Link' + + '' + + '' + + '' + + '' + + 'Quoted item 1' + + '' + + ''; + + domRoot.innerHTML = domContent; + viewRoot._appendChild( parse( content ) ); + + const viewH = viewRoot.getChild( 0 ); + const viewP = viewRoot.getChild( 1 ); + const viewQ = viewRoot.getChild( 2 ); + + const domH = domRoot.childNodes[ 0 ]; + const domHI = domH.childNodes[ 1 ]; + const domP = domRoot.childNodes[ 1 ]; + const domPT = domP.childNodes[ 0 ]; + const domPABI = domP.childNodes[ 2 ].childNodes[ 0 ].childNodes[ 1 ]; + const domC = domRoot.childNodes[ 2 ]; + const domQ = domRoot.childNodes[ 3 ]; + const domQULB = domQ.childNodes[ 0 ].childNodes[ 0 ].childNodes[ 1 ]; + + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + // Assert the comment is no longer in the content. + expect( domRoot.contains( domC ), 'domRoot should not contain the comment' ).to.be.false; + + // Assert content, without the comment. + expect( domRoot.innerHTML ).to.equal( '

Heading 1

Ph Bold' + + 'Link

' ); + + // Assert if other DOM elements did not change. + expect( domRoot.childNodes[ 0 ] ).to.equal( domH ); + expect( domH.childNodes[ 1 ] ).to.equal( domHI ); + expect( domRoot.childNodes[ 1 ] ).to.equal( domP ); + expect( domP.childNodes[ 0 ] ).to.equal( domPT ); + expect( domP.childNodes[ 2 ].childNodes[ 0 ].childNodes[ 1 ] ).to.equal( domPABI ); + // Note the shifted index of domQ, from 3 to 2. + expect( domRoot.childNodes[ 2 ] ).to.equal( domQ ); + expect( domQ.childNodes[ 0 ].childNodes[ 0 ].childNodes[ 1 ] ).to.equal( domQULB ); + + // Assert mappings. + const mappings = renderer.domConverter._domToViewMapping; + expect( mappings.get( domH ) ).to.equal( viewH ); + expect( mappings.get( domHI ) ).to.equal( viewH.getChild( 1 ) ); + expect( mappings.get( domP ) ).to.equal( viewP ); + expect( mappings.get( domPABI ) ).to.equal( viewP.getChild( 2 ).getChild( 0 ).getChild( 1 ) ); + expect( mappings.get( domQ ) ).to.equal( viewQ ); + expect( mappings.get( domQULB ) ).to.equal( viewQ.getChild( 0 ).getChild( 0 ).getChild( 1 ) ); + } ); + it( 'should not rerender DOM when view replaced with the same structure without first node', () => { const content = '' + 'He' +