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 #1811 from ckeditor/i/5600
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Reinmar authored Dec 17, 2019
2 parents 28c6f90 + 6e2a5d3 commit b7e2bfe
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 9 deletions.
11 changes: 7 additions & 4 deletions src/view/observer/mutationobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 4 additions & 0 deletions src/view/uielement.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}
*/
Expand Down
30 changes: 25 additions & 5 deletions tests/view/observer/mutationobserver.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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', () => {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -528,6 +528,7 @@ describe( 'MutationObserver', () => {
} );

describe( 'UIElement integration', () => {
const renderStub = sinon.stub();
function createUIElement( name ) {
const element = new UIElement( name );

Expand All @@ -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', () => {
Expand All @@ -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 );
} );
} );

Expand Down

0 comments on commit b7e2bfe

Please sign in to comment.