From 34edf59e0cfbb87ad65079f59a2b75d01c024be2 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 14:16:14 +0100 Subject: [PATCH 01/13] Prevented `EditingController` from updating view if there are no changes. --- src/controller/editingcontroller.js | 6 ++++-- src/model/document.js | 6 +++++- src/model/model.js | 7 ++++--- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index beab8569b..622b45986 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', ( data, { modelChanged } ) => { this.view._renderingDisabled = false; - this.view.render(); + if ( modelChanged ) { + 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..98ef861a5 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -155,7 +155,11 @@ export default class Document { // 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 ) { + const documentChanged = !this.differ.isEmpty || hasSelectionChanged; + + evt.return = documentChanged; + + if ( documentChanged ) { this._callPostFixers( writer ); if ( this.differ.hasDataChanges() ) { diff --git a/src/model/model.js b/src/model/model.js index ffb6b1178..faa7dd812 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -683,6 +683,7 @@ export default class Model { */ _runPendingChanges() { const ret = []; + let modelChanged = false; this.fire( '_beforeChanges' ); @@ -695,14 +696,14 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - // Fire internal `_change` event. - this.fire( '_change', this._currentWriter ); + // Fire internal `_change` event and collect the change result. + modelChanged = modelChanged || this.fire( '_change', this._currentWriter ); this._pendingChanges.shift(); this._currentWriter = null; } - this.fire( '_afterChanges' ); + this.fire( '_afterChanges', { modelChanged } ); return ret; } From 37510a541cf6ddec34d13a8a16c16512f60a0942 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 15:31:33 +0100 Subject: [PATCH 02/13] Fixed _runPendingChanges(). --- src/model/model.js | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/model/model.js b/src/model/model.js index faa7dd812..7a7b07645 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -696,8 +696,9 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - // Fire internal `_change` event and collect the change result. - modelChanged = modelChanged || this.fire( '_change', this._currentWriter ); + // Fire internal `_change` event and collect the information whether the model is changed after the callback. + const modelChangedAfterCallback = this.fire( '_change', this._currentWriter ); + modelChanged = modelChanged || modelChangedAfterCallback; this._pendingChanges.shift(); this._currentWriter = null; @@ -733,6 +734,8 @@ export default class Model { * * @protected * @event _afterChanges + * @param {Object} options + * @param {Boolean} options.modelChanged A boolean indicates whether the model was changed during the {@link module:engine/model/model~Model#change} blocks. */ /** From bfea5958d28f76f925aa0dc971db0d0a3d43797d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 16:38:36 +0100 Subject: [PATCH 03/13] Introduced `Document#runPostFixersAndResetDiffer` and `Document#_hasDocumentChangedFromTheLastChangeBlock`. --- src/model/document.js | 87 ++++++++++++++++++++++++++----------------- src/model/model.js | 19 +++++++--- 2 files changed, 66 insertions(+), 40 deletions(-) diff --git a/src/model/document.js b/src/model/document.js index 98ef861a5..080ad98f6 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 ); @@ -143,34 +151,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 ) => { - const documentChanged = !this.differ.isEmpty || hasSelectionChanged; - - evt.return = documentChanged; - - if ( documentChanged ) { - 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. @@ -295,6 +277,42 @@ export default class Document { this._postFixers.add( postFixer ); } + /** + * 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. + * + * @fires change + * @fires change:data + * @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixers will be called. + */ + runPostFixersAndResetDiffer( 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` when document has changed from the last change block. + */ + _hasDocumentChangedFromTheLastChangeBlock() { + return !this.differ.isEmpty || this._hasSelectionChangedFromTheLastChangeBlock; + } + /** * A custom `toJSON()` method to solve child-parent circular dependencies. * @@ -363,6 +381,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 7a7b07645..96d77d9cc 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -683,7 +683,7 @@ export default class Model { */ _runPendingChanges() { const ret = []; - let modelChanged = false; + let hasDocumentChanged = false; this.fire( '_beforeChanges' ); @@ -696,15 +696,21 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - // Fire internal `_change` event and collect the information whether the model is changed after the callback. - const modelChangedAfterCallback = this.fire( '_change', this._currentWriter ); - modelChanged = modelChanged || modelChangedAfterCallback; + this.document.callPostFixers + + // Collect an information whether the model document has changed during from the last pending change callback. + hasDocumentChanged = hasDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); + + this.document.runPostFixersAndResetDiffer( this._currentWriter ); + + // With <3 for @scofalic. + this.fire( '_change', this._currentWriter ); this._pendingChanges.shift(); this._currentWriter = null; } - this.fire( '_afterChanges', { modelChanged } ); + this.fire( '_afterChanges', { hasDocumentChanged } ); return ret; } @@ -715,6 +721,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. @@ -735,7 +742,7 @@ export default class Model { * @protected * @event _afterChanges * @param {Object} options - * @param {Boolean} options.modelChanged A boolean indicates whether the model was changed during the {@link module:engine/model/model~Model#change} blocks. + * @param {Boolean} options.hasDocumentChanged A boolean indicates whether the model document has changed during the {@link module:engine/model/model~Model#change} blocks. */ /** From 6f1d845b0d01e3d3b30d34f6974f351664f94f2f Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 16:40:28 +0100 Subject: [PATCH 04/13] Made runPostFixersAndResetDiffer protected. --- src/model/document.js | 3 ++- src/model/model.js | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/model/document.js b/src/model/document.js index 080ad98f6..aa793d013 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -282,11 +282,12 @@ export default class Document { * 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. */ - runPostFixersAndResetDiffer( writer ) { + _runPostFixersAndResetDiffer( writer ) { if ( this._hasDocumentChangedFromTheLastChangeBlock() ) { this._callPostFixers( writer ); diff --git a/src/model/model.js b/src/model/model.js index 96d77d9cc..ae1145298 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -701,7 +701,7 @@ export default class Model { // Collect an information whether the model document has changed during from the last pending change callback. hasDocumentChanged = hasDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); - this.document.runPostFixersAndResetDiffer( this._currentWriter ); + this.document._runPostFixersAndResetDiffer( this._currentWriter ); // With <3 for @scofalic. this.fire( '_change', this._currentWriter ); From 0d1928627c535f15de5bdc9041498f9108419d57 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 16:46:11 +0100 Subject: [PATCH 05/13] Fixed variable names. --- src/controller/editingcontroller.js | 4 ++-- src/model/model.js | 7 +++---- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index 622b45986..ac7827011 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -78,9 +78,9 @@ export default class EditingController { this.view._renderingDisabled = true; }, { priority: 'highest' } ); - this.listenTo( this.model, '_afterChanges', ( data, { modelChanged } ) => { + this.listenTo( this.model, '_afterChanges', ( data, { hasModelDocumentChanged } ) => { this.view._renderingDisabled = false; - if ( modelChanged ) { + if ( hasModelDocumentChanged ) { this.view.render(); } }, { priority: 'lowest' } ); diff --git a/src/model/model.js b/src/model/model.js index ae1145298..7304c2946 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -683,7 +683,7 @@ export default class Model { */ _runPendingChanges() { const ret = []; - let hasDocumentChanged = false; + let hasModelDocumentChanged = false; this.fire( '_beforeChanges' ); @@ -699,18 +699,17 @@ export default class Model { this.document.callPostFixers // Collect an information whether the model document has changed during from the last pending change callback. - hasDocumentChanged = hasDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); + hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); this.document._runPostFixersAndResetDiffer( this._currentWriter ); - // With <3 for @scofalic. this.fire( '_change', this._currentWriter ); this._pendingChanges.shift(); this._currentWriter = null; } - this.fire( '_afterChanges', { hasDocumentChanged } ); + this.fire( '_afterChanges', { hasModelDocumentChanged } ); return ret; } From 6a990ffeade5261c29c8e285cd9169408e8a9b39 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 16:47:34 +0100 Subject: [PATCH 06/13] Added missing comment. --- src/model/document.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/model/document.js b/src/model/document.js index aa793d013..65f8f1835 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -151,6 +151,7 @@ export default class Document { } }, { priority: 'low' } ); + // Listen to selection changes. If selection changed, mark it. this.listenTo( this.selection, 'change', () => { this._hasSelectionChangedFromTheLastChangeBlock = true; } ); From d2caf0209f9878602d1a08380c3aed7403e4ae73 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 17:18:57 +0100 Subject: [PATCH 07/13] Added integration test. --- src/controller/editingcontroller.js | 2 +- tests/tickets/1653.js | 34 +++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) create mode 100644 tests/tickets/1653.js diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index ac7827011..18e09f072 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -78,7 +78,7 @@ export default class EditingController { this.view._renderingDisabled = true; }, { priority: 'highest' } ); - this.listenTo( this.model, '_afterChanges', ( data, { hasModelDocumentChanged } ) => { + this.listenTo( this.model, '_afterChanges', ( evt, { hasModelDocumentChanged } ) => { this.view._renderingDisabled = false; if ( hasModelDocumentChanged ) { this.view.render(); diff --git a/tests/tickets/1653.js b/tests/tickets/1653.js new file mode 100644 index 000000000..bf970c7dd --- /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' ); + const output = editor.data.parse( '

' ); + + sinon.assert.notCalled( spy ); + } ) + .then( () => { + element.remove(); + + return editor.destroy(); + } ); + } ) +} ); From 5af93623d5242c2c6ae2c2590c149a5eca9f3983 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 17:19:34 +0100 Subject: [PATCH 08/13] Simplified test. --- tests/tickets/1653.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tickets/1653.js b/tests/tickets/1653.js index bf970c7dd..62c5dfe38 100644 --- a/tests/tickets/1653.js +++ b/tests/tickets/1653.js @@ -21,7 +21,7 @@ describe( 'Bug ckeditor5-engine#1653', () => { editor = newEditor; const spy = sinon.spy( editor.editing.view, 'render' ); - const output = editor.data.parse( '

' ); + editor.data.parse( '

' ); sinon.assert.notCalled( spy ); } ) From e362dee6c9147f8014c86dedeec318e853b5417e Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 17:25:31 +0100 Subject: [PATCH 09/13] Fixed API docs. --- src/model/model.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/model/model.js b/src/model/model.js index 7304c2946..4ee0e80a7 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -741,7 +741,7 @@ export default class Model { * @protected * @event _afterChanges * @param {Object} options - * @param {Boolean} options.hasDocumentChanged A boolean indicates whether the model document has changed during the {@link module:engine/model/model~Model#change} blocks. + * @param {Boolean} options.hasModelDocumentChanged A boolean indicates whether the model document has changed during the {@link module:engine/model/model~Model#change} blocks. */ /** From c45f791dca6bccbd9b40d34e4f403303e3271878 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Thu, 31 Jan 2019 17:40:10 +0100 Subject: [PATCH 10/13] Fixed lint issues. --- src/model/document.js | 3 ++- src/model/model.js | 5 ++--- tests/tickets/1653.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/model/document.js b/src/model/document.js index 65f8f1835..57bd65c04 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -305,7 +305,8 @@ export default class Document { } /** - * 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} + * 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 diff --git a/src/model/model.js b/src/model/model.js index 4ee0e80a7..c745a40d8 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -696,8 +696,6 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - this.document.callPostFixers - // Collect an information whether the model document has changed during from the last pending change callback. hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); @@ -741,7 +739,8 @@ export default class Model { * @protected * @event _afterChanges * @param {Object} options - * @param {Boolean} options.hasModelDocumentChanged A boolean indicates whether the model document has changed during the {@link module:engine/model/model~Model#change} blocks. + * @param {Boolean} options.hasModelDocumentChanged A boolean indicates whether the model document has changed during the + * {@link module:engine/model/model~Model#change} blocks. */ /** diff --git a/tests/tickets/1653.js b/tests/tickets/1653.js index 62c5dfe38..fd6fe7052 100644 --- a/tests/tickets/1653.js +++ b/tests/tickets/1653.js @@ -30,5 +30,5 @@ describe( 'Bug ckeditor5-engine#1653', () => { return editor.destroy(); } ); - } ) + } ); } ); From 40b7345a98eb29cc2113ab74b1edef08f64f4504 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 1 Feb 2019 08:23:22 +0100 Subject: [PATCH 11/13] Fixed order of firing _change event and resetting differ. --- src/model/model.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/model/model.js b/src/model/model.js index c745a40d8..1351e796a 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -699,10 +699,11 @@ export default class Model { // Collect an information whether the model document has changed during from the last pending change callback. hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); - this.document._runPostFixersAndResetDiffer( this._currentWriter ); - + // Fire '_change' event before resetting differ. this.fire( '_change', this._currentWriter ); + this.document._runPostFixersAndResetDiffer( this._currentWriter ); + this._pendingChanges.shift(); this._currentWriter = null; } From 5bbeb84775b60f1bea431305aadb8a7d8c7c004d Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Fri, 1 Feb 2019 09:34:51 +0100 Subject: [PATCH 12/13] API docs fixes. --- src/model/document.js | 36 ++++++++++++++++++------------------ src/model/model.js | 8 ++++---- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/model/document.js b/src/model/document.js index 57bd65c04..1d376e75e 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -278,6 +278,21 @@ export default class Document { this._postFixers.add( postFixer ); } + /** + * A custom `toJSON()` method to solve child-parent circular dependencies. + * + * @returns {Object} A clone of this object with the document property changed to a string. + */ + toJSON() { + const json = clone( this ); + + // Due to circular references we need to remove parent reference. + json.selection = '[engine.model.DocumentSelection]'; + json.model = '[engine.model.Model]'; + + 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. @@ -289,7 +304,7 @@ export default class Document { * @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixers will be called. */ _runPostFixersAndResetDiffer( writer ) { - if ( this._hasDocumentChangedFromTheLastChangeBlock() ) { + if ( this._hasDocumentChanged() ) { this._callPostFixers( writer ); if ( this.differ.hasDataChanges() ) { @@ -310,27 +325,12 @@ export default class Document { * or {@link module:engine/model/model~Model#change `change()` block}. * * @protected - * @returns {Boolean} Returns `true` when document has changed from the last change block. + * @returns {Boolean} Returns `true` if document has changed from the differ's reset. */ - _hasDocumentChangedFromTheLastChangeBlock() { + _hasDocumentChanged() { return !this.differ.isEmpty || this._hasSelectionChangedFromTheLastChangeBlock; } - /** - * A custom `toJSON()` method to solve child-parent circular dependencies. - * - * @returns {Object} A clone of this object with the document property changed to a string. - */ - toJSON() { - const json = clone( this ); - - // Due to circular references we need to remove parent reference. - json.selection = '[engine.model.DocumentSelection]'; - json.model = '[engine.model.Model]'; - - return json; - } - /** * 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. diff --git a/src/model/model.js b/src/model/model.js index 1351e796a..7cc71ae85 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -696,8 +696,8 @@ export default class Model { const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); ret.push( callbackReturnValue ); - // Collect an information whether the model document has changed during from the last pending change callback. - hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); + // Collect an information whether the model document has changed during from the last pending change. + hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChanged(); // Fire '_change' event before resetting differ. this.fire( '_change', this._currentWriter ); @@ -740,8 +740,8 @@ export default class Model { * @protected * @event _afterChanges * @param {Object} options - * @param {Boolean} options.hasModelDocumentChanged A boolean indicates whether the model document has changed during the - * {@link module:engine/model/model~Model#change} blocks. + * @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. */ /** From 0cb7c9fa40ad24eb84e0a45d29ca5cbd5865ea67 Mon Sep 17 00:00:00 2001 From: Maciej Bukowski Date: Mon, 4 Feb 2019 09:59:26 +0100 Subject: [PATCH 13/13] Renamed methods. API docs fixes. --- src/model/document.js | 8 ++++---- src/model/model.js | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/model/document.js b/src/model/document.js index 1d376e75e..41ecbb5d5 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -303,8 +303,8 @@ export default class Document { * @fires change:data * @param {module:engine/model/writer~Writer writer} writer The writer on which post-fixers will be called. */ - _runPostFixersAndResetDiffer( writer ) { - if ( this._hasDocumentChanged() ) { + _handleChangeBlock( writer ) { + if ( this._hasDocumentChangedFromTheLastChangeBlock() ) { this._callPostFixers( writer ); if ( this.differ.hasDataChanges() ) { @@ -325,9 +325,9 @@ export default class Document { * or {@link module:engine/model/model~Model#change `change()` block}. * * @protected - * @returns {Boolean} Returns `true` if document has changed from the differ's reset. + * @returns {Boolean} Returns `true` if document has changed from the last `change()` or `enqueueChange()` block. */ - _hasDocumentChanged() { + _hasDocumentChangedFromTheLastChangeBlock() { return !this.differ.isEmpty || this._hasSelectionChangedFromTheLastChangeBlock; } diff --git a/src/model/model.js b/src/model/model.js index 7cc71ae85..ca78ab126 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -697,12 +697,12 @@ export default class Model { ret.push( callbackReturnValue ); // Collect an information whether the model document has changed during from the last pending change. - hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChanged(); + hasModelDocumentChanged = hasModelDocumentChanged || this.document._hasDocumentChangedFromTheLastChangeBlock(); // Fire '_change' event before resetting differ. this.fire( '_change', this._currentWriter ); - this.document._runPostFixersAndResetDiffer( this._currentWriter ); + this.document._handleChangeBlock( this._currentWriter ); this._pendingChanges.shift(); this._currentWriter = null;