diff --git a/src/controller/editingcontroller.js b/src/controller/editingcontroller.js index 933bcca22..6527bdf24 100644 --- a/src/controller/editingcontroller.js +++ b/src/controller/editingcontroller.js @@ -8,7 +8,6 @@ */ import RootEditableElement from '../view/rooteditableelement'; -import ModelDiffer from '../model/differ'; import ViewDocument from '../view/document'; import Mapper from '../conversion/mapper'; import ModelConversionDispatcher from '../conversion/modelconversiondispatcher'; @@ -84,47 +83,15 @@ export default class EditingController { viewSelection: this.view.selection } ); - // Model differ object. It's role is to buffer changes done on model and then calculates a diff of those changes. - // The diff is then passed to model conversion dispatcher which generates proper events and kicks-off conversion. - const modelDiffer = new ModelDiffer(); - - // Before an operation is applied on model, buffer the change in differ. - this.listenTo( this.model, 'applyOperation', ( evt, args ) => { - const operation = args[ 0 ]; - - if ( operation.isDocumentOperation ) { - modelDiffer.bufferOperation( operation ); - } - }, { priority: 'high' } ); - - // Buffer marker changes. - // This is not covered in buffering operations because markers may change outside of them (when they - // are modified using `model.document.markers` collection, not through `MarkerOperation`). - this.listenTo( this.model.markers, 'add', ( evt, marker ) => { - // Whenever a new marker is added, buffer that change. - modelDiffer.bufferMarkerChange( marker.name, null, marker.getRange() ); - - // Whenever marker changes, buffer that. - marker.on( 'change', ( evt, oldRange ) => { - modelDiffer.bufferMarkerChange( marker.name, oldRange, marker.getRange() ); - } ); - } ); - - this.listenTo( this.model.markers, 'remove', ( evt, marker ) => { - // Whenever marker is removed, buffer that change. - modelDiffer.bufferMarkerChange( marker.name, marker.getRange(), null ); - } ); + const doc = this.model.document; // When all changes are done, get the model diff containing all the changes and convert them to view and then render to DOM. - this.listenTo( this.model, 'changesDone', () => { + this.listenTo( doc, 'change', () => { // Convert changes stored in `modelDiffer`. - this.modelToView.convertChanges( modelDiffer ); - - // Reset model diff object. When next operation is applied, new diff will be created. - modelDiffer.reset(); + this.modelToView.convertChanges( doc.differ ); // After the view is ready, convert selection from model to view. - this.modelToView.convertSelection( this.model.document.selection ); + this.modelToView.convertSelection( doc.selection ); // When everything is converted to the view, render it to DOM. this.view.render(); diff --git a/src/conversion/modelconversiondispatcher.js b/src/conversion/modelconversiondispatcher.js index 1dd5d29e2..90e337e64 100644 --- a/src/conversion/modelconversiondispatcher.js +++ b/src/conversion/modelconversiondispatcher.js @@ -135,11 +135,6 @@ export default class ModelConversionDispatcher { // Convert changes that happened on model tree. for ( const entry of differ.getChanges() ) { - // Skip all the changes that happens in graveyard. These are not converted. - if ( _isInGraveyard( entry ) ) { - continue; - } - if ( entry.type == 'insert' ) { this.convertInsert( Range.createFromPositionAndShift( entry.position, entry.length ) ); } else if ( entry.type == 'remove' ) { @@ -596,9 +591,3 @@ function shouldMarkerChangeBeConverted( modelPosition, marker, mapper ) { return !hasCustomHandling; } - -// Checks whether entry change describes changes that happen in graveyard. -function _isInGraveyard( entry ) { - return ( entry.position && entry.position.root.rootName == '$graveyard' ) || - ( entry.range && entry.range.root.rootName == '$graveyard' ); -} diff --git a/src/dev-utils/enableenginedebug.js b/src/dev-utils/enableenginedebug.js index 408e10bcd..be878ec0b 100644 --- a/src/dev-utils/enableenginedebug.js +++ b/src/dev-utils/enableenginedebug.js @@ -656,7 +656,8 @@ class DebugPlugin extends Plugin { constructor( editor ) { super( editor ); - const modelDocument = this.editor.model.document; + const model = this.editor.model; + const modelDocument = model.document; const viewDocument = this.editor.editing.view; modelDocument[ treeDump ] = []; @@ -665,11 +666,11 @@ class DebugPlugin extends Plugin { dumpTrees( modelDocument, modelDocument.version ); dumpTrees( viewDocument, modelDocument.version ); - modelDocument.on( 'change', () => { + model.on( 'applyOperation', () => { dumpTrees( modelDocument, modelDocument.version ); }, { priority: 'lowest' } ); - modelDocument.on( 'changesDone', () => { + model.document.on( 'change', () => { dumpTrees( viewDocument, modelDocument.version ); }, { priority: 'lowest' } ); } diff --git a/src/model/differ.js b/src/model/differ.js index 29ad3ea4d..921c6f8e2 100644 --- a/src/model/differ.js +++ b/src/model/differ.js @@ -61,6 +61,40 @@ export default class Differ { * @type {Number} */ this._changeCount = 0; + + /** + * For efficiency purposes, `Differ` stores the change set returned by the differ after {@link #getChanges} call. + * Cache is reset each time a new operation is buffered. If the cache has not been reset, {@link #getChanges} will + * return the cached value instead of calculating it again. + * + * This property stores those changes that did not take place in graveyard root. + * + * @private + * @type {Array.|null} + */ + this._cachedChanges = null; + + /** + * For efficiency purposes, `Differ` stores the change set returned by the differ after {@link #getChanges} call. + * Cache is reset each time a new operation is buffered. If the cache has not been reset, {@link #getChanges} will + * return the cached value instead of calculating it again. + * + * This property stores all changes evaluated by `Differ`, also those that took place in graveyard. + * + * @private + * @type {Array.|null} + */ + this._cachedChangesWithGraveyard = null; + } + + /** + * Informs whether there are any changes buffered in `Differ`. + * + * @readonly + * @type {Boolean} + */ + get isEmpty() { + return this._changesInElement.size == 0 && this._changedMarkers.size == 0; } /** @@ -98,6 +132,9 @@ export default class Differ { break; } + + // Clear cache after each buffered operation as it is no longer valid. + this._cachedChanges = null; } /** @@ -168,9 +205,24 @@ export default class Differ { * the position on which the change happened. If a position {@link module:engine/model/position~Position#isBefore is before} * another one, it will be on an earlier index in the diff set. * + * Because calculating diff is a costly operation, the result is cached. If no new operation was buffered since the + * previous {@link #getChanges} call, the next call with return the cached value. + * + * @param {Object} options Additional options. + * @param {Boolean} [options.includeChangesInGraveyard=false] If set to `true`, also changes that happened + * in graveyard root will be returned. By default, changes in graveyard root are not returned. * @returns {Array.} Diff between old and new model tree state. */ - getChanges() { + getChanges( options = { includeChangesInGraveyard: false } ) { + // If there are cached changes, just return them instead of calculating changes again. + if ( this._cachedChanges ) { + if ( options.includeChangesInGraveyard ) { + return this._cachedChangesWithGraveyard.slice(); + } else { + return this._cachedChanges.slice(); + } + } + // Will contain returned results. const diffSet = []; @@ -320,7 +372,15 @@ export default class Differ { this._changeCount = 0; - return diffSet; + // Cache changes. + this._cachedChangesWithGraveyard = diffSet.slice(); + this._cachedChanges = diffSet.slice().filter( _changesInGraveyardFilter ); + + if ( options.includeChangesInGraveyard ) { + return this._cachedChangesWithGraveyard; + } else { + return this._cachedChanges; + } } /** @@ -330,6 +390,7 @@ export default class Differ { this._changesInElement.clear(); this._elementSnapshots.clear(); this._changedMarkers.clear(); + this._cachedChanges = null; } /** @@ -865,3 +926,11 @@ function _generateActionsFromChanges( oldChildrenLength, changes ) { return actions; } + +// Filter callback for Array.filter that filters out change entries that are in graveyard. +function _changesInGraveyardFilter( entry ) { + const posInGy = entry.position && entry.position.root.rootName == '$graveyard'; + const rangeInGy = entry.range && entry.range.root.rootName == '$graveyard'; + + return !posInGy && !rangeInGy; +} diff --git a/src/model/document.js b/src/model/document.js index 487f92893..16695b1b2 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -7,6 +7,7 @@ * @module engine/model/document */ +import Differ from './differ'; import Range from './range'; import Position from './position'; import RootElement from './rootelement'; @@ -81,28 +82,26 @@ export default class Document { */ this.roots = new Collection( { idProperty: 'rootName' } ); - // Add events that will ensure selection correctness. - this.selection.on( 'change:range', () => { - for ( const range of this.selection.getRanges() ) { - if ( !this._validateSelectionRange( range ) ) { - /** - * Range from {@link module:engine/model/documentselection~DocumentSelection document selection} - * starts or ends at incorrect position. - * - * @error document-selection-wrong-position - * @param {module:engine/model/range~Range} range - */ - throw new CKEditorError( - 'document-selection-wrong-position: Range from document selection starts or ends at incorrect position.', - { range } - ); - } - } - } ); + /** + * Model differ object. Its role is to buffer changes done on model document and then calculate a diff of those changes. + * + * @readonly + * @member {module:engine/model/differ~Differ} + */ + this.differ = new Differ(); + + /** + * Post-fixer callbacks registered to the model document. + * + * @private + * @member {Set} + */ + this._postFixers = new Set(); // Graveyard tree root. Document always have a graveyard root, which stores removed nodes. this.createRoot( '$root', graveyardName ); + // First, if the operation is a document operation check if it's base version is correct. this.listenTo( model, 'applyOperation', ( evt, args ) => { const operation = args[ 0 ]; @@ -118,22 +117,67 @@ export default class Document { { operation } ); } - - operation._validate(); }, { priority: 'highest' } ); + // Then, still before an operation is applied on model, buffer the change in differ. + this.listenTo( model, 'applyOperation', ( evt, args ) => { + const operation = args[ 0 ]; + + if ( operation.isDocumentOperation ) { + this.differ.bufferOperation( operation ); + } + }, { priority: 'high' } ); + + // After the operation is applied, bump document's version and add the operation to the history. this.listenTo( model, 'applyOperation', ( evt, args ) => { const operation = args[ 0 ]; if ( operation.isDocumentOperation ) { this.version++; this.history.addDelta( operation.delta ); - this.fire( 'change', operation.type, evt.return, operation.delta.batch, operation.delta.type ); } }, { priority: 'low' } ); - // Temporary compatibility. - model.delegate( 'changesDone' ).to( this ); + // 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. + this.listenTo( model, '_change', ( evt, writer ) => { + if ( !this.differ.isEmpty || hasSelectionChanged ) { + this._callPostFixers( writer ); + + this.fire( 'change', writer.batch ); + + this.differ.reset(); + hasSelectionChanged = false; + } + } ); + + // Buffer marker changes. + // This is not covered in buffering operations because markers may change outside of them (when they + // are modified using `model.markers` collection, not through `MarkerOperation`). + this.listenTo( model.markers, 'add', ( evt, marker ) => { + // TODO: Should filter out changes of markers that are not in document. + // Whenever a new marker is added, buffer that change. + this.differ.bufferMarkerChange( marker.name, null, marker.getRange() ); + + // Whenever marker changes, buffer that. + marker.on( 'change', ( evt, oldRange ) => { + this.differ.bufferMarkerChange( marker.name, oldRange, marker.getRange() ); + } ); + } ); + + this.listenTo( model.markers, 'remove', ( evt, marker ) => { + // TODO: Should filter out changes of markers that are not in document. + // Whenever marker is removed, buffer that change. + this.differ.bufferMarkerChange( marker.name, marker.getRange(), null ); + } ); } /** @@ -257,6 +301,44 @@ export default class Document { return null; } + /** + * Used to register a post-fixer callback. Post-fixers mechanism guarantees that the features that listen to + * {@link module:engine/model/model~Model#event:_change model's change event} will operate on a correct model state. + * + * Execution of a feature may lead to an incorrect document tree state. The callbacks are used to fix document tree after + * it has changed. Post-fixers are fired just after all changes from the outermost change block were applied but + * before {@link module:engine/model/document~Document#event:change} is fired. If a post-fixer callback made a change, + * it should return `true`. When this happens, all post-fixers are fired again to check if something else should + * not be fixed in the new document tree state. + * + * As a parameter, a post-fixer callback receives {@link module:engine/model/writer~Writer} instance connected with the executed + * changes block. Thanks to that, all changes done by the callback will be added to the same {@link module:engine/model/batch~Batch} + * (and undo step) as the original changes. This makes post-fixer changes transparent for the user. + * + * An example of a post-fixer is a callback that checks if all the data was removed from the editor. If so, the + * callback should add an empty paragraph, so that the editor is never empty: + * + * document.registerPostFixer( writer => { + * const changes = document.differ.getChanges(); + * + * // Check if the changes lead to an empty root in an editor. + * for ( const entry of changes ) { + * if ( entry.type == 'remove' && entry.position.root.isEmpty ) { + * writer.insertElement( 'paragraph', entry.position.root, 0 ); + * + * // It is fine to return early, even if multiple roots would need to be fixed. + * // All post-fixers will be fired again, so if there more empty roots, those will be fixed too. + * return true; + * } + * } + * } ); + * + * @param {Function} postFixer + */ + registerPostFixer( postFixer ) { + this._postFixers.add( postFixer ); + } + /** * Custom toJSON method to solve child-parent circular dependencies. * @@ -320,48 +402,31 @@ export default class Document { } /** - * Fired when document changes by applying an operation. - * - * There are several types of change: + * Performs post-fixer loops. Executes post-fixer callbacks as long as neither of them has done any changes to model. * - * * 'insert' when nodes are inserted, - * * 'remove' when nodes are removed, - * * 'reinsert' when remove is undone, - * * 'move' when nodes are moved, - * * 'rename' when element is renamed, - * * 'marker' when a marker changes (added, removed or its range is changed), - * * 'addAttribute' when attributes are added, - * * 'removeAttribute' when attributes are removed, - * * 'changeAttribute' when attributes change, - * * 'addRootAttribute' when attribute for root is added, - * * 'removeRootAttribute' when attribute for root is removed, - * * 'changeRootAttribute' when attribute for root changes. - * - * @event change - * @param {String} type Change type. - * @param {Object} data Additional information about the change. - * @param {module:engine/model/range~Range} [data.range] Range in model containing changed nodes. Note that the range state is - * after changes has been done, i.e. for 'remove' the range will be in the {@link #graveyard graveyard root}. - * The range is not defined for root, rename and marker types. - * @param {module:engine/model/position~Position} [data.sourcePosition] Change source position. - * Exists for 'remove', 'reinsert' and 'move'. - * Note that this position state is before changes has been done, i.e. for 'reinsert' the source position will be in the - * {@link #graveyard graveyard root}. - * @param {String} [data.key] Only for attribute types. Key of changed / inserted / removed attribute. - * @param {*} [data.oldValue] Only for 'removeAttribute', 'removeRootAttribute', 'changeAttribute' or - * 'changeRootAttribute' type. - * @param {*} [data.newValue] Only for 'addAttribute', 'addRootAttribute', 'changeAttribute' or - * 'changeRootAttribute' type. - * @param {module:engine/model/rootelement~RootElement} [data.root] Root element which attributes got changed. This is defined - * only for root types. - * @param {module:engine/model/batch~Batch} batch A {@link module:engine/model/batch~Batch batch} - * of changes which this change is a part of. + * @private */ + _callPostFixers( writer ) { + let wasFixed = false; + + do { + for ( const callback of this._postFixers ) { + wasFixed = callback( writer ); + + if ( wasFixed ) { + break; + } + } + } while ( wasFixed ); + } /** - * Fired when all queued document changes are done. See {@link module:engine/model/model~Model#change}. + * Fired after an {@link module:engine/model/model~Model#enqueueChange enqueueChange block} or the outermost + * {@link module:engine/model/model~Model#change change block} has been executed and the document model tree was changed + * during that block execution. * - * @event changesDone + * @event change + * @param {@link module:engine/model/batch~Batch} batch Batch which was used in the executed changes block. */ } diff --git a/src/model/documentselection.js b/src/model/documentselection.js index b369a7fa4..5c20f140a 100644 --- a/src/model/documentselection.js +++ b/src/model/documentselection.js @@ -84,22 +84,48 @@ export default class DocumentSelection extends Selection { */ this._attributePriority = new Map(); - this.listenTo( this._document, 'change', ( evt, type, changes, batch ) => { + // Add events that will ensure selection correctness. + this.on( 'change:range', () => { + for ( const range of this.getRanges() ) { + if ( !this._document._validateSelectionRange( range ) ) { + /** + * Range from {@link module:engine/model/documentselection~DocumentSelection document selection} + * starts or ends at incorrect position. + * + * @error document-selection-wrong-position + * @param {module:engine/model/range~Range} range + */ + throw new CKEditorError( + 'document-selection-wrong-position: Range from document selection starts or ends at incorrect position.', + { range } + ); + } + } + } ); + + this.listenTo( this._model, 'applyOperation', ( evt, args ) => { + const operation = args[ 0 ]; + + if ( !operation.isDocumentOperation ) { + return; + } + // Whenever attribute operation is performed on document, update selection attributes. // This is not the most efficient way to update selection attributes, but should be okay for now. - if ( attrOpTypes.has( type ) ) { + if ( attrOpTypes.has( operation.type ) ) { this._updateAttributes( false ); } + const batch = operation.delta.batch; + // Batch may not be passed to the document#change event in some tests. // See https://github.com/ckeditor/ckeditor5-engine/issues/1001#issuecomment-314202352 - // Ignore also transparent batches because they are... transparent. - if ( batch && batch.type !== 'transparent' ) { + if ( batch ) { // Whenever element which had selection's attributes stored in it stops being empty, // the attributes need to be removed. - clearAttributesStoredInElement( changes, this._model, batch ); + clearAttributesStoredInElement( operation, this._model, batch ); } - } ); + }, { priority: 'low' } ); } /** @@ -710,10 +736,6 @@ export default class DocumentSelection extends Selection { } } -/** - * @event change:attribute - */ - // Helper function for {@link module:engine/model/documentselection~DocumentSelection#_updateAttributes}. // // It takes model item, checks whether it is a text node (or text proxy) and, if so, returns it's attributes. If not, returns `null`. @@ -729,11 +751,15 @@ function getAttrsIfCharacter( node ) { } // Removes selection attributes from element which is not empty anymore. -function clearAttributesStoredInElement( changes, model, batch ) { - const changeParent = changes.range && changes.range.start.parent; +function clearAttributesStoredInElement( operation, model, batch ) { + let changeParent = null; + + if ( operation.type == 'insert' ) { + changeParent = operation.position.parent; + } else if ( operation.type == 'move' || operation.type == 'reinsert' || operation.type == 'remove' ) { + changeParent = operation.getMovedRangeStart().parent; + } - // `changes.range` is not set in case of rename, root and marker operations. - // None of them may lead to the element becoming non-empty. if ( !changeParent || changeParent.isEmpty ) { return; } diff --git a/src/model/liveposition.js b/src/model/liveposition.js index 8ae2799ac..f80488672 100644 --- a/src/model/liveposition.js +++ b/src/model/liveposition.js @@ -140,14 +140,20 @@ function bindWithDocument() { const supportedTypes = new Set( [ 'insert', 'move', 'remove', 'reinsert' ] ); this.listenTo( - this.root.document, - 'change', - ( event, type, changes ) => { - if ( supportedTypes.has( type ) ) { - transform.call( this, type, changes.range, changes.sourcePosition ); + this.root.document.model, + 'applyOperation', + ( event, args ) => { + const operation = args[ 0 ]; + + if ( !operation.isDocumentOperation ) { + return; + } + + if ( supportedTypes.has( operation.type ) ) { + transform.call( this, operation ); } }, - { priority: 'high' } + { priority: 'low' } ); } @@ -157,16 +163,24 @@ function bindWithDocument() { * @ignore * @private * @method transform - * @param {String} type Type of changes applied to the Tree Model. - * @param {module:engine/model/range~Range} range Range containing the result of applied change. - * @param {module:engine/model/position~Position} [position] Additional position parameter provided by some change events. + * @param {module:engine/model/operation/operation~Operation} operation Executed operation. */ -function transform( type, range, position ) { +function transform( operation ) { /* eslint-disable no-case-declarations */ + let range; + let position; + + if ( operation.type == 'insert' ) { + range = Range.createFromPositionAndShift( operation.position, operation.nodes.maxOffset ); + } else { + range = Range.createFromPositionAndShift( operation.getMovedRangeStart(), operation.howMany ); + position = operation.sourcePosition; + } + const howMany = range.end.offset - range.start.offset; let transformed; - switch ( type ) { + switch ( operation.type ) { case 'insert': const insertBefore = this.stickiness == 'sticksToNext'; transformed = this._getTransformedByInsertion( range.start, howMany, insertBefore ); @@ -186,7 +200,13 @@ function transform( type, range, position ) { transformed = this._getCombined( position, range.start ); } else { const insertBefore = this.stickiness == 'sticksToNext'; - transformed = this._getTransformedByMove( position, range.start, howMany, insertBefore ); + + // `Position._getTransformedByMove` is expecting `targetPosition` to be "before" move + // (before transformation). `range.start` is already after the move happened. + // We have to revert `targetPosition` to the state before the move. + const targetPosition = range.start._getTransformedByInsertion( position, howMany ); + + transformed = this._getTransformedByMove( position, targetPosition, howMany, insertBefore ); } break; } diff --git a/src/model/liverange.js b/src/model/liverange.js index 2d56f2c6c..b12669b4a 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -120,14 +120,20 @@ function bindWithDocument() { const supportedTypes = new Set( [ 'insert', 'move', 'remove', 'reinsert' ] ); this.listenTo( - this.root.document, - 'change', - ( event, type, changes, batch, deltaType ) => { - if ( supportedTypes.has( type ) ) { - transform.call( this, type, deltaType, batch, changes.range, changes.sourcePosition ); + this.root.document.model, + 'applyOperation', + ( event, args ) => { + const operation = args[ 0 ]; + + if ( !operation.isDocumentOperation ) { + return; + } + + if ( supportedTypes.has( operation.type ) ) { + transform.call( this, operation ); } }, - { priority: 'high' } + { priority: 'low' } ); } @@ -137,13 +143,22 @@ function bindWithDocument() { * @ignore * @private * @method transform - * @param {String} [changeType] Type of change applied to the model document. - * @param {String} [deltaType] Type of delta which introduced the change. - * @param {module:engine/model/batch~Batch} batch Batch which changes the live range. - * @param {module:engine/model/range~Range} targetRange Range containing the result of applied change. - * @param {module:engine/model/position~Position} [sourcePosition] Source position for move, remove and reinsert change types. + * @param {module:engine/model/operation/operation~Operation} operation Executed operation. */ -function transform( changeType, deltaType, batch, targetRange, sourcePosition ) { +function transform( operation ) { + const changeType = operation.type; + const batch = operation.delta.batch; + + let targetRange; + let sourcePosition; + + if ( changeType == 'insert' ) { + targetRange = Range.createFromPositionAndShift( operation.position, operation.nodes.maxOffset ); + } else { + targetRange = Range.createFromPositionAndShift( operation.getMovedRangeStart(), operation.howMany ); + sourcePosition = operation.sourcePosition; + } + const howMany = targetRange.end.offset - targetRange.start.offset; let targetPosition = targetRange.start; @@ -154,7 +169,7 @@ function transform( changeType, deltaType, batch, targetRange, sourcePosition ) targetPosition = targetPosition._getTransformedByInsertion( sourcePosition, howMany ); } - const result = this._getTransformedByDocumentChange( changeType, deltaType, targetPosition, howMany, sourcePosition ); + const result = this._getTransformedByDocumentChange( changeType, operation.delta.type, targetPosition, howMany, sourcePosition ); // Decide whether moved part should be included in the range. // diff --git a/src/model/model.js b/src/model/model.js index c0296c88d..5655415b4 100644 --- a/src/model/model.js +++ b/src/model/model.js @@ -28,10 +28,12 @@ import modifySelection from './utils/modifyselection'; import getSelectedContent from './utils/getselectedcontent'; /** - * Editors data model class. Model defines all data: either nodes users see in editable roots, grouped as the - * {@link module:engine/model/model~Model#document}, and all detached nodes, used to data manipulation. All of them are - * created and modified by the {@link module:engine/model/writer~Writer}, which can be get using - * {@link module:engine/model/model~Model#change} or {@link module:engine/model/model~Model#enqueueChange} methods. + * Editor's data model class. Model defines all the data: both nodes that are attached to the roots of the + * {@link module:engine/model/model~Model#document model document}, and also all detached nodes which has not been yet + * added to the document. + * + * All those nodes are created and modified by the {@link module:engine/model/writer~Writer model writer}, which can be + * accessed by using {@link module:engine/model/model~Model#change} or {@link module:engine/model/model~Model#enqueueChange} methods. * * @mixes module:utils/observablemixin~ObservableMixin */ @@ -46,6 +48,14 @@ export default class Model { */ this._pendingChanges = []; + /** + * Models markers' collection. + * + * @readonly + * @member {module:engine/model/markercollection~MarkerCollection} + */ + this.markers = new MarkerCollection(); + /** * Editors document model. * @@ -54,23 +64,31 @@ export default class Model { this.document = new Document( this ); /** - * Schema for editors model. + * The last created and currently used writer instance. * - * @member {module:engine/model/schema~Schema} + * @private + * @member {module:engine/model/writer~Writer} */ - this.schema = new Schema(); + this._currentWriter = null; /** - * Models markers' collection. + * Schema for editors model. * - * @readonly - * @member {module:engine/model/markercollection~MarkerCollection} + * @member {module:engine/model/schema~Schema} */ - this.markers = new MarkerCollection(); + this.schema = new Schema(); [ 'insertContent', 'deleteContent', 'modifySelection', 'getSelectedContent', 'applyOperation' ] .forEach( methodName => this.decorate( methodName ) ); + // Adding operation validation with `highest` priority, so it is called before any other feature would like + // to do anything with the operation. If the operation has incorrect parameters it should throw on the earliest occasion. + this.on( 'applyOperation', ( evt, args ) => { + const operation = args[ 0 ]; + + operation._validate(); + }, { priority: 'highest' } ); + // Register some default abstract entities. this.schema.register( '$root', { isLimit: true @@ -91,26 +109,26 @@ export default class Model { /** * Change method is the primary way of changing the model. You should use it to modify any node, including detached - * nodes, not added to the {@link module:engine/model/model~Model#document}. + * nodes (not added to the {@link module:engine/model/model~Model#document model document}). * * model.change( writer => { * writer.insertText( 'foo', paragraph, 'end' ); * } ); * - * All changes inside the change block use the same {@link module:engine/model/batch~Batch} so share the same + * All changes inside the change block use the same {@link module:engine/model/batch~Batch} so they share the same * undo step. * * model.change( writer => { - * writer.insertText( 'foo', paragraph, 'end' ); // foo + * writer.insertText( 'foo', paragraph, 'end' ); // foo. * * model.change( writer => { - * writer.insertText( 'bar', paragraph, 'end' ); // foobar + * writer.insertText( 'bar', paragraph, 'end' ); // foobar. * } ); * - * writer.insertText( 'bom', paragraph, 'end' ); // foobarbom + * writer.insertText( 'bom', paragraph, 'end' ); // foobarbom. * } ); * - * Change block is executed imminently. + * Change block is executed immediately. * * You can also return a value from the change block. * @@ -118,43 +136,43 @@ export default class Model { * return writer.createElement( 'img' ); * } ); * - * When the outermost block is done the {@link #event:change} event is fired. + * When the outermost block is done the {@link #event:_change} event is fired. * * @see #enqueueChange - * @fires event:change - * @fires event:changesDone * @param {Function} callback Callback function which may modify the model. - * @returns {*} Value returned by the callback + * @returns {*} Value returned by the callback. */ change( callback ) { if ( this._pendingChanges.length === 0 ) { + // If this is the outermost block, create a new batch and start `_runPendingChanges` execution flow. this._pendingChanges.push( { batch: new Batch(), callback } ); return this._runPendingChanges()[ 0 ]; } else { + // If this is not the outermost block, just execute the callback. return callback( this._currentWriter ); } } /** - * `enqueueChange` method is very similar to the {@link #change change method}, with two major differences. + * `enqueueChange` method performs similar task as the {@link #change change method}, with two major differences. * * First, the callback of the `enqueueChange` is executed when all other changes are done. It might be executed - * imminently if it is not nested in any other change block, but if it is nested in another change it will be delayed - * and executed after the outermost block. If will be also executed after all previous `enqueueChange` blocks. + * immediately if it is not nested in any other change block, but if it is nested in another (enqueue)change block, + * it will be delayed and executed after the outermost block. * * model.change( writer => { * console.log( 1 ); * * model.enqueueChange( writer => { - * console.log( 3 ); + * console.log( 2 ); * } ); * - * console.log( 2 ); - * } ); + * console.log( 3 ); + * } ); // Will log: 1, 3, 2. * - * Second, it let you define the {@link module:engine/model/batch~Batch} to which you want to add your changes. - * By default it creates a new batch, note that in the sample above `change` and `enqueueChange` blocks use a different + * Second, it lets you define the {@link module:engine/model/batch~Batch} into which you want to add your changes. + * By default, a new batch is created. In the sample above, `change` and `enqueueChange` blocks use a different * batch (and different {@link module:engine/model/writer~Writer} since each of them operates on the separate batch). * * Using `enqueueChange` block you can also add some changes to the batch you used before. @@ -163,10 +181,10 @@ export default class Model { * writer.insertText( 'foo', paragraph, 'end' ); * } ); * - * @fires event:change - * @fires event:changesDone + * `Batch` instance can be obtained from {@link module:engine/model/writer~Writer#batch the writer}. + * * @param {module:engine/model/batch~Batch|String} batchOrType Batch or batch type should be used in the callback. - * If not defined new batch will be created. + * If not defined, a new batch will be created. * @param {Function} callback Callback function which may modify the model. */ enqueueChange( batchOrType, callback ) { @@ -195,19 +213,21 @@ export default class Model { const ret = []; while ( this._pendingChanges.length ) { - this._currentWriter = new Writer( this, this._pendingChanges[ 0 ].batch ); + // Create a new writer using batch instance created for this chain of changes. + const currentBatch = this._pendingChanges[ 0 ].batch; + this._currentWriter = new Writer( this, currentBatch ); - ret.push( this._pendingChanges[ 0 ].callback( this._currentWriter ) ); + // Execute changes callback and gather the returned value. + const callbackReturnValue = this._pendingChanges[ 0 ].callback( this._currentWriter ); + ret.push( callbackReturnValue ); - this.fire( 'change' ); + // Fire internal `_change` event. + this.fire( '_change', this._currentWriter ); this._pendingChanges.shift(); - this._currentWriter = null; } - this.fire( 'changesDone' ); - return ret; } @@ -216,11 +236,9 @@ export default class Model { * {@link module:engine/model/operation/operation~Operation operations} on the model. * * @param {module:engine/model/operation/operation~Operation} operation Operation to apply - * @returns {Object} Object with additional information about the applied changes. It properties depends on the - * operation type. */ applyOperation( operation ) { - return operation._execute(); + operation._execute(); } /** @@ -324,89 +342,84 @@ export default class Model { } /** - * Removes all events listeners set by model instance and destroy Document. + * Removes all events listeners set by model instance and destroys {@link module:engine/model/document~Document}. */ destroy() { this.document.destroy(); this.stopListening(); } -} -mix( Model, ObservableMixin ); - -/** - * Fired after leaving each {@link module:engine/model/model~Model#enqueueChange} block or outermost - * {@link module:engine/model/model~Model#change} block. - * Have the same parameters as {@link module:engine/model/model~Model#change}. - * - * @event change - */ + /** + * Fired after leaving each {@link module:engine/model/model~Model#enqueueChange} block or outermost + * {@link module:engine/model/model~Model#change} block. + * + * **Note:** This is an internal event! Use {@link module:engine/model/document~Document#event:change} instead. + * + * @protected + * @event _change + * @param {module:engine/model/writer~Writer} writer `Writer` instance that has been used in the change block. + */ -/** - * Fired when all queued model changes are done. - * - * @see #change - * @see #enqueueChange - * @event changesDone - */ + /** + * Fired every time any {@link module:engine/model/operation/operation~Operation operation} is applied on the model + * using {@link #applyOperation}. + * + * Note that this event is suitable only for very specific use-cases. Use it if you need to listen to every single operation + * applied on the document. However, in most cases {@link module:engine/model/document~Document#event:change} should + * be used. + * + * A few callbacks are already added to this event by engine internal classes: + * + * * with `highest` priority operation is validated, + * * with `normal` priority operation is executed, + * * with `low` priority the {@link module:engine/model/document~Document} updates its version, + * * with `low` priority {@link module:engine/model/liveposition~LivePosition} and {@link module:engine/model/liverange~LiveRange} + * update themselves. + * + * @event applyOperation + * @param {Array} args Arguments of the `applyOperation` which is an array with a single element - applied + * {@link module:engine/model/operation/operation~Operation operation}. + */ -/** - * Fired every time any {@link module:engine/model/operation/operation~Operation operation} is applied on the model - * using {@link #applyOperation}. - * - * Note that this is an internal event for the specific use-cases. You can use it if you need to know about each operation - * applied on the document, but in most cases {@link #change} event which is fired when all changes in a - * {@link module:engine/model/batch~Batch} are applied, is a better choice. - * - * With the high priority operation is validated. - * - * With the normal priority operation is executed. After that priority you will be able to get additional - * information about the applied changes returned by {@link module:engine/model/operation/operation~Operation#_execute} - * as `evt.return`. - * - * With the low priority the {@link module:engine/model/document~Document} listen on this event and updates its version. - * - * @event applyOperation - * @param {Array} args Arguments of the `applyOperation` which are an array with a single element: - * {@link module:engine/model/operation/operation~Operation operation}. - */ + /** + * Event fired when {@link #insertContent} method is called. + * + * The {@link #insertContent default action of that method} is implemented as a + * listener to this event so it can be fully customized by the features. + * + * @event insertContent + * @param {Array} args The arguments passed to the original method. + */ -/** - * Event fired when {@link #insertContent} method is called. - * - * The {@link #insertContent default action of that method} is implemented as a - * listener to this event so it can be fully customized by the features. - * - * @event insertContent - * @param {Array} args The arguments passed to the original method. - */ + /** + * Event fired when {@link #deleteContent} method is called. + * + * The {@link #deleteContent default action of that method} is implemented as a + * listener to this event so it can be fully customized by the features. + * + * @event deleteContent + * @param {Array} args The arguments passed to the original method. + */ -/** - * Event fired when {@link #deleteContent} method is called. - * - * The {@link #deleteContent default action of that method} is implemented as a - * listener to this event so it can be fully customized by the features. - * - * @event deleteContent - * @param {Array} args The arguments passed to the original method. - */ + /** + * Event fired when {@link #modifySelection} method is called. + * + * The {@link #modifySelection default action of that method} is implemented as a + * listener to this event so it can be fully customized by the features. + * + * @event modifySelection + * @param {Array} args The arguments passed to the original method. + */ -/** - * Event fired when {@link #modifySelection} method is called. - * - * The {@link #modifySelection default action of that method} is implemented as a - * listener to this event so it can be fully customized by the features. - * - * @event modifySelection - * @param {Array} args The arguments passed to the original method. - */ + /** + * Event fired when {@link #getSelectedContent} method is called. + * + * The {@link #getSelectedContent default action of that method} is implemented as a + * listener to this event so it can be fully customized by the features. + * + * @event getSelectedContent + * @param {Array} args The arguments passed to the original method. + */ +} -/** - * Event fired when {@link #getSelectedContent} method is called. - * - * The {@link #getSelectedContent default action of that method} is implemented as a - * listener to this event so it can be fully customized by the features. - * - * @event getSelectedContent - * @param {Array} args The arguments passed to the original method. - */ +mix( Model, ObservableMixin ); diff --git a/src/model/operation/attributeoperation.js b/src/model/operation/attributeoperation.js index 83a397a4a..f98bb3e41 100644 --- a/src/model/operation/attributeoperation.js +++ b/src/model/operation/attributeoperation.js @@ -157,8 +157,6 @@ export default class AttributeOperation extends Operation { // Execution. _setAttribute( this.range, this.key, this.newValue ); } - - return { range: this.range, key: this.key, oldValue: this.oldValue, newValue: this.newValue }; } /** diff --git a/src/model/operation/detachoperation.js b/src/model/operation/detachoperation.js index 0432d695f..e572dc1b9 100644 --- a/src/model/operation/detachoperation.js +++ b/src/model/operation/detachoperation.js @@ -78,9 +78,7 @@ export default class DetachOperation extends Operation { * @inheritDoc */ _execute() { - const nodes = _remove( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ) ); - - return { nodes }; + _remove( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ) ); } /** diff --git a/src/model/operation/insertoperation.js b/src/model/operation/insertoperation.js index 4b7ffc825..821485efd 100644 --- a/src/model/operation/insertoperation.js +++ b/src/model/operation/insertoperation.js @@ -113,9 +113,7 @@ export default class InsertOperation extends Operation { const originalNodes = this.nodes; this.nodes = new NodeList( [ ...originalNodes ].map( node => node.clone( true ) ) ); - const range = _insert( this.position, originalNodes ); - - return { range }; + _insert( this.position, originalNodes ); } /** diff --git a/src/model/operation/markeroperation.js b/src/model/operation/markeroperation.js index 65b8487ce..9d0131c8f 100644 --- a/src/model/operation/markeroperation.js +++ b/src/model/operation/markeroperation.js @@ -112,8 +112,6 @@ export default class MarkerOperation extends Operation { const type = this.newRange ? 'set' : 'remove'; this._markers[ type ]( this.name, this.newRange ); - - return { name: this.name, type }; } /** diff --git a/src/model/operation/moveoperation.js b/src/model/operation/moveoperation.js index 7d6c8cd19..85b0211bf 100644 --- a/src/model/operation/moveoperation.js +++ b/src/model/operation/moveoperation.js @@ -189,12 +189,7 @@ export default class MoveOperation extends Operation { * @inheritDoc */ _execute() { - const range = _move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition ); - - return { - sourcePosition: this.sourcePosition, - range - }; + _move( Range.createFromPositionAndShift( this.sourcePosition, this.howMany ), this.targetPosition ); } /** diff --git a/src/model/operation/nooperation.js b/src/model/operation/nooperation.js index 11f9504fe..0f79d252b 100644 --- a/src/model/operation/nooperation.js +++ b/src/model/operation/nooperation.js @@ -54,11 +54,7 @@ export default class NoOperation extends Operation { return new NoOperation( this.baseVersion + 1 ); } - /** - * @inheritDoc - */ _execute() { - return {}; } /** diff --git a/src/model/operation/operation.js b/src/model/operation/operation.js index 3fa041423..ef35ba629 100644 --- a/src/model/operation/operation.js +++ b/src/model/operation/operation.js @@ -73,12 +73,10 @@ export default class Operation { */ /** - * Executes the operation - modifications described by the operation attributes will be applied to the tree model. + * Executes the operation - modifications described by the operation properties will be applied to the model tree. * * @protected * @method #_execute - * @returns {Object} Object with additional information about the applied changes. It properties depends on the - * operation type. */ } diff --git a/src/model/operation/reinsertoperation.js b/src/model/operation/reinsertoperation.js index 4b8200b44..b037945ba 100644 --- a/src/model/operation/reinsertoperation.js +++ b/src/model/operation/reinsertoperation.js @@ -82,13 +82,6 @@ export default class ReinsertOperation extends MoveOperation { } } - /** - * @inheritDoc - */ - _execute() { - return super._execute(); - } - /** * @inheritDoc */ diff --git a/src/model/operation/removeoperation.js b/src/model/operation/removeoperation.js index 944fa0a68..966aa43ee 100644 --- a/src/model/operation/removeoperation.js +++ b/src/model/operation/removeoperation.js @@ -67,13 +67,6 @@ export default class RemoveOperation extends MoveOperation { } } - /** - * @inheritDoc - */ - _execute() { - return super._execute(); - } - /** * @inheritDoc */ diff --git a/src/model/operation/renameoperation.js b/src/model/operation/renameoperation.js index a381d81d5..a37ed292d 100644 --- a/src/model/operation/renameoperation.js +++ b/src/model/operation/renameoperation.js @@ -117,8 +117,6 @@ export default class RenameOperation extends Operation { const element = this.position.nodeAfter; element.name = this.newName; - - return { element, oldName: this.oldName }; } /** diff --git a/src/model/operation/rootattributeoperation.js b/src/model/operation/rootattributeoperation.js index 5808d36ef..1975e7da6 100644 --- a/src/model/operation/rootattributeoperation.js +++ b/src/model/operation/rootattributeoperation.js @@ -149,8 +149,6 @@ export default class RootAttributeOperation extends Operation { } else { this.root.removeAttribute( this.key ); } - - return { root: this.root, key: this.key, oldValue: this.oldValue, newValue: this.newValue }; } /** diff --git a/src/model/schema.js b/src/model/schema.js index 9311e2c93..482d312eb 100644 --- a/src/model/schema.js +++ b/src/model/schema.js @@ -871,12 +871,21 @@ export class SchemaContext { */ constructor( context ) { if ( Array.isArray( context ) ) { - this._items = context.map( mapContextItem ); + if ( context[ 0 ] && typeof context[ 0 ] != 'string' && context[ 0 ].is( 'documentFragment' ) ) { + context.shift(); + } } - // Item or position (PS. It's ok that Position#getAncestors() doesn't accept params). else { - this._items = context.getAncestors( { includeSelf: true } ).map( mapContextItem ); + // `context` is item or position. + // Position#getAncestors() doesn't accept any parameters but it works just fine here. + context = context.getAncestors( { includeSelf: true } ); + + if ( context[ 0 ].is( 'documentFragment' ) ) { + context.shift(); + } } + + this._items = context.map( mapContextItem ); } /** diff --git a/src/model/writer.js b/src/model/writer.js index d64462f98..e2d6e373a 100644 --- a/src/model/writer.js +++ b/src/model/writer.js @@ -68,11 +68,13 @@ export default class Writer { */ constructor( model, batch ) { /** + * @readonly * @type {module:engine/model/model~Model} */ this.model = model; /** + * @readonly * @type {module:engine/model/batch~Batch} */ this.batch = batch; @@ -143,8 +145,8 @@ export default class Writer { * If you want to move {@link module:engine/model/range~Range range} instead of an * {@link module:engine/model/item~Item item} use {@link module:engine/model/writer~Writer#move move}. * - * @param {module:engine/model/item~Item|module:engine/model/documentfragment~DocumentFragment} - * item Item or document fragment to insert. + * @param {module:engine/model/item~Item|module:engine/model/documentfragment~DocumentFragment} item Item or document + * fragment to insert. * @param {module:engine/model/item~Item|module:engine/model/position~Position} itemOrPosition * @param {Number|'end'|'before'|'after'} [offset=0] Offset or one of the flags. Used only when * second parameter is a {@link module:engine/model/item~Item model item}. diff --git a/tests/controller/datacontroller.js b/tests/controller/datacontroller.js index a13077b4b..dfa35896d 100644 --- a/tests/controller/datacontroller.js +++ b/tests/controller/datacontroller.js @@ -156,11 +156,11 @@ describe( 'DataController', () => { expect( count( modelDocument.history.getDeltas() ) ).to.equal( 1 ); } ); - it( 'should fire #changesDone', () => { + it( 'should cause firing change event', () => { const spy = sinon.spy(); schema.extend( '$text', { allowIn: '$root' } ); - modelDocument.on( 'changesDone', spy ); + model.document.on( 'change', spy ); data.set( 'foo' ); diff --git a/tests/conversion/buildviewconverter.js b/tests/conversion/buildviewconverter.js index 5987dae0a..f8d5e80f8 100644 --- a/tests/conversion/buildviewconverter.js +++ b/tests/conversion/buildviewconverter.js @@ -550,6 +550,21 @@ describe( 'View converter builder', () => { // expect( modelToString( conversionResult ) ).to.equal( 'foo' ); // } ); + it( 'should not set attribute when it is not allowed', () => { + buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' ); + buildViewConverter().for( dispatcher ).fromElement( 'u' ).toAttribute( 'underscore', true ); + + const viewElement = new ViewContainerElement( 'p', null, + new ViewAttributeElement( 'u', null, + new ViewText( 'foo' ) + ) + ); + + const conversionResult = dispatcher.convert( viewElement, additionalData ); + + expect( modelToString( conversionResult ) ).to.equal( 'foo' ); + } ); + it( 'should stop to element conversion if creating function returned null', () => { buildViewConverter() .for( dispatcher ) diff --git a/tests/model/differ.js b/tests/model/differ.js index cb62fe69d..b0b910364 100644 --- a/tests/model/differ.js +++ b/tests/model/differ.js @@ -230,7 +230,6 @@ describe( 'Differ', () => { remove( position, 1 ); expectChanges( [ - { type: 'insert', name: 'paragraph', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: 'paragraph', length: 1, position } ] ); } ); @@ -241,8 +240,6 @@ describe( 'Differ', () => { remove( position, 2 ); expectChanges( [ - { type: 'insert', name: 'paragraph', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, - { type: 'insert', name: 'paragraph', length: 1, position: Position.createAt( doc.graveyard, 1 ) }, { type: 'remove', name: 'paragraph', length: 1, position }, { type: 'remove', name: 'paragraph', length: 1, position } ] ); @@ -254,8 +251,6 @@ describe( 'Differ', () => { remove( position, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, - { type: 'remove', name: '$text', length: 1, position: new Position( root, [ 0, 1 ] ) } ] ); } ); @@ -266,7 +261,6 @@ describe( 'Differ', () => { remove( position, 2 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 2, position } ] ); } ); @@ -279,7 +273,6 @@ describe( 'Differ', () => { remove( position, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 3, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 3, position } ] ); } ); @@ -291,7 +284,6 @@ describe( 'Differ', () => { remove( position.getShiftedBy( 1 ), 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 1, position }, { type: 'remove', name: '$text', length: 1, position: position.getShiftedBy( 1 ) } ] ); @@ -306,7 +298,6 @@ describe( 'Differ', () => { remove( removePosition, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 1, position: removePosition }, { type: 'insert', name: '$text', length: 1, position: removePosition } ] ); @@ -320,7 +311,6 @@ describe( 'Differ', () => { remove( removePosition, 2 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 1, position: removePosition }, { type: 'insert', name: '$text', length: 2, position: removePosition } ] ); @@ -334,7 +324,6 @@ describe( 'Differ', () => { remove( removePosition, 3 ); expectChanges( [ - { type: 'insert', name: '$text', length: 3, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'insert', name: '$text', length: 1, position: insertPosition }, { type: 'remove', name: '$text', length: 1, position: removePosition } ] ); @@ -348,7 +337,6 @@ describe( 'Differ', () => { remove( removePosition, 4 ); expectChanges( [ - { type: 'insert', name: '$text', length: 4, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 2, position: removePosition } ] ); } ); @@ -361,7 +349,6 @@ describe( 'Differ', () => { remove( removePositionB, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 1, position: removePositionB }, { type: 'remove', name: '$text', length: 1, position: new Position( root, [ 0, 1 ] ) } ] ); @@ -375,7 +362,6 @@ describe( 'Differ', () => { remove( removePositionB, 2 ); expectChanges( [ - { type: 'insert', name: '$text', length: 3, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 3, position: removePositionB }, ] ); } ); @@ -392,7 +378,6 @@ describe( 'Differ', () => { const newRange = Range.createFromParentsAndOffsets( p1, 1, p1, 2 ); expectChanges( [ - { type: 'insert', name: '$text', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 1, position }, { type: 'attribute', range: newRange, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true } ] ); @@ -410,7 +395,6 @@ describe( 'Differ', () => { const newRange = Range.createFromParentsAndOffsets( p1, 0, p1, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'remove', name: '$text', length: 2, position }, { type: 'attribute', range: newRange, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true } ] ); @@ -429,7 +413,6 @@ describe( 'Differ', () => { const rangeAfter = Range.createFromParentsAndOffsets( p1, 1, p1, 2 ); expectChanges( [ - { type: 'insert', name: '$text', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'attribute', range: rangeBefore, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true }, { type: 'remove', name: '$text', length: 1, position }, { type: 'attribute', range: rangeAfter, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true } @@ -448,7 +431,6 @@ describe( 'Differ', () => { const newRange = Range.createFromParentsAndOffsets( p1, 0, p1, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 2, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'attribute', range: newRange, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true }, { type: 'remove', name: '$text', length: 2, position } ] ); @@ -464,7 +446,6 @@ describe( 'Differ', () => { remove( position, 1 ); expectChanges( [ - { type: 'insert', name: '$text', length: 1, position: Position.createAt( doc.graveyard, 0 ) }, { type: 'attribute', range, attributeKey: 'bold', attributeOldValue: null, attributeNewValue: true }, { type: 'remove', name: '$text', length: 1, position } ] ); @@ -521,7 +502,6 @@ describe( 'Differ', () => { move( sourcePosition, 1, targetPosition ); expectChanges( [ - { type: 'remove', name: 'listItem', length: 1, position: sourcePosition }, { type: 'insert', name: 'listItem', length: 1, position: targetPosition } ] ); } ); @@ -675,7 +655,7 @@ describe( 'Differ', () => { ] ); } ); - it( 'over all inserted nodes and some old nodes', () => { + it( 'only on inserted nodes', () => { const position = new Position( root, [ 0, 1 ] ); const p1 = root.getChild( 0 ); @@ -689,7 +669,7 @@ describe( 'Differ', () => { ] ); } ); - it( 'only on inserted nodes', () => { + it( 'on some inserted nodes and old nodes', () => { const position = new Position( root, [ 0, 1 ] ); const p1 = root.getChild( 0 ); @@ -706,7 +686,7 @@ describe( 'Differ', () => { ] ); } ); - it( 'on some inserted nodes and old nodes', () => { + it( 'over all inserted nodes and some old nodes', () => { const position = new Position( root, [ 0, 1 ] ); const p1 = root.getChild( 0 ); @@ -905,6 +885,83 @@ describe( 'Differ', () => { } ); } ); + describe( 'getChanges()', () => { + let position, p1, rangeAttrChange; + + beforeEach( () => { + position = new Position( root, [ 0, 1 ] ); + p1 = root.getChild( 0 ); + + const range = Range.createFromParentsAndOffsets( p1, 2, p1, 4 ); + rangeAttrChange = Range.createFromParentsAndOffsets( p1, 3, p1, 4 ); + + insert( new Text( 'xx' ), position ); + attribute( range, 'key', null, 'foo' ); + } ); + + it( 'should return changes in graveyard if a flag was set up', () => { + const removePosition = new Position( root, [ 1 ] ); + remove( removePosition, 1 ); + + expectChanges( [ + { type: 'insert', name: 'paragraph', length: 1, position: new Position( doc.graveyard, [ 0 ] ) }, + { type: 'insert', name: '$text', length: 2, position }, + { type: 'attribute', range: rangeAttrChange, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' }, + { type: 'remove', name: 'paragraph', position: removePosition, length: 1 } + ], true ); + } ); + + // Below tests test caching. + it( 'should return same change set if was called twice in a row', () => { + const changesA = differ.getChanges(); + const changesB = differ.getChanges(); + + expect( changesA ).to.deep.equal( changesB ); + } ); + + it( 'should return same change set if was called twice in a row - graveyard changes', () => { + const removePosition = new Position( root, [ 1 ] ); + remove( removePosition, 1 ); + + const changesA = differ.getChanges( { includeChangesInGraveyard: true } ); + const changesB = differ.getChanges( { includeChangesInGraveyard: true } ); + + expect( changesA ).to.deep.equal( changesB ); + } ); + + it( 'should return correct changes if change happened between getChanges() calls', () => { + expectChanges( [ + { type: 'insert', name: '$text', length: 2, position }, + { type: 'attribute', range: rangeAttrChange, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' } + ] ); + + const removePosition = new Position( root, [ 1 ] ); + remove( removePosition, 1 ); + + expectChanges( [ + { type: 'insert', name: '$text', length: 2, position }, + { type: 'attribute', range: rangeAttrChange, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' }, + { type: 'remove', name: 'paragraph', position: removePosition, length: 1 } + ] ); + } ); + + it( 'should return correct changes if reset happened between getChanges() calls', () => { + expectChanges( [ + { type: 'insert', name: '$text', length: 2, position }, + { type: 'attribute', range: rangeAttrChange, attributeKey: 'key', attributeOldValue: null, attributeNewValue: 'foo' } + ] ); + + differ.reset(); + + const removePosition = new Position( root, [ 1 ] ); + remove( removePosition, 1 ); + + expectChanges( [ + { type: 'remove', name: 'paragraph', position: removePosition, length: 1 } + ] ); + } ); + } ); + function insert( item, position ) { const operation = new InsertOperation( position, item, doc.version ); @@ -946,8 +1003,8 @@ describe( 'Differ', () => { model.applyOperation( wrapInDelta( operation ) ); } - function expectChanges( expected ) { - const changes = differ.getChanges(); + function expectChanges( expected, includeChangesInGraveyard = false ) { + const changes = differ.getChanges( { includeChangesInGraveyard } ); for ( let i = 0; i < expected.length; i++ ) { for ( const key in expected[ i ] ) { diff --git a/tests/model/document/document.js b/tests/model/document.js similarity index 72% rename from tests/model/document/document.js rename to tests/model/document.js index 075178a49..ae6bda849 100644 --- a/tests/model/document/document.js +++ b/tests/model/document.js @@ -3,17 +3,18 @@ * For licensing, see LICENSE.md. */ -import Model from '../../../src/model/model'; -import Document from '../../../src/model/document'; -import RootElement from '../../../src/model/rootelement'; -import Batch from '../../../src/model/batch'; -import Delta from '../../../src/model/delta/delta'; -import Range from '../../../src/model/range'; +import Model from '../../src/model/model'; +import Document from '../../src/model/document'; +import RootElement from '../../src/model/rootelement'; +import Text from '../../src/model/text'; +import Batch from '../../src/model/batch'; +import Delta from '../../src/model/delta/delta'; +import Range from '../../src/model/range'; import Collection from '@ckeditor/ckeditor5-utils/src/collection'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import count from '@ckeditor/ckeditor5-utils/src/count'; -import { jsonParseStringify } from '../../../tests/model/_utils/utils'; -import { setData, getData } from '../../../src/dev-utils/model'; +import { jsonParseStringify } from './_utils/utils'; +import { setData, getData } from '../../src/dev-utils/model'; describe( 'Document', () => { let model, doc; @@ -62,35 +63,22 @@ describe( 'Document', () => { batch.addDelta( delta ); } ); - it( 'for document operation: should increase document version, execute operation and fire change event with proper data', () => { - const changeCallback = sinon.spy(); - - doc.on( 'change', changeCallback ); + it( 'for document operation: should increase document version and execute operation', () => { model.applyOperation( operation ); expect( doc.version ).to.equal( 1 ); expect( doc.history._deltas.length ).to.equal( 1 ); sinon.assert.calledOnce( operation._execute ); - - sinon.assert.calledOnce( changeCallback ); - expect( changeCallback.args[ 0 ][ 1 ] ).to.equal( 't' ); - expect( changeCallback.args[ 0 ][ 2 ] ).to.equal( data ); - expect( changeCallback.args[ 0 ][ 3 ] ).to.equal( batch ); - expect( changeCallback.args[ 0 ][ 4 ] ).to.equal( delta.type ); } ); it( 'for non-document operation: should only execute operation', () => { - const changeCallback = sinon.spy(); operation.isDocumentOperation = false; - doc.on( 'change', changeCallback ); model.applyOperation( operation ); expect( doc.version ).to.equal( 0 ); expect( doc.history._deltas.length ).to.equal( 0 ); sinon.assert.calledOnce( operation._execute ); - - sinon.assert.notCalled( changeCallback ); } ); it( 'should do nothing if operation event was cancelled', () => { @@ -180,58 +168,6 @@ describe( 'Document', () => { } ); } ); - describe( 'selection', () => { - it( 'should get updated attributes whenever attribute operation is applied', () => { - sinon.spy( doc.selection, '_updateAttributes' ); - - doc.fire( 'change', 'addAttribute' ); - - expect( doc.selection._updateAttributes.called ).to.be.true; - } ); - - it( 'should throw if one of ranges starts or ends inside surrogate pair', () => { - const root = doc.createRoot(); - root.appendChildren( '\uD83D\uDCA9' ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 0, root, 1 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 2 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - } ); - - it( 'should throw if one of ranges starts or ends between base character and combining mark', () => { - const root = doc.createRoot(); - root.appendChildren( 'foo̻̐ͩbar' ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 3, root, 9 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 4, root, 9 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 5, root, 9 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 3 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 4 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - - expect( () => { - doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 5 ) ] ); - } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); - } ); - } ); - describe( 'getNearestSelectionRange()', () => { let selection; @@ -503,6 +439,169 @@ describe( 'Document', () => { } ); } ); + describe( 'differ', () => { + beforeEach( () => { + doc.createRoot(); + } ); + + it( 'should buffer document operations in differ', () => { + sinon.spy( doc.differ, 'bufferOperation' ); + + model.change( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + } ); + + expect( doc.differ.bufferOperation.called ).to.be.true; + } ); + + it( 'should not buffer changes not done on document', () => { + sinon.spy( doc.differ, 'bufferOperation' ); + + model.change( writer => { + const docFrag = writer.createDocumentFragment(); + writer.insertText( 'foo', docFrag, 0 ); + } ); + + expect( doc.differ.bufferOperation.called ).to.be.false; + } ); + + it( 'should buffer marker changes in differ', () => { + sinon.spy( doc.differ, 'bufferMarkerChange' ); + + model.change( () => { + model.markers.set( 'marker', Range.createCollapsedAt( doc.getRoot(), 0 ) ); + } ); + + expect( doc.differ.bufferMarkerChange.called ).to.be.true; + } ); + + it( 'should reset differ after change block is done', () => { + model.change( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + + expect( doc.differ.getChanges().length > 0 ).to.be.true; + } ); + + expect( doc.differ.getChanges().length ).to.equal( 0 ); + } ); + } ); + + describe( 'registerPostFixer()', () => { + beforeEach( () => { + doc.createRoot(); + } ); + + it( 'should add a callback that is fired after changes are done', () => { + const spy = sinon.spy(); + + doc.registerPostFixer( spy ); + + model.change( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + } ); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should not fire callbacks if no changes on document were done', () => { + const spy = sinon.spy(); + + doc.registerPostFixer( spy ); + + model.change( writer => { + const docFrag = writer.createDocumentFragment(); + + writer.insertText( 'foo', docFrag, 0 ); + } ); + + expect( spy.called ).to.be.false; + } ); + + it( 'should call all already processed callbacks again if a callback returned true', () => { + const callA = sinon.spy(); + const callB = sinon.stub().onFirstCall().returns( true ).onSecondCall().returns( false ); + const callC = sinon.spy(); + + doc.registerPostFixer( callA ); + doc.registerPostFixer( callB ); + doc.registerPostFixer( callC ); + + model.change( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + } ); + + expect( callA.calledTwice ).to.be.true; + expect( callB.calledTwice ).to.be.true; + expect( callC.calledOnce ).to.be.true; + } ); + } ); + + describe( 'event change', () => { + it( 'should be fired if there was a change in a document tree in a change block and have a batch as a param', () => { + doc.createRoot(); + const spy = sinon.spy(); + + doc.on( 'change', ( evt, batch ) => { + spy(); + expect( batch ).to.be.instanceof( Batch ); + } ); + + model.change( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + } ); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should be fired if there was a change in a document tree in a change block and have a batch as param', () => { + doc.createRoot(); + const spy = sinon.spy(); + + doc.on( 'change', ( evt, batch ) => { + spy(); + expect( batch ).to.be.instanceof( Batch ); + } ); + + model.enqueueChange( writer => { + writer.insertText( 'foo', doc.getRoot(), 0 ); + } ); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should be fired if there was a selection change in an (enqueue)change block', () => { + doc.createRoot(); + const spy = sinon.spy(); + + const root = doc.getRoot(); + root.appendChildren( new Text( 'foo' ) ); + + doc.on( 'change', spy ); + + model.change( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 2, root, 2 ) ] ); + } ); + + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should not be fired if writer was used on non-document tree', () => { + const spy = sinon.spy(); + + doc.on( 'change', ( evt, batch ) => { + spy(); + expect( batch ).to.be.instanceof( Batch ); + } ); + + model.change( writer => { + const docFrag = writer.createDocumentFragment(); + writer.insertText( 'foo', docFrag, 0 ); + } ); + + expect( spy.calledOnce ).to.be.false; + } ); + } ); + it( 'should be correctly converted to json', () => { const serialized = jsonParseStringify( doc ); diff --git a/tests/model/document/change-event.js b/tests/model/document/change-event.js deleted file mode 100644 index fa5e6cdf3..000000000 --- a/tests/model/document/change-event.js +++ /dev/null @@ -1,169 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -import Model from '../../../src/model/model'; -import Document from '../../../src/model/document'; -import Element from '../../../src/model/element'; -import Text from '../../../src/model/text'; -import Position from '../../../src/model/position'; -import Range from '../../../src/model/range'; -import AttributeOperation from '../../../src/model/operation/attributeoperation'; -import InsertOperation from '../../../src/model/operation/insertoperation'; -import MoveOperation from '../../../src/model/operation/moveoperation'; -import RemoveOperation from '../../../src/model/operation/removeoperation'; -import { wrapInDelta } from '../../../tests/model/_utils/utils'; - -describe( 'Document change event', () => { - let model, doc, root, graveyard, types, changes; - - beforeEach( () => { - model = new Model(); - doc = new Document( model ); - root = doc.createRoot(); - graveyard = doc.graveyard; - - types = []; - changes = []; - - doc.on( 'change', ( evt, type, change ) => { - types.push( type ); - changes.push( change ); - } ); - } ); - - it( 'should be fired when text is inserted', () => { - model.applyOperation( wrapInDelta( new InsertOperation( new Position( root, [ 0 ] ), 'foo', doc.version ) ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'insert' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) ); - } ); - - it( 'should be fired when element is inserted', () => { - const element = new Element( 'p' ); - model.applyOperation( wrapInDelta( new InsertOperation( new Position( root, [ 0 ] ), element, doc.version ) ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'insert' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 1 ) ); - } ); - - it( 'should be fired when nodes are inserted', () => { - const element = new Element( 'p' ); - model.applyOperation( wrapInDelta( new InsertOperation( new Position( root, [ 0 ] ), [ element, 'foo' ], doc.version ) ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'insert' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 4 ) ); - } ); - - it( 'should be fired when nodes are moved', () => { - const p1 = new Element( 'p' ); - p1.insertChildren( 0, [ new Element( 'p' ), new Text( 'foo' ) ] ); - - const p2 = new Element( 'p' ); - - root.insertChildren( 0, [ p1, p2 ] ); - - model.applyOperation( wrapInDelta( - new MoveOperation( - new Position( root, [ 0, 0 ] ), - 3, - new Position( root, [ 1, 0 ] ), - doc.version - ) - ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'move' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( p2, 0, p2, 3 ) ); - expect( changes[ 0 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( p1, 0 ) ); - } ); - - it( 'should be fired when multiple nodes are removed and reinserted', () => { - root.insertChildren( 0, new Text( 'foo' ) ); - - const removeOperation = new RemoveOperation( new Position( root, [ 0 ] ), 3, new Position( doc.graveyard, [ 0 ] ), doc.version ); - model.applyOperation( wrapInDelta( removeOperation ) ); - - const reinsertOperation = removeOperation.getReversed(); - model.applyOperation( wrapInDelta( reinsertOperation ) ); - - expect( changes ).to.have.length( 2 ); - - expect( types[ 0 ] ).to.equal( 'remove' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( graveyard, 0, graveyard, 3 ) ); - expect( changes[ 0 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( root, 0 ) ); - - expect( types[ 1 ] ).to.equal( 'reinsert' ); - expect( changes[ 1 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) ); - expect( changes[ 1 ].sourcePosition ).to.deep.equal( Position.createFromParentAndOffset( graveyard, 0 ) ); - } ); - - it( 'should be fired when attribute is inserted', () => { - root.insertChildren( 0, new Text( 'foo' ) ); - - model.applyOperation( wrapInDelta( - new AttributeOperation( - Range.createFromParentsAndOffsets( root, 0, root, 3 ), - 'key', - null, - 'new', - doc.version - ) - ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'addAttribute' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, root, 3 ) ); - expect( changes[ 0 ].key ).to.equal( 'key' ); - expect( changes[ 0 ].oldValue ).to.be.null; - expect( changes[ 0 ].newValue ).to.equal( 'new' ); - } ); - - it( 'should be fired when attribute is removed', () => { - const elem = new Element( 'p', { key: 'old' } ); - root.insertChildren( 0, elem ); - - model.applyOperation( wrapInDelta( - new AttributeOperation( - Range.createFromParentsAndOffsets( root, 0, elem, 0 ), - 'key', - 'old', - null, - doc.version - ) - ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'removeAttribute' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, elem, 0 ) ); - expect( changes[ 0 ].key ).to.equal( 'key' ); - expect( changes[ 0 ].oldValue ).to.equal( 'old' ); - expect( changes[ 0 ].newValue ).to.be.null; - } ); - - it( 'should be fired when attribute changes', () => { - const elem = new Element( 'p', { key: 'old' } ); - root.insertChildren( 0, elem ); - - model.applyOperation( wrapInDelta( - new AttributeOperation( - Range.createFromParentsAndOffsets( root, 0, elem, 0 ), - 'key', - 'old', - 'new', - doc.version - ) - ) ); - - expect( changes ).to.have.length( 1 ); - expect( types[ 0 ] ).to.equal( 'changeAttribute' ); - expect( changes[ 0 ].range ).to.deep.equal( Range.createFromParentsAndOffsets( root, 0, elem, 0 ) ); - expect( changes[ 0 ].key ).to.equal( 'key' ); - expect( changes[ 0 ].oldValue ).to.equal( 'old' ); - expect( changes[ 0 ].newValue ).to.equal( 'new' ); - } ); -} ); diff --git a/tests/model/documentselection.js b/tests/model/documentselection.js index a78ae5e44..9c74d8ead 100644 --- a/tests/model/documentselection.js +++ b/tests/model/documentselection.js @@ -996,7 +996,11 @@ describe( 'DocumentSelection', () => { describe( 'parent element\'s attributes', () => { it( 'are set using a normal batch', () => { const batchTypes = []; - doc.on( 'change', ( event, type, changes, batch ) => { + + model.on( 'applyOperation', ( event, args ) => { + const operation = args[ 0 ]; + const batch = operation.delta.batch; + batchTypes.push( batch.type ); } ); @@ -1015,7 +1019,10 @@ describe( 'DocumentSelection', () => { selection.setAttribute( 'foo', 'bar' ); selection.setAttribute( 'abc', 'bar' ); - doc.on( 'change', ( event, type, changes, batch ) => { + model.on( 'applyOperation', ( event, args ) => { + const operation = args[ 0 ]; + const batch = operation.delta.batch; + batchTypes.set( batch, batch.type ); } ); @@ -1121,20 +1128,6 @@ describe( 'DocumentSelection', () => { expect( emptyP.parent ).to.equal( root ); // Just to be sure we're checking the right element. } ); - it( 'are not removed on transparent batches', () => { - selection.setRanges( [ rangeInEmptyP ] ); - selection.setAttribute( 'foo', 'bar' ); - - model.enqueueChange( 'transparent', writer => { - sinon.spy( model, 'enqueueChange' ); - - writer.insertText( 'x', rangeInEmptyP.start ); - - expect( model.enqueueChange.called ).to.be.false; - expect( emptyP.getAttribute( fooStoreAttrKey ) ).to.equal( 'bar' ); - } ); - } ); - // Rename and some other deltas don't specify range in doc#change event. // So let's see if there's no crash or something. it( 'are not removed on rename', () => { @@ -1152,4 +1145,46 @@ describe( 'DocumentSelection', () => { } ); } ); } ); + + it( 'should throw if one of ranges starts or ends inside surrogate pair', () => { + root.removeChildren( 0, root.childCount ); + root.appendChildren( '\uD83D\uDCA9' ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 0, root, 1 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 2 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + } ); + + it( 'should throw if one of ranges starts or ends between base character and combining mark', () => { + root.removeChildren( 0, root.childCount ); + root.appendChildren( 'foo̻̐ͩbar' ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 3, root, 9 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 4, root, 9 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 5, root, 9 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 3 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 4 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + + expect( () => { + doc.selection.setRanges( [ Range.createFromParentsAndOffsets( root, 1, root, 5 ) ] ); + } ).to.throw( CKEditorError, /document-selection-wrong-position/ ); + } ); } ); diff --git a/tests/model/liveposition.js b/tests/model/liveposition.js index b0b5c2c77..3d81bc6ec 100644 --- a/tests/model/liveposition.js +++ b/tests/model/liveposition.js @@ -13,10 +13,10 @@ import Range from '../../src/model/range'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; describe( 'LivePosition', () => { - let doc, root, ul, p, li1, li2; + let model, doc, root, ul, p, li1, li2; - before( () => { - const model = new Model(); + beforeEach( () => { + model = new Model(); doc = model.document; root = doc.createRoot(); @@ -29,6 +29,10 @@ describe( 'LivePosition', () => { root.insertChildren( 0, [ p, ul ] ); } ); + afterEach( () => { + doc.destroy(); + } ); + it( 'should be an instance of Position', () => { const live = new LivePosition( root, [ 0 ] ); live.detach(); @@ -42,13 +46,13 @@ describe( 'LivePosition', () => { } ).to.throw( CKEditorError, /model-liveposition-root-not-rootelement/ ); } ); - it( 'should listen to a change event of the document that owns this position root', () => { + it( 'should listen to the model applyOperation event', () => { sinon.spy( LivePosition.prototype, 'listenTo' ); const live = new LivePosition( root, [ 0 ] ); live.detach(); - expect( live.listenTo.calledWith( doc, 'change' ) ).to.be.true; + expect( live.listenTo.calledWith( model, 'applyOperation' ) ).to.be.true; LivePosition.prototype.listenTo.restore(); } ); @@ -92,7 +96,7 @@ describe( 'LivePosition', () => { let live, spy; beforeEach( () => { - live = new LivePosition( root, [ 1, 4, 6 ] ); + live = new LivePosition( root, [ 1, 1, 3 ] ); spy = sinon.spy(); live.on( 'change', spy ); @@ -104,129 +108,122 @@ describe( 'LivePosition', () => { describe( 'insertion', () => { it( 'is in the same parent and closer offset', () => { - const insertRange = new Range( new Position( root, [ 1, 4, 0 ] ), new Position( root, [ 1, 4, 3 ] ) ); + model.change( writer => { + writer.insertText( 'foo', new Position( root, [ 1, 1, 0 ] ) ); + } ); - doc.fire( 'change', 'insert', { range: insertRange }, null ); - - expect( live.path ).to.deep.equal( [ 1, 4, 9 ] ); + expect( live.path ).to.deep.equal( [ 1, 1, 6 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is at the same position and live position is sticking to right side', () => { - const insertRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( root, [ 1, 1, 3 ] ) ); + } ); - expect( live.path ).to.deep.equal( [ 1, 4, 9 ] ); + expect( live.path ).to.deep.equal( [ 1, 1, 6 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is before a node from the live position path', () => { - const insertRange = new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 2 ] ) ); + model.change( writer => { + writer.insert( new Element( 'paragraph' ), new Position( root, [ 1, 0 ] ) ); + } ); - doc.fire( 'change', 'insert', { range: insertRange }, null ); - - expect( live.path ).to.deep.equal( [ 1, 6, 6 ] ); + expect( live.path ).to.deep.equal( [ 1, 2, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); } ); describe( 'range move', () => { it( 'is at the same parent and closer offset', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 4, 0 ] ), new Position( root, [ 1, 4, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 1, 1, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 1, 4, 9 ] ); + expect( live.path ).to.deep.equal( [ 1, 1, 6 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is at the same position and live position is sticking to right side', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 1, 1, 3 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 1, 4, 9 ] ); + expect( live.path ).to.deep.equal( [ 1, 1, 6 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is at a position before a node from the live position path', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 2 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 1, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 1, 6, 6 ] ); + expect( live.path ).to.deep.equal( [ 1, 3, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is from the same parent and closer offset', () => { - const moveSource = new Position( root, [ 1, 4, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 1, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 1, 0, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 1, 4, 2 ] ); + expect( live.path ).to.deep.equal( [ 1, 1, 1 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is from a position before a node from the live position path', () => { - const moveSource = new Position( root, [ 1, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 1, 2 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 1, 0, 6 ] ); + expect( live.path ).to.deep.equal( [ 1, 0, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'contains live position (same level)', () => { - const moveSource = new Position( root, [ 1, 4, 4 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 1, 2 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 1, 0, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 2, 2 ] ); + expect( live.path ).to.deep.equal( [ 1, 0, 1 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'contains live position (deep)', () => { - const moveSource = new Position( root, [ 1, 3 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 1, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( [ 2, 1, 6 ] ); + expect( live.path ).to.deep.equal( [ 1, 0, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); } ); @@ -235,12 +232,9 @@ describe( 'LivePosition', () => { describe( 'should not get transformed if', () => { let path, otherRoot, spy, live; - before( () => { - path = [ 1, 4, 6 ]; - otherRoot = doc.createRoot( '$root', 'otherRoot' ); - } ); - beforeEach( () => { + path = [ 1, 1, 3 ]; + otherRoot = doc.createRoot( '$root', 'otherRoot' ); live = new LivePosition( root, path ); spy = sinon.spy(); @@ -253,9 +247,9 @@ describe( 'LivePosition', () => { describe( 'insertion', () => { it( 'is in the same parent and further offset', () => { - const insertRange = new Range( new Position( root, [ 1, 4, 7 ] ), new Position( root, [ 1, 4, 9 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( root, [ 1, 1, 6 ] ) ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -266,9 +260,9 @@ describe( 'LivePosition', () => { spy = sinon.spy(); newLive.on( 'change', spy ); - const insertRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( root, [ 1, 1, 3 ] ) ); + } ); expect( newLive.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -277,18 +271,18 @@ describe( 'LivePosition', () => { } ); it( 'is after a node from the position path', () => { - const insertRange = new Range( new Position( root, [ 1, 5 ] ), new Position( root, [ 1, 7 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertElement( 'paragraph', new Position( root, [ 2 ] ) ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; } ); it( 'is in different root', () => { - const insertRange = new Range( new Position( otherRoot, [ 1, 4, 0 ] ), new Position( otherRoot, [ 1, 4, 4 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( otherRoot, [ 0 ] ) ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -297,14 +291,13 @@ describe( 'LivePosition', () => { describe( 'range move', () => { it( 'is at the same parent and further offset', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 4, 7 ] ), new Position( root, [ 1, 4, 9 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 1, 1, 6 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -315,14 +308,13 @@ describe( 'LivePosition', () => { spy = sinon.spy(); newLive.on( 'change', spy ); - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 4, 6 ] ), new Position( root, [ 1, 4, 9 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 1, 1, 3 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( newLive.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -331,70 +323,60 @@ describe( 'LivePosition', () => { } ); it( 'is at a position after a node from the live position path', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 1, 5 ] ), new Position( root, [ 1, 7 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 0, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 2 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; } ); it( 'is from the same parent and further offset', () => { - const moveSource = new Position( root, [ 1, 4, 7 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 1, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( otherRoot, [ 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; } ); it( 'is from a position after a node from the live position path', () => { - const moveSource = new Position( root, [ 1, 5 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); - - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); - - expect( live.path ).to.deep.equal( path ); - expect( spy.called ).to.be.false; - } ); + const newLive = new LivePosition( root, [ 1, 0, 3 ] ); + spy = sinon.spy(); + newLive.on( 'change', spy ); - it( 'is to different root', () => { - const moveSource = new Position( root, [ 2, 0 ] ); - const moveRange = new Range( new Position( otherRoot, [ 1, 0 ] ), new Position( otherRoot, [ 1, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 1, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( otherRoot, [ 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.path ).to.deep.equal( path ); + expect( newLive.path ).to.deep.equal( [ 1, 0, 3 ] ); expect( spy.called ).to.be.false; + + newLive.detach(); } ); it( 'is from different root', () => { - const moveSource = new Position( otherRoot, [ 1, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + writer.insertText( 'foo', new Position( otherRoot, [ 0 ] ) ); + + const sourcePosition = new Position( otherRoot, [ 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( otherRoot, [ 3 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; @@ -402,14 +384,9 @@ describe( 'LivePosition', () => { } ); it( 'attributes changed', () => { - const changes = { - range: new Range( new Position( root, [ 1, 4, 0 ] ), new Position( root, [ 1, 4, 10 ] ) ), - key: 'foo', - oldValue: null, - newValue: 'bar' - }; - - doc.fire( 'change', 'setAttribute', changes, null ); + model.change( writer => { + writer.setAttribute( 'foo', 'bar', new Range( new Position( root, [ 1, 1, 0 ] ), new Position( root, [ 1, 1, 6 ] ) ) ); + } ); expect( live.path ).to.deep.equal( path ); expect( spy.called ).to.be.false; diff --git a/tests/model/liverange.js b/tests/model/liverange.js index 8b743afd3..8acc46a16 100644 --- a/tests/model/liverange.js +++ b/tests/model/liverange.js @@ -3,6 +3,7 @@ * For licensing, see LICENSE.md. */ +import Batch from '../../src/model/batch'; import Model from '../../src/model/model'; import Element from '../../src/model/element'; import Position from '../../src/model/position'; @@ -43,13 +44,13 @@ describe( 'LiveRange', () => { expect( live ).to.be.instanceof( Range ); } ); - it( 'should listen to a change event of the document that owns this range', () => { + it( 'should listen to the model applyOperation event', () => { sinon.spy( LiveRange.prototype, 'listenTo' ); const live = new LiveRange( new Position( root, [ 0 ] ), new Position( root, [ 1 ] ) ); live.detach(); - expect( live.listenTo.calledWith( doc, 'change' ) ).to.be.true; + expect( live.listenTo.calledWith( model, 'applyOperation' ) ).to.be.true; LiveRange.prototype.listenTo.restore(); } ); @@ -96,16 +97,15 @@ describe( 'LiveRange', () => { const spy = sinon.spy(); live.on( 'change:range', spy ); - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 3 ] ) ); + const sourcePosition = new Position( root, [ 2 ] ); + const targetPosition = new Position( root, [ 0 ] ); + const batch = new Batch(); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - const batch = {}; + model.enqueueChange( batch, writer => { + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); - doc.fire( 'change', 'move', changes, batch ); + writer.move( sourceRange, targetPosition ); + } ); expect( spy.calledOnce ).to.be.true; @@ -115,26 +115,25 @@ describe( 'LiveRange', () => { // Second parameter is an object with data about model changes that caused the live range to change. expect( spy.args[ 0 ][ 2 ].type ).to.equal( 'move' ); expect( spy.args[ 0 ][ 2 ].batch ).to.equal( batch ); - expect( spy.args[ 0 ][ 2 ].range.isEqual( moveRange ) ).to.be.true; - expect( spy.args[ 0 ][ 2 ].sourcePosition.isEqual( moveSource ) ).to.be.true; + expect( spy.args[ 0 ][ 2 ].range.isEqual( Range.createFromPositionAndShift( targetPosition, 1 ) ) ).to.be.true; + expect( spy.args[ 0 ][ 2 ].sourcePosition.isEqual( sourcePosition ) ).to.be.true; } ); it( 'should fire change:content event with proper data when content inside the range has changed', () => { - const live = new LiveRange( new Position( root, [ 1 ] ), new Position( root, [ 3 ] ) ); + const live = new LiveRange( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 3 ] ) ); const spy = sinon.spy(); live.on( 'change:content', spy ); - const moveSource = new Position( root, [ 2, 0 ] ); - const moveRange = new Range( new Position( root, [ 4, 0 ] ), new Position( root, [ 4, 2 ] ) ); + const sourcePosition = new Position( root, [ 0, 2, 0 ] ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); + const batch = new Batch(); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - const batch = {}; + model.enqueueChange( batch, writer => { + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); - doc.fire( 'change', 'move', changes, batch ); + writer.move( sourceRange, targetPosition ); + } ); expect( spy.calledOnce ).to.be.true; @@ -145,14 +144,10 @@ describe( 'LiveRange', () => { // Second parameter is an object with data about model changes that caused the live range to change. expect( spy.args[ 0 ][ 2 ].type ).to.equal( 'move' ); expect( spy.args[ 0 ][ 2 ].batch ).to.equal( batch ); - expect( spy.args[ 0 ][ 2 ].range.isEqual( moveRange ) ).to.be.true; - expect( spy.args[ 0 ][ 2 ].sourcePosition.isEqual( moveSource ) ).to.be.true; + expect( spy.args[ 0 ][ 2 ].range.isEqual( Range.createFromPositionAndShift( targetPosition, 2 ) ) ).to.be.true; + expect( spy.args[ 0 ][ 2 ].sourcePosition.isEqual( sourcePosition ) ).to.be.true; } ); - // Examples may seem weird when you compare them with the tree structure generated at the beginning of tests. - // Since change event is fired _after_ operation is executed on tree model, you have to imagine that generated - // structure is representing what is _after_ operation is executed. So live LiveRange properties are describing - // virtual tree that is not existing anymore and event ranges are operating on the tree generated above. describe( 'should get transformed and fire change:range if', () => { let live, spy; @@ -169,19 +164,19 @@ describe( 'LiveRange', () => { describe( 'insertion', () => { it( 'is in the same parent as range start and before it', () => { - const insertRange = new Range( new Position( root, [ 0, 1, 0 ] ), new Position( root, [ 0, 1, 4 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'xxx', new Position( root, [ 0, 1, 0 ] ) ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 1, 7 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is in the same parent as range end and before it', () => { - const insertRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 3 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'xxx', new Position( root, [ 0, 2, 0 ] ) ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 5 ] ); @@ -189,19 +184,19 @@ describe( 'LiveRange', () => { } ); it( 'is at a position before a node from range start path', () => { - const insertRange = new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 0, 2 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insert( new Element( 'li' ), new Position( root, [ 0, 0 ] ) ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 3, 4 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 4, 2 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 2, 4 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is at a position before a node from range end path', () => { - const insertRange = new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 3 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insert( new Element( 'li' ), new Position( root, [ 0, 2 ] ) ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] ); @@ -211,26 +206,25 @@ describe( 'LiveRange', () => { it( 'is at the live range start position and live range is collapsed', () => { live.end.path = [ 0, 1, 4 ]; - const insertRange = new Range( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 1, 8 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'xxx', new Position( root, [ 0, 1, 4 ] ) ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 1, 8 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 1, 7 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 1, 7 ] ); expect( spy.calledOnce ).to.be.true; } ); } ); describe( 'range move', () => { it( 'is to the same parent as range start and before it', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 0, 1, 0 ] ), new Position( root, [ 0, 1, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 4 ); + const targetPosition = new Position( root, [ 0, 1, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 8 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] ); @@ -238,14 +232,13 @@ describe( 'LiveRange', () => { } ); it( 'is to the same parent as range end and before it', () => { - const moveSource = new Position( root, [ 3 ] ); - const moveRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 4 ); + const targetPosition = new Position( root, [ 0, 2, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 6 ] ); @@ -253,14 +246,13 @@ describe( 'LiveRange', () => { } ); it( 'is to a position before a node from range start path', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 0, 0 ] ), new Position( root, [ 0, 2 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 0, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 3, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 4, 2 ] ); @@ -268,14 +260,13 @@ describe( 'LiveRange', () => { } ); it( 'is to a position before a node from range end path', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 2 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 3, 2 ] ); @@ -283,44 +274,55 @@ describe( 'LiveRange', () => { } ); it( 'is from the same parent as range start and before it', () => { - const moveSource = new Position( root, [ 0, 1, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 1 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] ); expect( spy.calledOnce ).to.be.true; } ); - it( 'is from the same parent as range end and before it', () => { - const moveSource = new Position( root, [ 0, 2, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 2 ] ) ); + it( 'is from the same parent as range end and before it - #1', () => { + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 2, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); - expect( live.end.path ).to.deep.equal( [ 2, 2 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] ); + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'is from the same parent as range end and before it - #2', () => { + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 2, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); + + writer.move( sourceRange, targetPosition ); + } ); + + expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 4, 2 ] ); expect( spy.calledOnce ).to.be.true; } ); it( 'is from a position before a node from range start path', () => { - const moveSource = new Position( root, [ 0, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 1 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 4 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 0, 4 ] ); expect( live.end.path ).to.deep.equal( [ 0, 1, 2 ] ); @@ -328,14 +330,13 @@ describe( 'LiveRange', () => { } ); it( 'intersects on live range left side', () => { - const moveSource = new Position( root, [ 0, 1, 2 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 2 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 4 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 2 ] ); expect( live.end.path ).to.deep.equal( [ 0, 2, 2 ] ); @@ -343,83 +344,78 @@ describe( 'LiveRange', () => { } ); it( 'intersects on live range right side', () => { - const moveSource = new Position( root, [ 0, 2, 1 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 2, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 4 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); - expect( live.end.path ).to.deep.equal( [ 2, 1 ] ); // Included some nodes. + expect( live.end.path ).to.deep.equal( [ 0, 4, 1 ] ); // Included some nodes. expect( spy.calledOnce ).to.be.true; } ); - it( 'intersects with live range and is moved into live range', () => { - const moveSource = new Position( root, [ 0, 2, 1 ] ); - const moveRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 5 ] ) ); + it( 'is equal to live range', () => { + live.end.path = [ 0, 1, 7 ]; - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] ); + writer.move( sourceRange, targetPosition ); + } ); + + expect( live.start.path ).to.deep.equal( [ 0, 4, 0 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 4, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); - it( 'is equal to live range', () => { - live.end.path = [ 0, 1, 7 ]; + it( 'contains live range', () => { + live.end.path = [ 0, 1, 6 ]; - const moveSource = new Position( root, [ 0, 1, 4 ] ); - const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 3 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 5 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 3, 0 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 3, 3 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 4, 1 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 4, 3 ] ); expect( spy.calledOnce ).to.be.true; } ); - it( 'contains live range', () => { + it( 'is intersecting with live range on left and points to live range', () => { live.end.path = [ 0, 1, 7 ]; - const moveSource = new Position( root, [ 0, 1, 3 ] ); - const moveRange = new Range( new Position( root, [ 0, 3, 0 ] ), new Position( root, [ 0, 3, 9 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 2 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 1, 8 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 3, 1 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 3, 4 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 1, 2 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 1, 4 ] ); expect( spy.calledOnce ).to.be.true; } ); - it( 'is intersecting with live range and points to live range', () => { - live.end.path = [ 0, 1, 12 ]; - - const moveSource = new Position( root, [ 0, 1, 2 ] ); - const moveRange = new Range( new Position( root, [ 0, 1, 7 ] ), new Position( root, [ 0, 1, 10 ] ) ); + it( 'is intersecting with live range on right and is moved into live range', () => { + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 2, 1 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 5 ); + const targetPosition = new Position( root, [ 0, 2, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); - expect( live.start.path ).to.deep.equal( [ 0, 1, 9 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] ); + expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 2, 1 ] ); expect( spy.calledOnce ).to.be.true; } ); } ); @@ -628,9 +624,9 @@ describe( 'LiveRange', () => { describe( 'insertion', () => { it( 'inside the range', () => { - const insertRange = new Range( new Position( root, [ 0, 1, 7 ] ), new Position( root, [ 0, 1, 9 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'xxx', new Position( root, [ 0, 1, 7 ] ) ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.calledOnce ).to.be.true; @@ -639,61 +635,57 @@ describe( 'LiveRange', () => { describe( 'range move', () => { it( 'inside the range', () => { - const moveSource = new Position( root, [ 4 ] ); - const moveRange = new Range( new Position( root, [ 0, 1, 7 ] ), new Position( root, [ 0, 1, 9 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 1, 5 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.calledOnce ).to.be.true; } ); it( 'from the range', () => { - const moveSource = new Position( root, [ 0, 1, 6 ] ); - const moveRange = new Range( new Position( root, [ 4, 0 ] ), new Position( root, [ 4, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 5 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.calledOnce ).to.be.true; } ); it( 'from the beginning of range', () => { - const moveSource = new Position( root, [ 0, 1, 4 ] ); - const moveRange = new Range( new Position( root, [ 4, 0 ] ), new Position( root, [ 4, 3 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.calledOnce ).to.be.true; } ); it( 'from the range to the range', () => { - live.end.path = [ 0, 1, 12 ]; + live.end.path = [ 0, 1, 8 ]; - const moveSource = new Position( root, [ 0, 1, 6 ] ); - const moveRange = new Range( new Position( root, [ 0, 1, 8 ] ), new Position( root, [ 0, 1, 10 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 1, 5 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 1, 7 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.start.path ).to.deep.equal( [ 0, 1, 4 ] ); - expect( live.end.path ).to.deep.equal( [ 0, 1, 12 ] ); + expect( live.end.path ).to.deep.equal( [ 0, 1, 8 ] ); expect( spy.calledOnce ).to.be.true; } ); } ); @@ -720,27 +712,27 @@ describe( 'LiveRange', () => { describe( 'insertion', () => { it( 'is in the same parent as range end and after it', () => { - const insertRange = new Range( new Position( root, [ 0, 2, 7 ] ), new Position( root, [ 0, 2, 9 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( root, [ 0, 2, 7 ] ) ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is to a position after a node from range end path', () => { - const insertRange = new Range( new Position( root, [ 3 ] ), new Position( root, [ 4 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insert( new Element( 'li' ), new Position( root, [ 3 ] ) ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is in different root', () => { - const insertRange = new Range( new Position( otherRoot, [ 0, 0 ] ), new Position( otherRoot, [ 0, 2 ] ) ); - - doc.fire( 'change', 'insert', { range: insertRange }, null ); + model.change( writer => { + writer.insert( new Element( 'li' ), new Position( otherRoot, [ 0 ] ) ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; @@ -749,84 +741,80 @@ describe( 'LiveRange', () => { describe( 'range move', () => { it( 'is to the same parent as range end and after it', () => { - const moveSource = new Position( root, [ 4 ] ); - const moveRange = new Range( new Position( root, [ 0, 2, 3 ] ), new Position( root, [ 0, 2, 5 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4, 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 2, 4 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is to a position after a node from range end path', () => { - const moveSource = new Position( root, [ 4 ] ); - const moveRange = new Range( new Position( root, [ 0, 3 ] ), new Position( root, [ 0, 5 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 5 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 4 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is from the same parent as range end and after it', () => { - const moveSource = new Position( root, [ 0, 2, 4 ] ); - const moveRange = new Range( new Position( root, [ 4, 0 ] ), new Position( root, [ 4, 2 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 2, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 3 ); + const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is from a position after a node from range end path', () => { - const moveSource = new Position( root, [ 0, 3 ] ); - const moveRange = new Range( new Position( root, [ 5, 0 ] ), new Position( root, [ 5, 1 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 5 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is to different root', () => { - const moveSource = new Position( root, [ 2 ] ); - const moveRange = new Range( new Position( otherRoot, [ 0, 1, 0 ] ), new Position( otherRoot, [ 0, 1, 4 ] ) ); + model.change( writer => { + const sourcePosition = new Position( root, [ 0, 4 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( otherRoot, [ 0 ] ); - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; } ); it( 'is from different root', () => { - const moveSource = new Position( otherRoot, [ 0, 2, 0 ] ); - const moveRange = new Range( new Position( root, [ 2, 0 ] ), new Position( root, [ 2, 2 ] ) ); - - const changes = { - range: moveRange, - sourcePosition: moveSource - }; - doc.fire( 'change', 'move', changes, null ); + model.change( writer => { + writer.insertText( 'foo', new Position( otherRoot, [ 0 ] ) ); + + const sourcePosition = new Position( otherRoot, [ 0 ] ); + const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); + const targetPosition = new Position( root, [ 0, 4 ] ); + + writer.move( sourceRange, targetPosition ); + } ); expect( live.isEqual( clone ) ).to.be.true; expect( spy.called ).to.be.false; diff --git a/tests/model/model.js b/tests/model/model.js index a459fcfc4..9c7d77dca 100644 --- a/tests/model/model.js +++ b/tests/model/model.js @@ -196,87 +196,71 @@ describe( 'Model', () => { } ); it( 'should be possible to nest enqueueChange in enqueueChange event', () => { - model.once( 'change', () => { - model.enqueueChange( () => { - changes += 'C'; - } ); - + model.once( '_change', () => { changes += 'B'; } ); - model.on( 'changesDone', () => { - changes += 'D'; - } ); - model.enqueueChange( () => { + model.enqueueChange( () => { + changes += 'C'; + } ); + changes += 'A'; } ); - expect( changes ).to.equal( 'ABCD' ); + expect( changes ).to.equal( 'ABC' ); } ); it( 'should be possible to nest enqueueChange in changes event', () => { - model.once( 'change', () => { - model.enqueueChange( () => { - changes += 'C'; - } ); - + model.once( '_change', () => { changes += 'B'; } ); - model.on( 'changesDone', () => { - changes += 'D'; - } ); - model.change( () => { + model.enqueueChange( () => { + changes += 'C'; + } ); + changes += 'A'; } ); - expect( changes ).to.equal( 'ABCD' ); + expect( changes ).to.equal( 'ABC' ); } ); it( 'should be possible to nest changes in enqueueChange event', () => { - model.once( 'change', () => { - model.change( () => { - changes += 'B'; - } ); - + model.once( '_change', () => { changes += 'C'; } ); - model.on( 'changesDone', () => { - changes += 'D'; - } ); - model.enqueueChange( () => { - changes += 'A'; + model.change( () => { + changes += 'A'; + } ); + + changes += 'B'; } ); - expect( changes ).to.equal( 'ABCD' ); + expect( changes ).to.equal( 'ABC' ); } ); it( 'should be possible to nest changes in changes event', () => { - model.once( 'change', () => { - model.change( () => { - changes += 'B'; - } ); - + model.once( '_change', () => { changes += 'C'; } ); - model.on( 'changesDone', () => { - changes += 'D'; - } ); - model.change( () => { - changes += 'A'; + model.change( () => { + changes += 'A'; + } ); + + changes += 'B'; } ); - expect( changes ).to.equal( 'ABCD' ); + expect( changes ).to.equal( 'ABC' ); } ); it( 'should let mix blocks', () => { - model.once( 'change', () => { + model.once( '_change', () => { model.change( () => { changes += 'B'; @@ -290,15 +274,11 @@ describe( 'Model', () => { changes += 'D'; } ); - model.on( 'changesDone', () => { - changes += 'F'; - } ); - model.change( () => { changes += 'A'; } ); - expect( changes ).to.equal( 'ABCDEF' ); + expect( changes ).to.equal( 'ABCDE' ); function nestedEnqueue() { model.enqueueChange( () => { @@ -344,18 +324,15 @@ describe( 'Model', () => { } ); describe( 'applyOperation()', () => { - it( 'should execute provided operation end return the result of operation', () => { - const returnValue = { foo: 'bar' }; - + it( 'should execute provided operation', () => { const operation = { - _execute: sinon.stub().returns( returnValue ), + _execute: sinon.spy(), _validate: () => true }; model.applyOperation( operation ); sinon.assert.calledOnce( operation._execute ); - expect( model.applyOperation( operation ) ).to.equal( returnValue ); } ); } ); diff --git a/tests/model/operation/markeroperation.js b/tests/model/operation/markeroperation.js index 07d621663..9a6045ec5 100644 --- a/tests/model/operation/markeroperation.js +++ b/tests/model/operation/markeroperation.js @@ -31,13 +31,6 @@ describe( 'MarkerOperation', () => { it( 'should add marker to document marker collection', () => { sinon.spy( model.markers, 'set' ); - sinon.spy( doc, 'fire' ); - - doc.on( 'change', ( evt, type, changes ) => { - expect( type ).to.equal( 'marker' ); - expect( changes.name ).to.equal( 'name' ); - expect( changes.type ).to.equal( 'set' ); - } ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', null, range, model.markers, doc.version ) @@ -46,7 +39,6 @@ describe( 'MarkerOperation', () => { expect( doc.version ).to.equal( 1 ); expect( model.markers.set.calledWith( 'name', matchRange( range ) ) ); expect( model.markers.get( 'name' ).getRange().isEqual( range ) ).to.be.true; - expect( doc.fire.called ).to.be.true; } ); it( 'should update marker in document marker collection', () => { @@ -57,7 +49,6 @@ describe( 'MarkerOperation', () => { const range2 = Range.createFromParentsAndOffsets( root, 0, root, 3 ); sinon.spy( model.markers, 'set' ); - sinon.spy( doc, 'fire' ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, range2, model.markers, doc.version ) @@ -66,7 +57,6 @@ describe( 'MarkerOperation', () => { expect( doc.version ).to.equal( 2 ); expect( model.markers.set.calledWith( 'name', matchRange( range2 ) ) ); expect( model.markers.get( 'name' ).getRange().isEqual( range2 ) ).to.be.true; - expect( doc.fire.called ).to.be.true; } ); it( 'should remove marker from document marker collection', () => { @@ -75,13 +65,6 @@ describe( 'MarkerOperation', () => { ) ); sinon.spy( model.markers, 'remove' ); - sinon.spy( doc, 'fire' ); - - doc.on( 'change', ( evt, type, changes ) => { - expect( type ).to.equal( 'marker' ); - expect( changes.name ).to.equal( 'name' ); - expect( changes.type ).to.equal( 'remove' ); - } ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, null, model.markers, doc.version ) @@ -90,44 +73,27 @@ describe( 'MarkerOperation', () => { expect( doc.version ).to.equal( 2 ); expect( model.markers.remove.calledWith( 'name' ) ); expect( model.markers.get( 'name' ) ).to.be.null; - expect( doc.fire.called ).to.be.true; } ); - it( 'should fire document change event but not document markers remove event if removing non-existing range', () => { - sinon.spy( doc, 'fire' ); + it( 'should not fire document markers remove event if removing non-existing range', () => { sinon.spy( model.markers, 'fire' ); - doc.on( 'change', ( evt, type, changes ) => { - expect( type ).to.equal( 'marker' ); - expect( changes.name ).to.equal( 'name' ); - expect( changes.type ).to.equal( 'remove' ); - } ); - model.applyOperation( wrapInDelta( new MarkerOperation( 'name', null, null, model.markers, doc.version ) ) ); - expect( doc.fire.calledWith( 'change', 'marker' ) ).to.be.true; expect( model.markers.fire.notCalled ).to.be.true; } ); - it( 'should fire document change event but not document markers set event if newRange is same as current marker range', () => { + it( 'should not fire document markers set event if newRange is same as current marker range', () => { model.markers.set( 'name', range ); - sinon.spy( doc, 'fire' ); sinon.spy( model.markers, 'fire' ); - doc.on( 'change', ( evt, type, changes ) => { - expect( type ).to.equal( 'marker' ); - expect( changes.name ).to.equal( 'name' ); - expect( changes.type ).to.equal( 'set' ); - } ); - model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, range, model.markers, doc.version ) ) ); - expect( doc.fire.calledWith( 'change', 'marker' ) ).to.be.true; expect( model.markers.fire.notCalled ).to.be.true; } ); diff --git a/tests/model/operation/nooperation.js b/tests/model/operation/nooperation.js index ea0f350f6..04b998d48 100644 --- a/tests/model/operation/nooperation.js +++ b/tests/model/operation/nooperation.js @@ -20,10 +20,6 @@ describe( 'NoOperation', () => { expect( () => model.applyOperation( wrapInDelta( noop ) ) ).to.not.throw( Error ); } ); - it( 'should return empty object when executed', () => { - expect( noop._execute() ).to.deep.equal( {} ); - } ); - it( 'should create a NoOperation as a reverse', () => { const reverse = noop.getReversed(); diff --git a/tests/model/schema.js b/tests/model/schema.js index 94a9a0b1a..fac673b43 100644 --- a/tests/model/schema.js +++ b/tests/model/schema.js @@ -9,6 +9,7 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import Model from '../../src/model/model'; +import DocumentFragment from '../../src/model/documentfragment'; import Element from '../../src/model/element'; import Text from '../../src/model/text'; import TextProxy from '../../src/model/textproxy'; @@ -2081,6 +2082,35 @@ describe( 'SchemaContext', () => { expect( Array.from( ctx.getItem( 2 ).getAttributeKeys() ).sort() ).to.deep.equal( [ 'align' ] ); } ); + + it( 'filters out DocumentFragment when it is a first item of context - array', () => { + const ctx = new SchemaContext( [ new DocumentFragment(), 'paragraph' ] ); + + expect( ctx.length ).to.equal( 1 ); + expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] ); + } ); + + it( 'filters out DocumentFragment when it is a first item of context - element', () => { + const p = new Element( 'paragraph' ); + const docFrag = new DocumentFragment(); + docFrag.appendChildren( p ); + + const ctx = new SchemaContext( p ); + + expect( ctx.length ).to.equal( 1 ); + expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] ); + } ); + + it( 'filters out DocumentFragment when it is a first item of context - position', () => { + const p = new Element( 'paragraph' ); + const docFrag = new DocumentFragment(); + docFrag.appendChildren( p ); + + const ctx = new SchemaContext( new Position( docFrag, [ 0, 0 ] ) ); + + expect( ctx.length ).to.equal( 1 ); + expect( Array.from( ctx.getNames() ) ).to.deep.equal( [ 'paragraph' ] ); + } ); } ); describe( 'length', () => { diff --git a/tests/model/writer.js b/tests/model/writer.js index b7d968f56..e4eeb39d4 100644 --- a/tests/model/writer.js +++ b/tests/model/writer.js @@ -327,32 +327,6 @@ describe( 'Writer', () => { expect( range.end.path ).to.deep.equal( [ 2, 5 ] ); } ); - it( 'should set each marker as a separate operation', () => { - const root = doc.createRoot(); - - const spy = sinon.spy(); - const docFrag = createDocumentFragment(); - - appendText( 'abcd', root ); - appendElement( 'p', docFrag ); - insertText( 'foo bar', new Position( docFrag, [ 0, 0 ] ) ); - - const marker1 = new Range( new Position( docFrag, [ 0, 1 ] ), new Position( docFrag, [ 0, 2 ] ) ); - const marker2 = new Range( new Position( docFrag, [ 0, 5 ] ), new Position( docFrag, [ 0, 6 ] ) ); - - docFrag.markers.set( 'marker1', marker1 ); - docFrag.markers.set( 'marker2', marker2 ); - - doc.on( 'change', spy ); - - insert( docFrag, new Position( root, [ 2 ] ) ); - - sinon.assert.calledThrice( spy ); - expect( spy.firstCall.args[ 1 ] ).to.equal( 'insert' ); - expect( spy.secondCall.args[ 1 ] ).to.equal( 'marker' ); - expect( spy.thirdCall.args[ 1 ] ).to.equal( 'marker' ); - } ); - it( 'should throw when trying to use detached writer', () => { const writer = new Writer( model, batch ); const root = doc.createRoot(); @@ -1929,13 +1903,13 @@ describe( 'Writer', () => { const marker = model.markers.set( 'name', range ); const spy = sinon.spy(); - doc.on( 'change', spy ); + model.on( 'applyOperation', spy ); setMarker( marker ); const op = batch.deltas[ 0 ].operations[ 0 ]; - sinon.assert.calledOnce( spy ); - sinon.assert.calledWith( spy, sinon.match.any, 'marker' ); + expect( spy.calledOnce ).to.be.true; + expect( spy.firstCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); expect( op.oldRange ).to.be.null; expect( op.newRange.isEqual( range ) ).to.be.true; } );