Przeglądaj źródła

Fixed: Change in UIElement view will no longer force a view render nor will it trigger mutations event on the document.

Marek Lewandowski 6 lat temu
rodzic
commit
5075a0b77f

+ 7 - 4
packages/ckeditor5-engine/src/view/observer/mutationobserver.js

@@ -244,11 +244,14 @@ export default class MutationObserver extends Observer {
 			}
 		}
 
-		this.document.fire( 'mutations', viewMutations, viewSelection );
+		// In case only non-relevant mutations were recorded it skip event / force render for the sake of performance (#5600).
+		if ( viewMutations.length ) {
+			this.document.fire( 'mutations', viewMutations, viewSelection );
 
-		// 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.view.forceRender();
+			// 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.view.forceRender();
+		}
 
 		function sameNodes( child1, child2 ) {
 			// First level of comparison (array of children vs array of children) – use the Lodash's default behavior.

+ 26 - 5
packages/ckeditor5-engine/tests/view/observer/mutationobserver.js

@@ -404,7 +404,7 @@ describe( 'MutationObserver', () => {
 
 		mutationObserver.flush();
 
-		expect( lastMutations.length ).to.equal( 0 );
+		expect( lastMutations ).to.be.null;
 	} );
 
 	it( 'should ignore mutation with bogus br inserted on the end of the paragraph with text', () => {
@@ -417,7 +417,7 @@ describe( 'MutationObserver', () => {
 
 		mutationObserver.flush();
 
-		expect( lastMutations.length ).to.equal( 0 );
+		expect( lastMutations ).to.be.null;
 	} );
 
 	it( 'should ignore mutation with bogus br inserted on the end of the paragraph while processing text mutations', () => {
@@ -449,7 +449,7 @@ describe( 'MutationObserver', () => {
 
 		mutationObserver.flush();
 
-		expect( lastMutations.length ).to.equal( 0 );
+		expect( lastMutations ).to.be.null;
 	} );
 
 	// This case is more tricky than the previous one because DOMConverter will return a different
@@ -541,11 +541,15 @@ describe( 'MutationObserver', () => {
 			return element;
 		}
 
+		const renderStub = sinon.stub();
+
 		beforeEach( () => {
 			const uiElement = createUIElement( 'div' );
 			viewRoot._appendChild( uiElement );
 
 			view.forceRender();
+			renderStub.reset();
+			view.on( 'render', renderStub );
 		} );
 
 		it( 'should not collect text mutations from UIElement', () => {
@@ -553,7 +557,15 @@ describe( 'MutationObserver', () => {
 
 			mutationObserver.flush();
 
-			expect( lastMutations.length ).to.equal( 0 );
+			expect( lastMutations ).to.be.null;
+		} );
+
+		it( 'should not cause a render from UIElement', () => {
+			domEditor.childNodes[ 2 ].childNodes[ 0 ].data = 'foom';
+
+			mutationObserver.flush();
+
+			expect( renderStub.callCount ).to.be.equal( 0 );
 		} );
 
 		it( 'should not collect child mutations from UIElement', () => {
@@ -562,7 +574,16 @@ describe( 'MutationObserver', () => {
 
 			mutationObserver.flush();
 
-			expect( lastMutations.length ).to.equal( 0 );
+			expect( lastMutations ).to.be.null;
+		} );
+
+		it( 'should not cause a render when UIElement gets a child', () => {
+			const span = document.createElement( 'span' );
+			domEditor.childNodes[ 2 ].appendChild( span );
+
+			mutationObserver.flush();
+
+			expect( renderStub.callCount ).to.be.equal( 0 );
 		} );
 	} );