Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Commit

Permalink
Merge 0cb7c9f into 2e04af7
Browse files Browse the repository at this point in the history
  • Loading branch information
ma2ciek committed Feb 4, 2019
2 parents 2e04af7 + 0cb7c9f commit 41906c9
Show file tree
Hide file tree
Showing 4 changed files with 105 additions and 33 deletions.
6 changes: 4 additions & 2 deletions src/controller/editingcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
84 changes: 55 additions & 29 deletions src/model/document.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -58,23 +58,23 @@ 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;

/**
* The document's history.
*
* @readonly
* @member {module:engine/model/history~History}
* @type {module:engine/model/history~History}
*/
this.history = new History( this );

/**
* The selection in this document.
*
* @readonly
* @member {module:engine/model/documentselection~DocumentSelection}
* @type {module:engine/model/documentselection~DocumentSelection}
*/
this.selection = new DocumentSelection( this );

Expand All @@ -83,26 +83,34 @@ 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' } );

/**
* 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 );

/**
* Post-fixer callbacks registered to the model document.
*
* @private
* @member {Set}
* @type {Set.<Function>}
*/
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 );

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 12 additions & 2 deletions src/model/model.js
Original file line number Diff line number Diff line change
Expand Up @@ -683,6 +683,7 @@ export default class Model {
*/
_runPendingChanges() {
const ret = [];
let hasModelDocumentChanged = false;

this.fire( '_beforeChanges' );

Expand All @@ -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;
}
Expand All @@ -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.
Expand All @@ -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.
*/

/**
Expand Down
34 changes: 34 additions & 0 deletions tests/tickets/1653.js
Original file line number Diff line number Diff line change
@@ -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( '<p></p>' );

sinon.assert.notCalled( spy );
} )
.then( () => {
element.remove();

return editor.destroy();
} );
} );
} );

0 comments on commit 41906c9

Please sign in to comment.