Merge pull request #1811 from ckeditor/i/5600

Fix: Changes irrelevant to the view (e.g. inside UIElements) will no longer force a view render nor will they trigger mutation event on the document. Closes ckeditor/ckeditor5#5600.

Author: Reinmar

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 skips the event and force render (#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.

src/view/uielement.js

@@ -123,6 +123,10 @@ export default class UIElement extends Element {
 	 *			return domElement;
 	 *		};
 	 *
+	 * If changes in your UI element should trigger some editor UI update you should call
+	 * the {@link module:core/editor/editorui~EditorUI#update `editor.ui.update()`} method
+	 * after rendering your UI element.
+	 *
 	 * @param {Document} domDocument
 	 * @returns {HTMLElement}
 	 */

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
@@ -528,6 +528,7 @@ describe( 'MutationObserver', () => {
 	} );
 
 	describe( 'UIElement integration', () => {
+		const renderStub = sinon.stub();
 		function createUIElement( name ) {
 			const element = new UIElement( name );
 
@@ -546,14 +547,24 @@ describe( 'MutationObserver', () => {
 			viewRoot._appendChild( uiElement );
 
 			view.forceRender();
+			renderStub.reset();
+			view.on( 'render', renderStub );
 		} );
 
 		it( 'should not collect text mutations from UIElement', () => {
 			domEditor.childNodes[ 2 ].childNodes[ 0 ].data = 'foom';
 
 			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 +573,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 );
 		} );
 	} );