Преглед изворни кода

Merge pull request #1039 from ckeditor/t/1031

Fix: Mutation observer will ignore children mutations if as a result of several native mutations the element's children haven't changed. Closes #1031.
Piotr Jasiun пре 8 година
родитељ
комит
e564a6ac62

+ 35 - 10
packages/ckeditor5-engine/src/view/observer/mutationobserver.js

@@ -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
@@ -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.
@@ -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;
+		}
 	}
 
 	/**

+ 34 - 0
packages/ckeditor5-engine/tests/view/observer/mutationobserver.js

@@ -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>' ) );