Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge pull request #1039 from ckeditor/t/1031
Browse files Browse the repository at this point in the history
Fix: Mutation observer will ignore children mutations if as a result of several native mutations the element's children haven't changed. Closes #1031.
  • Loading branch information
Piotr Jasiun authored Jul 25, 2017
2 parents 4a4a89a + 852c390 commit 552198e
Show file tree
Hide file tree
Showing 2 changed files with 69 additions and 10 deletions.
45 changes: 35 additions & 10 deletions src/view/observer/mutationobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import Observer from './observer';
import ViewSelection from '../selection';
import { startsWithFiller, getDataWithoutFiller } from '../filler';
import isEqualWith from '@ckeditor/ckeditor5-utils/src/lib/lodash/isEqualWith';

/**
* Mutation observer class observes changes in the DOM, fires {@link module:engine/view/document~Document#event:mutations} event, mark view
Expand Down Expand Up @@ -204,16 +205,21 @@ export default class MutationObserver extends Observer {

for ( const viewElement of mutatedElements ) {
const domElement = domConverter.mapViewToDom( viewElement );
const viewChildren = viewElement.getChildren();
const newViewChildren = domConverter.domChildrenToView( domElement );

this.renderer.markToSync( 'children', viewElement );
viewMutations.push( {
type: 'children',
oldChildren: Array.from( viewChildren ),
newChildren: Array.from( newViewChildren ),
node: viewElement
} );
const viewChildren = Array.from( viewElement.getChildren() );
const newViewChildren = Array.from( domConverter.domChildrenToView( domElement ) );

// It may happen that as a result of many changes (sth was inserted and then removed),
// both elements haven't really changed. #1031
if ( !isEqualWith( viewChildren, newViewChildren, sameNodes ) ) {
this.renderer.markToSync( 'children', viewElement );

viewMutations.push( {
type: 'children',
oldChildren: viewChildren,
newChildren: newViewChildren,
node: viewElement
} );
}
}

// Retrieve `domSelection` using `ownerDocument` of one of mutated nodes.
Expand Down Expand Up @@ -244,6 +250,25 @@ export default class MutationObserver extends Observer {
// If nothing changes on `mutations` event, at this point we have "dirty DOM" (changed) and de-synched
// view (which has not been changed). In order to "reset DOM" we render the view again.
this.document.render();

function sameNodes( child1, child2 ) {
// First level of comparison (array of children vs array of children) – use the Lodash's default behavior.
if ( Array.isArray( child1 ) ) {
return;
}

// Elements.
if ( child1 === child2 ) {
return true;
}
// Texts.
else if ( child1.is( 'text' ) && child2.is( 'text' ) ) {
return child1.data === child2.data;
}

// Not matching types.
return false;
}
}

/**
Expand Down
34 changes: 34 additions & 0 deletions tests/view/observer/mutationobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,6 +291,40 @@ describe( 'MutationObserver', () => {
expect( lastMutations[ 0 ].newText ).to.equal( 'foo ' );
} );

it( 'should ignore child mutations which resulted in no changes – when element contains elements', () => {
viewRoot.appendChildren( parse( '<container:p><container:x></container:x></container:p>' ) );

viewDocument.render();

const domP = domEditor.childNodes[ 2 ];
const domY = document.createElement( 'y' );
domP.appendChild( domY );
domY.remove();

mutationObserver.flush();

expect( lastMutations.length ).to.equal( 0 );
} );

// This case is more tricky than the previous one because DOMConverter will return a different
// instances of view text nodes every time it converts a DOM text node.
it( 'should ignore child mutations which resulted in no changes – when element contains text nodes', () => {
const domP = domEditor.childNodes[ 0 ];
const domText = document.createTextNode( 'x' );
domP.appendChild( domText );
domText.remove();

const domP2 = domEditor.childNodes[ 1 ];
domP2.appendChild( document.createTextNode( 'x' ) );

mutationObserver.flush();

// There was onlu P2 change. P1 must be ignored.
const viewP2 = viewRoot.getChild( 1 );
expect( lastMutations.length ).to.equal( 1 );
expect( lastMutations[ 0 ].node ).to.equal( viewP2 );
} );

it( 'should not ignore mutation with br inserted not on the end of the paragraph', () => {
viewRoot.appendChildren( parse( '<container:p>foo</container:p>' ) );

Expand Down

0 comments on commit 552198e

Please sign in to comment.