diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index beab8569b..18e09f072 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -78,9 +78,11 @@ export default class EditingController { this.view._renderingDisabled = true; }, { priority: 'highest' } ); - this.listenTo( this.model, '_afterChanges', () => { + this.listenTo( this.model, '_afterChanges', ( evt, { hasModelDocumentChanged } ) => { this.view._renderingDisabled = false; - this.view.render(); + if ( hasModelDocumentChanged ) { + this.view.render(); + } }, { priority: 'lowest' } ); // Whenever model document is changed, convert those changes to the view (using model.Document#differ). diff --git a/src/model/document.js b/src/model/document.js index e367ba533..41ecbb5d5 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -46,7 +46,7 @@ export default class Document { * The {@link module:engine/model/model~Model model} that the document is a part of. * * @readonly - * @member {module:engine/model/model~Model} + * @type {module:engine/model/model~Model} */ this.model = model; @@ -58,7 +58,7 @@ export default class Document { * a {@link module:utils/ckeditorerror~CKEditorError model-document-applyOperation-wrong-version} error is thrown. * * @readonly - * @member {Number} + * @type {Number} */ this.version = 0; @@ -66,7 +66,7 @@ export default class Document { * The document's history. * * @readonly - * @member {module:engine/model/history~History} + * @type {module:engine/model/history~History} */ this.history = new History( this ); @@ -74,7 +74,7 @@ export default class Document { * The selection in this document. * * @readonly - * @member {module:engine/model/documentselection~DocumentSelection} + * @type {module:engine/model/documentselection~DocumentSelection} */ this.selection = new DocumentSelection( this ); @@ -83,7 +83,7 @@ export default class Document { * {@link #getRoot} to manipulate it. * * @readonly - * @member {module:utils/collection~Collection} + * @type {module:utils/collection~Collection} */ this.roots = new Collection( { idProperty: 'rootName' } ); @@ -91,7 +91,7 @@ export default class Document { * The model differ object. Its role is to buffer changes done on the model document and then calculate a diff of those changes. * * @readonly - * @member {module:engine/model/differ~Differ} + * @type {module:engine/model/differ~Differ} */ this.differ = new Differ( model.markers ); @@ -99,10 +99,18 @@ export default class Document { * Post-fixer callbacks registered to the model document. * * @private - * @member {Set} + * @type {Set.} */ this._postFixers = new Set(); + /** + * A boolean indicates whether the selection has changed until + * + * @private + * @type {Boolean} + */ + this._hasSelectionChangedFromTheLastChangeBlock = false; + // Graveyard tree root. Document always have a graveyard root, which stores removed nodes. this.createRoot( '$root', graveyardName ); @@ -144,29 +152,8 @@ export default class Document { }, { priority: 'low' } ); // Listen to selection changes. If selection changed, mark it. - let hasSelectionChanged = false; - this.listenTo( this.selection, 'change', () => { - hasSelectionChanged = true; - } ); - - // Wait for `_change` event from model, which signalizes that outermost change block has finished. - // When this happens, check if there were any changes done on document, and if so, call post-fixers, - // fire `change` event for features and conversion and then reset the differ. - // Fire `change:data` event when at least one operation or buffered marker changes the data. - this.listenTo( model, '_change', ( evt, writer ) => { - if ( !this.differ.isEmpty || hasSelectionChanged ) { - this._callPostFixers( writer ); - - if ( this.differ.hasDataChanges() ) { - this.fire( 'change:data', writer.batch ); - } else { - this.fire( 'change', writer.batch ); - } - - this.differ.reset(); - hasSelectionChanged = false; - } + this._hasSelectionChangedFromTheLastChangeBlock = true; } ); // Buffer marker changes. @@ -306,6 +293,44 @@ export default class Document { return json; } + /** + * Check if there were any changes done on document, and if so, call post-fixers, + * fire `change` event for features and conversion and then reset the differ. + * Fire `change:data` event when at least one operation or buffered marker changes the data. + * + * @protected + * @fires change + * @fires change:data + * @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixers will be called. + */ + _handleChangeBlock( writer ) { + if ( this._hasDocumentChangedFromTheLastChangeBlock() ) { + this._callPostFixers( writer ); + + if ( this.differ.hasDataChanges() ) { + this.fire( 'change:data', writer.batch ); + } else { + this.fire( 'change', writer.batch ); + } + + this.differ.reset(); + } + + this._hasSelectionChangedFromTheLastChangeBlock = false; + } + + /** + * Returns whether there is a buffered change or if the selection has changed from the last + * {@link module:engine/model/model~Model#enqueueChange `enqueueChange()` block} + * or {@link module:engine/model/model~Model#change `change()` block}. + * + * @protected + * @returns {Boolean} Returns `true` if document has changed from the last `change()` or `enqueueChange()` block. + */ + _hasDocumentChangedFromTheLastChangeBlock() { + return !this.differ.isEmpty || this._hasSelectionChangedFromTheLastChangeBlock; + } + /** * Returns the default root for this document which is either the first root that was added to the document using * {@link #createRoot} or the {@link #graveyard graveyard root} if no other roots were created. @@ -359,6 +384,7 @@ export default class Document { * Performs post-fixer loops. Executes post-fixer callbacks as long as none of them has done any changes to the model. * * @private + * @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixer callbacks will be called. */ _callPostFixers( writer ) { let wasFixed = false; diff --git a/src/model/model.js b/src/model/model.js index ffb6b1178..ca78ab126 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -683,6 +683,7 @@ export default class Model { */ _runPendingChanges() { const ret = []; + let hasModelDocumentChanged = false; this.fire( '_beforeChanges' ); @@ -695,14 +696,19 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - // Fire internal `_change` event. + // Collect an information whether the model document has changed during from the last pending change. + hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); + + // Fire '_change' event before resetting differ. this.fire( '_change', this._currentWriter ); + this.document._handleChangeBlock( this._currentWriter ); + this._pendingChanges.shift(); this._currentWriter = null; } - this.fire( '_afterChanges' ); + this.fire( '_afterChanges', { hasModelDocumentChanged } ); return ret; } @@ -713,6 +719,7 @@ export default class Model { * * **Note:** This is an internal event! Use {@link module:engine/model/document~Document#event:change} instead. * + * @deprecated * @protected * @event _change * @param {module:engine/model/writer~Writer} writer `Writer` instance that has been used in the change block. @@ -732,6 +739,9 @@ export default class Model { * * @protected * @event _afterChanges + * @param {Object} options + * @param {Boolean} options.hasModelDocumentChanged `true` if the model document has changed during the + * {@link module:engine/model/model~Model#change} or {@link module:engine/model/model~Model#enqueueChange} blocks. */ /** diff --git a/tests/tickets/1653.js b/tests/tickets/1653.js new file mode 100644 index 000000000..fd6fe7052 --- /dev/null +++ b/tests/tickets/1653.js @@ -0,0 +1,34 @@ +/** + * @license Copyright (c) 2003-2019, CKSource - Frederico Knabben. All rights reserved. + * For licensing, see LICENSE.md. + */ + +/* globals document */ + +import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor'; +import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph'; + +describe( 'Bug ckeditor5-engine#1653', () => { + it( '`DataController.parse()` should not invoke `editing.view.render()`', () => { + let editor; + + const element = document.createElement( 'div' ); + document.body.appendChild( element ); + + return ClassicTestEditor + .create( element, { plugins: [ Paragraph ] } ) + .then( newEditor => { + editor = newEditor; + + const spy = sinon.spy( editor.editing.view, 'render' ); + editor.data.parse( '

' ); + + sinon.assert.notCalled( spy ); + } ) + .then( () => { + element.remove(); + + return editor.destroy(); + } ); + } ); +} );