diff --git a/src/dev-utils/enableenginedebug.js b/src/dev-utils/enableenginedebug.js index a1f990100..45dda9372 100644 --- a/src/dev-utils/enableenginedebug.js +++ b/src/dev-utils/enableenginedebug.js @@ -25,7 +25,6 @@ import MoveOperation from '../model/operation/moveoperation'; import NoOperation from '../model/operation/nooperation'; import RenameOperation from '../model/operation/renameoperation'; import RootAttributeOperation from '../model/operation/rootattributeoperation'; -import transform from '../model/operation/transform'; import Model from '../model/model'; import ModelDocument from '../model/document'; import ModelDocumentFragment from '../model/documentfragment'; @@ -349,24 +348,6 @@ function enableLoggingTools() { `"${ this.key }": ${ JSON.stringify( this.oldValue ) } -> ${ JSON.stringify( this.newValue ) }, ${ this.root.rootName }`; } ); - const _transformTransform = transform.transform; - - sandbox.mock( transform, 'transform', function( a, b, context ) { - let results; - - try { - results = _transformTransform( a, b, context ); - } catch ( e ) { - logger.error( 'Error during operation transformation!' ); - logger.error( a.toString() + ( context.isStrong ? ' (important)' : '' ) ); - logger.error( b.toString() + ( context.isStrong ? '' : ' (important)' ) ); - - throw e; - } - - return results; - } ); - sandbox.mock( ViewText.prototype, 'toString', function() { return `#${ this.data }`; } ); diff --git a/src/model/documentselection.js b/src/model/documentselection.js index 3317be489..853c1b748 100644 --- a/src/model/documentselection.js +++ b/src/model/documentselection.js @@ -750,14 +750,14 @@ class LiveSelection extends Selection { const liveRange = LiveRange.createFromRange( range ); - liveRange.on( 'change:range', ( evt, oldRange, operation ) => { + liveRange.on( 'change:range', ( evt, oldRange, data ) => { this._hasChangedRange = true; // If `LiveRange` is in whole moved to the graveyard, save necessary data. It will be fixed on `Model#applyOperation` event. if ( liveRange.root == this._document.graveyard ) { this._fixGraveyardRangesData.push( { liveRange, - sourcePosition: operation.sourcePosition + sourcePosition: data.deletionPosition } ); } } ); diff --git a/src/model/liverange.js b/src/model/liverange.js index 8ce4b454a..73480f7b8 100644 --- a/src/model/liverange.js +++ b/src/model/liverange.js @@ -86,9 +86,7 @@ export default class LiveRange extends Range { * @param {Object} data Object with additional information about the change. Those parameters are passed from * {@link module:engine/model/document~Document#event:change document change event}. * @param {String} data.type Change type. - * @param {module:engine/model/batch~Batch} data.batch Batch which changed the live range. - * @param {module:engine/model/range~Range} data.range Range containing the result of applied change. - * @param {module:engine/model/position~Position} data.sourcePosition Source position for move, remove and reinsert change types. + * @param {module:engine/model/position~Position|null} deletionPosition Source position for move, remove and reinsert change types. */ /** @@ -145,18 +143,28 @@ function transform( operation ) { const result = Range.createFromRanges( ranges ); const boundariesChanged = !result.isEqual( this ); const contentChanged = doesOperationChangeRangeContent( this, operation ); + let deletionPosition = null; if ( boundariesChanged ) { + if ( result.root.rootName == '$graveyard' ) { + if ( operation.type == 'remove' ) { + deletionPosition = operation.sourcePosition; + } else { + // Merge operation. + deletionPosition = operation.deletionPosition; + } + } + // If range boundaries have changed, fire `change:range` event. const oldRange = Range.createFromRange( this ); this.start = result.start; this.end = result.end; - this.fire( 'change:range', oldRange, operation ); + this.fire( 'change:range', oldRange, { deletionPosition } ); } else if ( contentChanged ) { // If range boundaries have not changed, but there was change inside the range, fire `change:content` event. - this.fire( 'change:content', Range.createFromRange( this ), operation ); + this.fire( 'change:content', Range.createFromRange( this ), { deletionPosition } ); } } diff --git a/src/model/operation/detachoperation.js b/src/model/operation/detachoperation.js index 5441991fa..6da08e0d2 100644 --- a/src/model/operation/detachoperation.js +++ b/src/model/operation/detachoperation.js @@ -60,7 +60,6 @@ export default class DetachOperation extends Operation { if ( this.sourcePosition.root.document ) { /** * Cannot detach document node. - * Use {@link module:engine/model/operation/removeoperation~RemoveOperation remove operation} instead. * * @error detach-operation-on-document-node */ diff --git a/src/model/operation/insertoperation.js b/src/model/operation/insertoperation.js index a67577758..f5aa6a9dc 100644 --- a/src/model/operation/insertoperation.js +++ b/src/model/operation/insertoperation.js @@ -10,7 +10,7 @@ import Operation from './operation'; import Position from '../position'; import NodeList from '../nodelist'; -import RemoveOperation from './removeoperation'; +import MoveOperation from './moveoperation'; import { _insert, _normalizeNodes } from './utils'; import Text from '../text'; import Element from '../element'; @@ -87,13 +87,13 @@ export default class InsertOperation extends Operation { /** * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}. * - * @returns {module:engine/model/operation/removeoperation~RemoveOperation} + * @returns {module:engine/model/operation/moveoperation~MoveOperation} */ getReversed() { const graveyard = this.position.root.document.graveyard; const gyPosition = new Position( graveyard, [ 0 ] ); - return new RemoveOperation( this.position, this.nodes.maxOffset, gyPosition, this.baseVersion + 1 ); + return new MoveOperation( this.position, this.nodes.maxOffset, gyPosition, this.baseVersion + 1 ); } /** diff --git a/src/model/operation/moveoperation.js b/src/model/operation/moveoperation.js index 3672cee28..6ba3e841c 100644 --- a/src/model/operation/moveoperation.js +++ b/src/model/operation/moveoperation.js @@ -64,6 +64,12 @@ export default class MoveOperation extends Operation { * @inheritDoc */ get type() { + if ( this.targetPosition.root.rootName == '$graveyard' ) { + return 'remove'; + } else if ( this.sourcePosition.root.rootName == '$graveyard' ) { + return 'reinsert'; + } + return 'move'; } diff --git a/src/model/operation/operationfactory.js b/src/model/operation/operationfactory.js index d30dbc7ae..b5c02c4e3 100644 --- a/src/model/operation/operationfactory.js +++ b/src/model/operation/operationfactory.js @@ -13,8 +13,6 @@ import MarkerOperation from '../operation/markeroperation'; import MoveOperation from '../operation/moveoperation'; import NoOperation from '../operation/nooperation'; import Operation from '../operation/operation'; -import ReinsertOperation from '../operation/reinsertoperation'; -import RemoveOperation from '../operation/removeoperation'; import RenameOperation from '../operation/renameoperation'; import RootAttributeOperation from '../operation/rootattributeoperation'; import SplitOperation from '../operation/splitoperation'; @@ -29,8 +27,6 @@ operations[ MarkerOperation.className ] = MarkerOperation; operations[ MoveOperation.className ] = MoveOperation; operations[ NoOperation.className ] = NoOperation; operations[ Operation.className ] = Operation; -operations[ ReinsertOperation.className ] = ReinsertOperation; -operations[ RemoveOperation.className ] = RemoveOperation; operations[ RenameOperation.className ] = RenameOperation; operations[ RootAttributeOperation.className ] = RootAttributeOperation; operations[ SplitOperation.className ] = SplitOperation; diff --git a/src/model/operation/reinsertoperation.js b/src/model/operation/reinsertoperation.js deleted file mode 100644 index 270884e7f..000000000 --- a/src/model/operation/reinsertoperation.js +++ /dev/null @@ -1,76 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/** - * @module engine/model/operation/reinsertoperation - */ - -import MoveOperation from './moveoperation'; -import RemoveOperation from './removeoperation'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; - -/** - * Operation to reinsert previously removed nodes back to the non-graveyard root. This operation acts like - * {@link module:engine/model/operation/moveoperation~MoveOperation} but it returns - * {@link module:engine/model/operation/removeoperation~RemoveOperation} when reversed - * and fires different change event. - */ -export default class ReinsertOperation extends MoveOperation { - /** - * Position where nodes will be re-inserted. - * - * @type {module:engine/model/position~Position} - */ - get position() { - return this.targetPosition; - } - - /** - * @param {module:engine/model/position~Position} pos - */ - set position( pos ) { - this.targetPosition = pos; - } - - /** - * @inheritDoc - */ - get type() { - return 'reinsert'; - } - - /** - * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}. - * - * @returns {module:engine/model/operation/removeoperation~RemoveOperation} - */ - getReversed() { - const newTargetPosition = this.sourcePosition._getTransformedByInsertion( this.targetPosition, this.howMany ); - - return new RemoveOperation( this.getMovedRangeStart(), this.howMany, newTargetPosition, this.baseVersion + 1 ); - } - - /** - * @inheritDoc - */ - _validate() { - super._validate(); - - if ( !this.sourcePosition.root.document ) { - throw new CKEditorError( 'reinsert-operation-on-detached-item: Cannot reinsert detached item.' ); - } - - if ( !this.targetPosition.root.document ) { - throw new CKEditorError( 'reinsert-operation-to-detached-parent: Cannot reinsert item to detached parent.' ); - } - } - - /** - * @inheritDoc - */ - static get className() { - return 'engine.model.operation.ReinsertOperation'; - } -} diff --git a/src/model/operation/removeoperation.js b/src/model/operation/removeoperation.js deleted file mode 100644 index ba26e3629..000000000 --- a/src/model/operation/removeoperation.js +++ /dev/null @@ -1,60 +0,0 @@ -/** - * @license Copyright (c) 2003-2018, CKSource - Frederico Knabben. All rights reserved. - * For licensing, see LICENSE.md. - */ - -/** - * @module engine/model/operation/removeoperation - */ - -import MoveOperation from './moveoperation'; -import ReinsertOperation from './reinsertoperation'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; - -/** - * Operation to remove a range of nodes. - */ -export default class RemoveOperation extends MoveOperation { - /** - * @inheritDoc - */ - get type() { - return 'remove'; - } - - /** - * See {@link module:engine/model/operation/operation~Operation#getReversed `Operation#getReversed()`}. - * - * @returns {module:engine/model/operation/reinsertoperation~ReinsertOperation|module:engine/model/operation/nooperation~NoOperation} - */ - getReversed() { - const newTargetPosition = this.sourcePosition._getTransformedByInsertion( this.targetPosition, this.howMany ); - - return new ReinsertOperation( this.getMovedRangeStart(), this.howMany, newTargetPosition, this.baseVersion + 1 ); - } - - /** - * @inheritDoc - */ - _validate() { - super._validate(); - - if ( !this.sourcePosition.root.document ) { - /** - * Item that is going to be removed needs to be a {@link module:engine/model/document~Document document} child. - * To remove Item from detached document fragment use - * {@link module:engine/model/operation/detachoperation~DetachOperation DetachOperation}. - * - * @error remove-operation-on-detached-item - */ - throw new CKEditorError( 'remove-operation-on-detached-item: Cannot remove detached item.' ); - } - } - - /** - * @inheritDoc - */ - static get className() { - return 'engine.model.operation.RemoveOperation'; - } -} diff --git a/src/model/operation/splitoperation.js b/src/model/operation/splitoperation.js index 0b00419d5..4ed0267c1 100644 --- a/src/model/operation/splitoperation.js +++ b/src/model/operation/splitoperation.js @@ -43,6 +43,10 @@ export default class SplitOperation extends Operation { this.position.stickiness = 'toNext'; this.graveyardPosition = graveyardPosition ? Position.createFromPosition( graveyardPosition ) : null; + + if ( this.graveyardPosition ) { + this.graveyardPosition.stickiness = 'toNext'; + } } /** diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index f0a9dba73..bca951da0 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -3,8 +3,6 @@ import AttributeOperation from './attributeoperation'; import RenameOperation from './renameoperation'; import MarkerOperation from './markeroperation'; import MoveOperation from './moveoperation'; -import RemoveOperation from './removeoperation'; -import ReinsertOperation from './reinsertoperation'; import RootAttributeOperation from './rootattributeoperation'; import MergeOperation from './mergeoperation'; import SplitOperation from './splitoperation'; @@ -13,7 +11,9 @@ import UnwrapOperation from './unwrapoperation'; import NoOperation from './nooperation'; import Range from '../range'; import Position from '../position'; + import compareArrays from '@ckeditor/ckeditor5-utils/src/comparearrays'; +import log from '@ckeditor/ckeditor5-utils/src/log'; const transformations = new Map(); @@ -29,32 +29,10 @@ function setTransformation( OperationA, OperationB, transformationFunction ) { } function getTransformation( a, b ) { - const OperationA = a.constructor; - const OperationB = b.constructor; - - const aGroups = new Set(); - const aGroup = transformations.get( OperationA ); - - if ( aGroup ) { - aGroups.add( aGroup ); - } - - for ( const Operation of transformations.keys() ) { - if ( a instanceof Operation ) { - aGroups.add( transformations.get( Operation ) ); - } - } + const aGroup = transformations.get( a.constructor ); - for ( const group of aGroups ) { - if ( group.has( OperationB ) ) { - return group.get( OperationB ); - } - - for ( const Operation of group.keys() ) { - if ( b instanceof Operation ) { - return group.get( Operation ); - } - } + if ( aGroup && aGroup.has( b.constructor ) ) { + return aGroup.get( b.constructor ); } return noUpdateTransformation; @@ -72,13 +50,26 @@ function updateBaseVersions( operations, baseVersion ) { return operations; } -function transform( a, b, context = { aIsStrong: false } ) { +export function transform( a, b, context = {} ) { const transformationFunction = getTransformation( a, b ); - return transformationFunction( a.clone(), b, context ); + try { + return transformationFunction( a.clone(), b, context ); + } catch ( e ) { + log.error( 'Error during operation transformation!' ); + log.error( 'Transformed operation', a ); + log.error( 'Operation transformed by', b ); + log.error( 'context.aIsStrong', context.aIsStrong ); + log.error( 'context.aWasUndone', context.aWasUndone ); + log.error( 'context.bWasUndone', context.bWasUndone ); + log.error( 'context.abRelation', context.abRelation ); + log.error( 'context.baRelation', context.baRelation ); + + throw e; + } } -function transformSets( operationsA, operationsB, options ) { +export function transformSets( operationsA, operationsB, options ) { operationsA = operationsA.slice(); operationsB = operationsB.slice(); @@ -113,17 +104,34 @@ function transformSets( operationsA, operationsB, options ) { const opA = opsA[ k ]; const opB = opsB[ l ]; - context.aIsStrong = true; - const newOpA = transform( opA, opB, context ); - - context.aIsStrong = false; - const newOpB = transform( opB, opA, context ); + if ( options.useContext ) { + updateRelations( context, opA, opB ); + } - delete context.aIsStrong; + const contextAB = { + aIsStrong: true, + aWasUndone: context.wasUndone( opA ), + bWasUndone: context.wasUndone( opB ), + abRelation: context.getRelation( opA, opB ), + baRelation: context.getRelation( opB, opA ) + }; + + const contextBA = { + aIsStrong: false, + aWasUndone: context.wasUndone( opB ), + bWasUndone: context.wasUndone( opA ), + abRelation: context.getRelation( opB, opA ), + baRelation: context.getRelation( opA, opB ) + }; + + const newOpA = transform( opA, opB, contextAB ); + const newOpB = transform( opB, opA, contextBA ); if ( options.useContext ) { updateOriginalOperation( context, opA, newOpA ); updateOriginalOperation( context, opB, newOpB ); + + updateRelations( context, opA, opB ); } opsA.splice( k, 1, ...newOpA ); @@ -155,11 +163,6 @@ function transformSets( operationsA, operationsB, options ) { return { operationsA, operationsB }; } -export default { - transform, - transformSets -}; - function padWithNoOps( operations, howMany ) { for ( let i = 0; i < howMany; i++ ) { operations.push( new NoOperation( 0 ) ); @@ -176,6 +179,7 @@ function initializeContext( opsA, opsB, options ) { } context.document = options.document; + context.relations = new Map(); context.wasUndone = function( op ) { if ( !options.useContext ) { @@ -187,9 +191,152 @@ function initializeContext( opsA, opsB, options ) { return this.document.history.isUndoneOperation( originalOp ); }; + context.getRelation = function( opA, opB ) { + if ( !options.useContext ) { + return null; + } + + const origB = this.originalOperations.get( opB ); + const undoneB = this.document.history.getUndoneOperation( origB ); + + if ( !undoneB ) { + return null; + } + + const origA = this.originalOperations.get( opA ); + const relationsA = this.relations.get( origA ); + + if ( relationsA ) { + return relationsA.get( undoneB ) || null; + } + + return null; + }; + return context; } +function updateRelations( context, opA, opB ) { + switch ( opA.constructor ) { + case MoveOperation: { + switch ( opB.constructor ) { + case MergeOperation: { + if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.targetPosition ) ) { + setRelation( context, opA, opB, 'insertAtSource' ); + setRelation( context, opB, opA, 'splitBefore' ); + } + + break; + } + + case MoveOperation: { + if ( opA.targetPosition.isEqual( opB.sourcePosition ) || opA.targetPosition.isBefore( opB.sourcePosition ) ) { + setRelation( context, opA, opB, 'insertBefore' ); + setRelation( context, opB, opA, 'insertAfter' ); + } else { + setRelation( context, opA, opB, 'insertAfter' ); + setRelation( context, opB, opA, 'insertBefore' ); + } + + break; + } + + case UnwrapOperation: { + const isInside = opA.targetPosition.hasSameParentAs( opB.targetPosition ); + + if ( isInside ) { + setRelation( context, opA, opB, 'insertInside' ); + } + + break; + } + } + + break; + } + + case SplitOperation: { + switch ( opB.constructor ) { + case MergeOperation: { + if ( opA.position.isBefore( opB.sourcePosition ) ) { + setRelation( context, opA, opB, 'splitBefore' ); + setRelation( context, opB, opA, 'splitAfter' ); + } + + break; + } + + case MoveOperation: { + if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) { + setRelation( context, opA, opB, 'splitBefore' ); + setRelation( context, opB, opA, 'insertAtSource' ); + } + + break; + } + + case UnwrapOperation: { + const isInside = opA.position.hasSameParentAs( opB.position ); + + if ( isInside ) { + setRelation( context, opA, opB, 'splitInside' ); + } + + break; + } + } + + break; + } + + case InsertOperation: { + switch ( opB.constructor ) { + case MergeOperation: { + if ( opA.position.isEqual( opB.sourcePosition ) || opB.movedRange.containsPosition( opA.position ) ) { + setRelation( context, opA, opB, 'insertAtSource' ); + } + + break; + } + + case MoveOperation: { + if ( opA.position.isEqual( opB.sourcePosition ) || opA.position.isBefore( opB.sourcePosition ) ) { + setRelation( context, opA, opB, 'insertBefore' ); + } + + break; + } + + case UnwrapOperation: { + const isInside = opA.position.hasSameParentAs( opB.position ); + + if ( isInside ) { + setRelation( context, opA, opB, 'insertInside' ); + } + + break; + } + } + + break; + } + } +} + +function setRelation( context, opA, opB, relation ) { + const origA = context.originalOperations.get( opA ); + const origB = context.originalOperations.get( opB ); + + let relationsA = context.relations.get( origA ); + + if ( !relationsA ) { + relationsA = new Map(); + context.relations.set( origA, relationsA ); + } + + relationsA.set( origB, relation ); +} + function updateOriginalOperation( context, oldOp, newOps ) { const originalOp = context.originalOperations.get( oldOp ); @@ -269,7 +416,7 @@ setTransformation( AttributeOperation, InsertOperation, ( a, b ) => { //

Fo[zb]ar

// // New text with `highlight="red"` is typed: - //

Fo[z<$text higlight="red">xa]r

+ //

Fo[z<$text highlight="red">xa]r

// // In this case three operations are needed: `oldValue=null, newValue="yellow"` for `z`, `oldValue="red", // newValue="yellow"` for `x` and `oldValue=null, newValue="yellow"` for `a`. It could even happen that @@ -345,6 +492,8 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => { // Case 1: Attribute change on the merged element. In this case, the merged element was moved to graveyard. // An additional attribute operation that will change the (re)moved element needs to be generated. + // Do it only, if there is more than one element in attribute range. If there is only one element, + // it will be handled by the default algorithm. // if ( a.range.start.hasSameParentAs( b.deletionPosition ) ) { if ( a.range.containsPosition( b.deletionPosition ) || a.range.start.isEqual( b.deletionPosition ) ) { @@ -352,7 +501,12 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => { } } - ranges.push( a.range._getTransformedByMergeOperation( b ) ); + const range = a.range._getTransformedByMergeOperation( b ); + + // Do not add empty (collapsed) ranges to the result. `range` may be collapsed if it contained only the merged element. + if ( !range.isCollapsed ) { + ranges.push( range ); + } // Create `AttributeOperation`s out of the ranges. return ranges.map( range => { @@ -361,20 +515,10 @@ setTransformation( AttributeOperation, MergeOperation, ( a, b ) => { } ); setTransformation( AttributeOperation, MoveOperation, ( a, b ) => { - const movedRange = Range.createFromPositionAndShift( b.sourcePosition, b.howMany ); const ranges = breakRangeByMoveOperation( a.range, b, true ); // Create `AttributeOperation`s out of the ranges. - return ranges.map( range => { - if ( movedRange.containsRange( range, true ) ) { - range = range._getTransformedByMoveOperation( b, false )[ 0 ]; - } else { - range = range._getTransformedByDeletion( b.sourcePosition, b.howMany ); - range = range._getTransformedByInsertion( b.targetPosition, b.howMany, false )[ 0 ]; - } - - return new AttributeOperation( range, a.key, a.oldValue, a.newValue, a.baseVersion ); - } ); + return ranges.map( range => new AttributeOperation( range, a.key, a.oldValue, a.newValue, a.baseVersion ) ); } ); function breakRangeByMoveOperation( range, moveOp, includeCommon ) { @@ -382,32 +526,48 @@ function breakRangeByMoveOperation( range, moveOp, includeCommon ) { const movedRange = Range.createFromPositionAndShift( moveOp.sourcePosition, moveOp.howMany ); - if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) { + if ( movedRange.containsRange( range, true ) ) { + ranges = [ ...range._getTransformedByMoveOperation( moveOp ) ]; + } else if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) { ranges = range.getDifference( movedRange ); + _flatMoveTransform( ranges, moveOp ); + if ( includeCommon ) { const common = range.getIntersection( movedRange ); if ( common ) { - ranges.push( common ); + ranges.push( ...common._getTransformedByMoveOperation( moveOp ) ); } } } else { ranges = [ range ]; + + _flatMoveTransform( ranges, moveOp ); } + return ranges; +} + +function _flatMoveTransform( ranges, moveOp ) { + const targetPosition = moveOp.getMovedRangeStart(); + for ( let i = 0; i < ranges.length; i++ ) { - const range = ranges[ i ]; + let range = ranges[ i ]; - if ( range.start.hasSameParentAs( moveOp.targetPosition ) && range.containsPosition( moveOp.targetPosition ) ) { - ranges.splice( i, 1, - new Range( range.start, moveOp.targetPosition ), - new Range( moveOp.targetPosition, range.end ) - ); + if ( range.start.hasSameParentAs( moveOp.sourcePosition ) ) { + range = range._getTransformedByDeletion( moveOp.sourcePosition, moveOp.howMany ); } - } - return ranges; + if ( range.start.hasSameParentAs( targetPosition ) ) { + const result = range._getTransformedByInsertion( targetPosition, moveOp.howMany, true ); + + ranges.splice( i, 1, ...result ); + i = i - 1 + result.length; + } else { + ranges[ i ] = range; + } + } } setTransformation( AttributeOperation, SplitOperation, ( a, b ) => { @@ -443,10 +603,13 @@ setTransformation( AttributeOperation, SplitOperation, ( a, b ) => { if ( a.range.start.hasSameParentAs( b.position ) && a.range.containsPosition( b.position ) ) { const secondPart = a.clone(); - secondPart.range.start = Position.createFromPosition( b.moveTargetPosition ); - secondPart.range.end = a.range.end._getCombined( b.position, b.moveTargetPosition ); + secondPart.range = new Range( + Position.createFromPosition( b.moveTargetPosition ), + a.range.end._getCombined( b.position, b.moveTargetPosition ) + ); a.range.end = Position.createFromPosition( b.position ); + a.range.end.stickiness = 'toPrevious'; return [ a, secondPart ]; } @@ -541,13 +704,23 @@ setTransformation( InsertOperation, InsertOperation, ( a, b, context ) => { return [ a ]; } ); -setTransformation( InsertOperation, MoveOperation, ( a, b ) => { +setTransformation( InsertOperation, MoveOperation, ( a, b, context ) => { + if ( a.position.isEqual( b.targetPosition ) && context.abRelation == 'insertBefore' ) { + return [ a ]; + } + a.position = a.position._getTransformedByMoveOperation( b ); return [ a ]; } ); -setTransformation( InsertOperation, SplitOperation, ( a, b ) => { +setTransformation( InsertOperation, SplitOperation, ( a, b, context ) => { + if ( a.position.isEqual( b.position ) && context.abRelation == 'insertAtSource' ) { + a.position = b.moveTargetPosition; + + return [ a ]; + } + a.position = a.position._getTransformedBySplitOperation( b ); return [ a ]; @@ -559,7 +732,13 @@ setTransformation( InsertOperation, MergeOperation, ( a, b ) => { return [ a ]; } ); -setTransformation( InsertOperation, WrapOperation, ( a, b ) => { +setTransformation( InsertOperation, WrapOperation, ( a, b, context ) => { + if ( a.position.isEqual( b.position ) && context.abRelation == 'insertInside' ) { + a.position = b.targetPosition; + + return [ a ]; + } + a.position = a.position._getTransformedByWrapOperation( b ); return [ a ]; @@ -707,7 +886,7 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => { // const removedRange = Range.createFromPositionAndShift( b.sourcePosition, b.howMany ); - if ( b instanceof RemoveOperation && !context.wasUndone( b ) ) { + if ( b.type == 'remove' && !context.bWasUndone ) { if ( a.deletionPosition.hasSameParentAs( b.sourcePosition ) && removedRange.containsPosition( a.sourcePosition ) ) { return getNoOp(); } @@ -716,7 +895,7 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => { a.sourcePosition = a.sourcePosition._getTransformedByMoveOperation( b ); a.targetPosition = a.targetPosition._getTransformedByMoveOperation( b ); - if ( !a.graveyardPosition.isEqual( b.targetPosition ) || !context.aIsStrong ) { + if ( !a.graveyardPosition.isEqual( b.targetPosition ) ) { a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b ); } @@ -724,8 +903,6 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => { } ); setTransformation( MergeOperation, SplitOperation, ( a, b ) => { - a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b ); - if ( b.graveyardPosition ) { a.graveyardPosition = a.graveyardPosition._getTransformedByDeletion( b.graveyardPosition, 1 ); } @@ -744,13 +921,18 @@ setTransformation( MergeOperation, SplitOperation, ( a, b ) => { // This means that `targetPosition` needs to be transformed. This is the default case though. // For example, if the split would be after `F`, `targetPosition` should also be transformed. // - // There is one exception though - when the split operation is a result of undo. In those cases, it is needed - // to keep `targetPosition` intact, so the nodes are returned to the correct element. + // There is an exception though. It is when merge operation targets into inside of an element. + // Such merge operation can be a result of merge x merge transformation, when merges are identical. + // Such merge operation's source position is in graveyard and we will use that to recognize it + // (although a more precise method would be more correct). // - if ( a.targetPosition.isEqual( b.position ) && b.graveyardPosition ) { + if ( a.targetPosition.isEqual( b.position ) && a.sourcePosition.root.rootName == '$graveyard' ) { + a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b ); + return [ a ]; } + a.sourcePosition = a.sourcePosition._getTransformedBySplitOperation( b ); a.targetPosition = a.targetPosition._getTransformedBySplitOperation( b ); return [ a ]; @@ -774,6 +956,23 @@ setTransformation( MergeOperation, WrapOperation, ( a, b ) => { return [ a ]; } + // Case 2: Merged element is wrapped and this is the last (only) element in the wrap. + // Because of how this is resolved in `WrapOperation` x `MergeOperation`, we need to apply special handling here. + // If the last element from wrapped range is "removed" from it, the wrap is effectively on empty range. + // In that case, the wrapper element is moved to graveyard. This happens in `WrapOperation` x + // `MergeOperation` and we need to mirror it here. + // + if ( b.position.isEqual( a.deletionPosition ) && b.howMany == 1 ) { + // We need to change `MergeOperation#graveyardPosition` so the merged node is moved into the wrapper element. + // Since `UnwrapOperation` created from reverse has graveyard position at [ 0 ], we can safely set the path here to [ 0, 0 ]. + a.graveyardPosition = new Position( a.graveyardPosition.root, [ 0, 0 ] ); + + return [ + b.getReversed(), + a + ]; + } + a.sourcePosition = a.sourcePosition._getTransformedByWrapOperation( b ); a.targetPosition = a.targetPosition._getTransformedByWrapOperation( b ); @@ -814,14 +1013,14 @@ setTransformation( MergeOperation, UnwrapOperation, ( a, b, context ) => { // ----------------------- -setTransformation( MoveOperation, InsertOperation, ( a, b ) => { +setTransformation( MoveOperation, InsertOperation, ( a, b, context ) => { const moveRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); const transformed = moveRange._getTransformedByInsertOperation( b, false )[ 0 ]; a.sourcePosition = transformed.start; a.howMany = transformed.end.offset - transformed.start.offset; - if ( !a.targetPosition.isEqual( b.position ) ) { + if ( !a.targetPosition.isEqual( b.position ) || context.abRelation == 'insertBefore' ) { a.targetPosition = a.targetPosition._getTransformedByInsertOperation( b ); } @@ -840,12 +1039,27 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => { // this algorithm and we do not want to override original `context.aIsStrong` that will be used in later transformations. let aIsStrong = context.aIsStrong; + if ( context.abRelation == 'insertBefore' ) { + aIsStrong = true; + } else if ( context.abRelation == 'insertAfter' ) { + aIsStrong = false; + } + // `a.targetPosition` could be affected by the `b` operation. We will transform it. - const newTargetPosition = a.targetPosition._getTransformedByMove( - b.sourcePosition, - b.targetPosition, - b.howMany - ); + let newTargetPosition; + + if ( a.targetPosition.isEqual( b.targetPosition ) && aIsStrong ) { + newTargetPosition = a.targetPosition._getTransformedByDeletion( + b.sourcePosition, + b.howMany + ); + } else { + newTargetPosition = a.targetPosition._getTransformedByMove( + b.sourcePosition, + b.targetPosition, + b.howMany + ); + } // // Special case #1 + mirror. @@ -926,9 +1140,9 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => { // // If only one of operations is a remove operation, we force remove operation to be the "stronger" one // to provide more expected results. - if ( a instanceof RemoveOperation && !( b instanceof RemoveOperation ) ) { + if ( a.type == 'remove' && b.type != 'remove' ) { aIsStrong = true; - } else if ( !( a instanceof RemoveOperation ) && b instanceof RemoveOperation ) { + } else if ( a.type != 'remove' && b.type == 'remove' ) { aIsStrong = false; } @@ -993,7 +1207,7 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => { return makeMoveOperationsFromRanges( ranges, newTargetPosition ); } ); -setTransformation( MoveOperation, SplitOperation, ( a, b ) => { +setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => { const newTargetPosition = a.targetPosition._getTransformedBySplitOperation( b ); // Case 1: Last element in the moved range got split. @@ -1028,8 +1242,8 @@ setTransformation( MoveOperation, SplitOperation, ( a, b ) => { rightRange = rightRange._getTransformedBySplitOperation( b ); const ranges = [ - rightRange, - Range.createFromPositionAndShift( moveRange.start, b.position.offset - moveRange.start.offset ) + new Range( moveRange.start, b.position ), + rightRange ]; return makeMoveOperationsFromRanges( ranges, newTargetPosition ); @@ -1043,6 +1257,10 @@ setTransformation( MoveOperation, SplitOperation, ( a, b ) => { a.howMany = transformed.end.offset - transformed.start.offset; a.targetPosition = newTargetPosition; + if ( a.targetPosition.isEqual( b.position ) && context.abRelation == 'insertAtSource' ) { + a.targetPosition = b.targetPosition; + } + return [ a ]; } ); @@ -1050,14 +1268,14 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => { const movedRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); if ( b.deletionPosition.hasSameParentAs( a.sourcePosition ) && movedRange.containsPosition( b.sourcePosition ) ) { - if ( a instanceof RemoveOperation ) { + if ( a.type == 'remove' ) { // Case 1: The element to remove got merged. // Merge operation does support merging elements which are not siblings. So it would not be a problem // from technical point of view. However, if the element was removed, the intention of the user // deleting it was to have it all deleted. From user experience point of view, moving back the // removed nodes might be unexpected. This means that in this scenario we will reverse merging and remove the element. // - if ( !context.wasUndone( a ) ) { + if ( !context.aWasUndone ) { return [ b.getReversed(), a ]; } } else { @@ -1081,7 +1299,7 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => { return [ a ]; } ); -setTransformation( MoveOperation, WrapOperation, ( a, b ) => { +setTransformation( MoveOperation, WrapOperation, ( a, b, context ) => { const moveRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); const newTargetPosition = a.targetPosition._getTransformedByWrapOperation( b ); @@ -1133,6 +1351,10 @@ setTransformation( MoveOperation, WrapOperation, ( a, b ) => { a.howMany = transformed.end.offset - transformed.start.offset; a.targetPosition = newTargetPosition; + if ( a.targetPosition.isEqual( b.position ) && context.abRelation == 'insertInside' ) { + a.targetPosition = b.targetPosition; + } + return [ a ]; } ); @@ -1156,6 +1378,13 @@ setTransformation( RenameOperation, InsertOperation, ( a, b ) => { } ); setTransformation( RenameOperation, MergeOperation, ( a, b ) => { + if ( a.position.isEqual( b.deletionPosition ) ) { + a.position = Position.createFromPosition( b.graveyardPosition ); + a.position.stickiness = 'toNext'; + + return [ a ]; + } + a.position = a.position._getTransformedByMergeOperation( b ); return [ a ]; @@ -1235,7 +1464,11 @@ setTransformation( RootAttributeOperation, RootAttributeOperation, ( a, b, conte // ----------------------- -setTransformation( SplitOperation, InsertOperation, ( a, b ) => { +setTransformation( SplitOperation, InsertOperation, ( a, b, context ) => { + if ( a.position.isEqual( b.position ) && context.baRelation == 'insertAtSource' ) { + return [ a ]; + } + a.position = a.position._getTransformedByInsertOperation( b ); return [ a ]; @@ -1251,7 +1484,7 @@ setTransformation( SplitOperation, MergeOperation, ( a, b ) => { return [ a ]; } ); -setTransformation( SplitOperation, MoveOperation, ( a, b ) => { +setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => { if ( a.graveyardPosition ) { a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b ); } @@ -1279,6 +1512,12 @@ setTransformation( SplitOperation, MoveOperation, ( a, b ) => { return [ a ]; } + if ( a.position.isEqual( b.targetPosition ) && context.abRelation == 'splitBefore' ) { + a.position = a.position._getTransformedByDeletion( b.sourcePosition, b.howMany ); + + return [ a ]; + } + // The default case. // a.position = a.position._getTransformedByMoveOperation( b ); @@ -1286,7 +1525,7 @@ setTransformation( SplitOperation, MoveOperation, ( a, b ) => { return [ a ]; } ); -setTransformation( SplitOperation, SplitOperation, ( a, b ) => { +setTransformation( SplitOperation, SplitOperation, ( a, b, context ) => { if ( a.position.isEqual( b.position ) ) { if ( !a.graveyardPosition && !b.graveyardPosition ) { return getNoOp(); @@ -1295,6 +1534,8 @@ setTransformation( SplitOperation, SplitOperation, ( a, b ) => { if ( a.graveyardPosition && b.graveyardPosition && a.graveyardPosition.isEqual( b.graveyardPosition ) ) { return getNoOp(); } + } else if ( a.position.isEqual( b.insertionPosition ) && context.abRelation == 'splitBefore' ) { + return [ a ]; } else { a.position = a.position._getTransformedBySplitOperation( b ); } @@ -1306,7 +1547,7 @@ setTransformation( SplitOperation, SplitOperation, ( a, b ) => { return [ a ]; } ); -setTransformation( SplitOperation, WrapOperation, ( a, b ) => { +setTransformation( SplitOperation, WrapOperation, ( a, b, context ) => { // Case 1: If split position has been wrapped, reverse the wrapping so that split can be applied as intended. // This is an edge case scenario where it is difficult to find a correct solution. // Since it will be a rare (or only theoretical) scenario, the algorithm will perform the easy solution. @@ -1324,20 +1565,33 @@ setTransformation( SplitOperation, WrapOperation, ( a, b ) => { return [ reversed, a ]; } - a.position = a.position._getTransformedByWrapOperation( b ); + if ( a.position.isEqual( b.position ) && context.abRelation == 'splitInside' ) { + a.position = b.targetPosition; + } else { + a.position = a.position._getTransformedByWrapOperation( b ); + } - if ( a.graveyardPosition && b.graveyardPosition ) { - a.graveyardPosition = a.graveyardPosition._getTransformedByDeletion( b.graveyardPosition, 1 ); + if ( a.graveyardPosition ) { + a.graveyardPosition = a.graveyardPosition._getTransformedByWrapOperation( b ); } return [ a ]; } ); -setTransformation( SplitOperation, UnwrapOperation, ( a, b ) => { - a.position = a.position._getTransformedByUnwrapOperation( b ); +setTransformation( SplitOperation, UnwrapOperation, ( a, b, context ) => { + const splitInside = a.position.hasSameParentAs( b.position ); + + if ( splitInside && !context.bWasUndone ) { + const path = b.graveyardPosition.path.slice(); + path.push( 0 ); + + a.position = new Position( b.graveyardPosition.root, path ); + } else { + a.position = a.position._getTransformedByUnwrapOperation( b ); + } if ( a.graveyardPosition ) { - a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 ); + a.graveyardPosition = a.graveyardPosition._getTransformedByUnwrapOperation( b ); } return [ a ]; @@ -1345,7 +1599,13 @@ setTransformation( SplitOperation, UnwrapOperation, ( a, b ) => { // ----------------------- -setTransformation( WrapOperation, InsertOperation, ( a, b ) => { +setTransformation( WrapOperation, InsertOperation, ( a, b, context ) => { + if ( a.position.isEqual( b.position ) && context.baRelation == 'insertInside' ) { + a.howMany += b.howMany; + + return [ a ]; + } + const transformed = a.wrappedRange._getTransformedByInsertOperation( b, false )[ 0 ]; a.position = transformed.start; @@ -1355,31 +1615,41 @@ setTransformation( WrapOperation, InsertOperation, ( a, b ) => { } ); setTransformation( WrapOperation, MergeOperation, ( a, b ) => { + if ( a.graveyardPosition ) { + a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 ); + } + + // Case 1: The element to wrap got merged. + // + if ( a.position.isEqual( b.deletionPosition ) ) { + a.position = Position.createFromPosition( b.graveyardPosition ); + a.position.stickiness = 'toNext'; + + return [ a ]; + } + const transformed = a.wrappedRange._getTransformedByMergeOperation( b ); a.position = transformed.start; a.howMany = transformed.end.offset - transformed.start.offset; - if ( a.graveyardPosition ) { - a.graveyardPosition = a.graveyardPosition._getTransformedByInsertion( b.graveyardPosition, 1 ); - } - return [ a ]; } ); -setTransformation( WrapOperation, MoveOperation, ( a, b ) => { +setTransformation( WrapOperation, MoveOperation, ( a, b, context ) => { if ( a.graveyardPosition ) { a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b ); } - const ranges = breakRangeByMoveOperation( a.wrappedRange, b, false ); - - if ( ranges.length == 0 ) { - const range = a.wrappedRange._getTransformedByMoveOperation( b )[ 0 ]; + if ( a.position.isEqual( b.targetPosition ) && context.baRelation == 'insertInside' ) { + a.position._getTransformedByDeletion( b.sourcePosition, b.howMany ); + a.howMany += b.howMany; - ranges.push( range ); + return [ a ]; } + const ranges = breakRangeByMoveOperation( a.wrappedRange, b, false ); + return ranges.reverse().map( range => { const howMany = range.end.offset - range.start.offset; const elementOrGraveyardPosition = a.graveyardPosition ? a.graveyardPosition : a.element; @@ -1388,20 +1658,25 @@ setTransformation( WrapOperation, MoveOperation, ( a, b ) => { } ); } ); -setTransformation( WrapOperation, SplitOperation, ( a, b ) => { - // Case 1: If range to wrap got split by split operation cancel the wrapping. +setTransformation( WrapOperation, SplitOperation, ( a, b, context ) => { + // Case 1: If range to wrap got split cancel the wrapping. + // Do that only if this is not undo mode. If `b` operation was earlier transformed by unwrap operation + // and the split position was inside the unwrapped range, then proceed without special case. // - if ( a.position.hasSameParentAs( b.position ) && a.wrappedRange.containsPosition( b.position ) ) { + const isInside = a.position.hasSameParentAs( b.position ) && a.wrappedRange.containsPosition( b.position ); + + if ( isInside && context.baRelation !== 'splitInside' ) { // We cannot just return no-op in this case, because in the mirror case scenario the wrap is reversed, which // might introduce a new node in the graveyard (if the wrap didn't have `graveyardPosition`, then the wrap // created a new element which was put to the graveyard when the wrap was reversed). // // Instead, a node in graveyard will be inserted. + // if ( a.element ) { const graveyard = a.position.root.document.graveyard; const graveyardPosition = new Position( graveyard, [ 0 ] ); - return new InsertOperation( graveyardPosition, a.element, 0 ); + return [ new InsertOperation( graveyardPosition, a.element, 0 ) ]; } else { return getNoOp(); } @@ -1609,11 +1884,21 @@ setTransformation( UnwrapOperation, MergeOperation, ( a, b, context ) => { return [ b.getReversed(), a ]; } - const transformed = a.unwrappedRange._getTransformedByMergeOperation( b ); + // // Case 2: The element to unwrap was merged-to and has new nodes. + // // + // if ( a.position.hasSameParentAs( b.targetPosition ) ) { + // // Merge operation needs `howMany`! + // } - a.position = transformed.start; - a.position.stickiness = 'toPrevious'; - a.howMany = transformed.end.offset - transformed.start.offset; + if ( a.position.hasSameParentAs( b.graveyardPosition ) ) { + a.howMany++; + } + + if ( a.position.hasSameParentAs( b.deletionPosition ) ) { + a.howMany--; + } + + a.position = a.position._getTransformedByMergeOperation( b ); if ( !a.graveyardPosition.isEqual( b.graveyardPosition ) || !context.aIsStrong ) { a.graveyardPosition = a.graveyardPosition._getTransformedByMergeOperation( b ); @@ -1622,7 +1907,7 @@ setTransformation( UnwrapOperation, MergeOperation, ( a, b, context ) => { return [ a ]; } ); -setTransformation( UnwrapOperation, MoveOperation, ( a, b, context ) => { +setTransformation( UnwrapOperation, MoveOperation, ( a, b ) => { // Case 1: Move operation moves nodes from the unwrapped element. // This does not have any impact on `UnwrapOperation#position`, but `#howMany` has to be changed. // @@ -1640,7 +1925,7 @@ setTransformation( UnwrapOperation, MoveOperation, ( a, b, context ) => { a.position = a.position._getTransformedByMoveOperation( b ); - if ( !a.graveyardPosition.isEqual( b.targetPosition ) || !context.aIsStrong ) { + if ( !a.graveyardPosition.isEqual( b.targetPosition ) ) { a.graveyardPosition = a.graveyardPosition._getTransformedByMoveOperation( b ); } @@ -1667,17 +1952,17 @@ setTransformation( UnwrapOperation, SplitOperation, ( a, b ) => { // Case 2: The split element is the last element in unwrapped element. In this case, we need to manually modify // `howMany` property because it wouldn't be correctly calculated by `_getTransformedBySplitOperation`. // - if ( b.insertionPosition.isEqual( a.position.getShiftedBy( a.howMany ) ) ) { + if ( a.position.hasSameParentAs( b.insertionPosition ) ) { a.howMany++; return [ a ]; } - const transformed = a.unwrappedRange._getTransformedBySplitOperation( b ); + a.position = a.position._getTransformedBySplitOperation( b ); - a.position = transformed.start; - a.position.stickiness = 'toPrevious'; - a.howMany = transformed.end.offset - transformed.start.offset; + if ( b.graveyardPosition && b.graveyardPosition.hasSameParentAs( a.position ) ) { + a.howMany--; + } a.graveyardPosition = a.graveyardPosition._getTransformedBySplitOperation( b ); @@ -1691,6 +1976,10 @@ setTransformation( UnwrapOperation, WrapOperation, ( a, b ) => { a.howMany = a.howMany - b.howMany + 1; } + if ( b.graveyardPosition && compareArrays( a.position.getParentPath(), b.graveyardPosition.path ) == 'same' ) { + a.howMany = b.howMany; + } + // The default case. // a.position = a.position._getTransformedByWrapOperation( b ); @@ -1709,6 +1998,8 @@ setTransformation( UnwrapOperation, UnwrapOperation, ( a, b, context ) => { a.position = new Position( b.graveyardPosition.root, path ); a.howMany = 0; a.graveyardPosition = Position.createFromPosition( b.graveyardPosition ); + + return [ a ]; } a.position = a.position._getTransformedByUnwrapOperation( b ); @@ -1759,33 +2050,19 @@ function makeMoveOperationsFromRanges( ranges, targetPosition ) { ranges[ j ] = ranges[ j ]._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany )[ 0 ]; } - // targetPosition.stickiness = 'toPrevious'; - targetPosition = targetPosition._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany, true ); + targetPosition = targetPosition._getTransformedByMove( op.sourcePosition, op.targetPosition, op.howMany ); } return operations; } function makeMoveOperation( range, targetPosition ) { - // We want to keep correct operation class. - let OperationClass; - - if ( targetPosition.root.rootName == '$graveyard' ) { - OperationClass = RemoveOperation; - } else if ( range.start.root.rootName == '$graveyard' ) { - OperationClass = ReinsertOperation; - } else { - OperationClass = MoveOperation; - } - targetPosition.stickiness = 'toNone'; - const result = new OperationClass( + return new MoveOperation( range.start, range.end.offset - range.start.offset, targetPosition, - 0 // Is corrected anyway later. + 0 ); - - return result; } diff --git a/src/model/operation/wrapoperation.js b/src/model/operation/wrapoperation.js index 5de2c6a68..ccf15314f 100644 --- a/src/model/operation/wrapoperation.js +++ b/src/model/operation/wrapoperation.js @@ -62,6 +62,10 @@ export default class WrapOperation extends Operation { this.element = elementOrPosition instanceof Element ? elementOrPosition : null; this.graveyardPosition = elementOrPosition instanceof Element ? null : Position.createFromPosition( elementOrPosition ); + + if ( this.graveyardPosition ) { + this.graveyardPosition.stickiness = 'toNext'; + } } /** @@ -148,7 +152,10 @@ export default class WrapOperation extends Operation { const targetPosition = new Position( this.position.root, targetPath ); if ( this.element ) { - _insert( insertPosition, this.element._clone() ); + const originalElement = this.element; + this.element = this.element._clone(); + + _insert( insertPosition, originalElement ); } else { _move( Range.createFromPositionAndShift( this.graveyardPosition, 1 ), insertPosition ); } diff --git a/src/model/position.js b/src/model/position.js index 13a2810d7..c8be10b7a 100644 --- a/src/model/position.js +++ b/src/model/position.js @@ -563,6 +563,8 @@ export default class Position { // Above happens during OT when the merged element is moved before the merged-to element. pos = pos._getTransformedByDeletion( operation.deletionPosition, 1 ); } + } else if ( this.isEqual( operation.deletionPosition ) ) { + pos = Position.createFromPosition( operation.deletionPosition ); } else { pos = this._getTransformedByMove( operation.deletionPosition, operation.graveyardPosition, 1 ); } @@ -599,15 +601,24 @@ export default class Position { unwrappedRange.start.isEqual( this ) || unwrappedRange.end.isEqual( this ); + let pos; + if ( isContained ) { - return this._getCombined( operation.position, operation.targetPosition ); + pos = this._getCombined( operation.position, operation.targetPosition ); } else if ( this.isEqual( operation.targetPosition ) ) { - return Position.createFromPosition( this ); + pos = Position.createFromPosition( this ); } else { - const pos = this._getTransformedByInsertion( operation.targetPosition, operation.howMany - 1 ); + pos = this._getTransformedByInsertion( operation.targetPosition, operation.howMany ); + } - return pos._getTransformedByInsertion( operation.graveyardPosition, 1 ); + const targetPosition = operation.targetPosition.getShiftedBy( operation.howMany ); + + if ( !targetPosition.isEqual( operation.graveyardPosition ) ) { + pos = pos._getTransformedByDeletion( targetPosition, 1 ); + pos = pos._getTransformedByInsertion( operation.graveyardPosition, 1 ); } + + return pos; } /** @@ -761,6 +772,7 @@ export default class Position { // The first part of a path to combined position is a path to the place where nodes were moved. const combined = Position.createFromPosition( target ); + combined.stickiness = this.stickiness; // Then we have to update the rest of the path. diff --git a/src/model/range.js b/src/model/range.js index 4dc795e90..30746b6af 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -493,16 +493,37 @@ export default class Range { } _getTransformedByMergeOperation( operation ) { - const start = this.start._getTransformedByMergeOperation( operation ); - const end = this.end._getTransformedByMergeOperation( operation ); + let start = this.start._getTransformedByMergeOperation( operation ); + let end = this.end._getTransformedByMergeOperation( operation ); if ( start.isAfter( end ) ) { - // This happens in the following case: + // This happens in the following two, similar cases: // - //

abc

{

x}yz

->

abcx}yz

{ - //

abcx{yz

} + // Case 1: Range start is directly before merged node. + // Resulting range should include only nodes from the merged element: // - return new Range( end, start ); + // Before:

aa

{

b}b

cc

+ // Merge:

aab}b

{

cc

+ // Fix:

aa{b}b

cc

+ // + // Case 2: Range start is not directly before merged node. + // Result should include all nodes that were in the original range. + // + // Before:

aa

{

cc

b}b

+ // Merge:

aab}b

{

cc

+ // Fix:

aa{bb

cc

} + // + // The range is expanded by an additional `b` letter but it is better than dropping the whole `cc` paragraph. + // + if ( !operation.deletionPosition.isEqual( start ) ) { + // Case 2. + end = operation.deletionPosition; + } + + // In both cases start is at the end of the merge-to element. + start = operation.targetPosition; + + return new Range( start, end ); } return new Range( start, end ); diff --git a/src/model/schema.js b/src/model/schema.js index 61cca5ff2..180ec353a 100644 --- a/src/model/schema.js +++ b/src/model/schema.js @@ -665,38 +665,54 @@ export default class Schema { * * @param {Array.} ranges Ranges to be validated. * @param {String} attribute The name of the attribute to check. - * @returns {Array.} Ranges in which the attribute is allowed. + * @returns {Iterator.} Ranges in which the attribute is allowed. */ - getValidRanges( ranges, attribute ) { - const validRanges = []; + * getValidRanges( ranges, attribute ) { + ranges = convertToMinimalFlatRanges( ranges ); for ( const range of ranges ) { - let last = range.start; - let from = range.start; - const to = range.end; - - for ( const value of range.getWalker() ) { - if ( !this.checkAttribute( value.item, attribute ) ) { - if ( !from.isEqual( last ) ) { - validRanges.push( new Range( from, last ) ); - } + yield* this._getValidRangesForRange( range, attribute ); + } + } - from = value.nextPosition; - } + /** + * Takes a flat range and an attribute name. Traverses the range recursively and deeply to find and return all ranges + * inside the given range on which the attribute can be applied. + * + * This is a helper function for {@link ~Schema#getValidRanges}. + * + * @private + * @param {module:engine/model/range~Range} range Range to process. + * @param {String} attribute The name of the attribute to check. + * @returns {Iterator.} Ranges in which the attribute is allowed. + */ + * _getValidRangesForRange( range, attribute ) { + let start = range.start; + let end = range.start; - last = value.nextPosition; + for ( const item of range.getItems( { shallow: true } ) ) { + if ( item.is( 'element' ) ) { + yield* this._getValidRangesForRange( Range.createIn( item ), attribute ); } - if ( from && !from.isEqual( to ) ) { - validRanges.push( new Range( from, to ) ); + if ( !this.checkAttribute( item, attribute ) ) { + if ( !start.isEqual( end ) ) { + yield new Range( start, end ); + } + + start = Position.createAfter( item ); } + + end = Position.createAfter( item ); } - return validRanges; + if ( !start.isEqual( end ) ) { + yield new Range( start, end ); + } } /** - * Basing on given the `position`, finds and returns a {@link module:engine/model/range~Range range} which is + * Basing on given `position`, finds and returns a {@link module:engine/model/range~Range range} which is * nearest to that `position` and is a correct range for selection. * * The correct selection range might be collapsed when it is located in a position where the text node can be placed. @@ -1572,3 +1588,14 @@ function* combineWalkers( backward, forward ) { } } } + +// Takes an array of non-intersecting ranges. For each of them gets minimal flat ranges covering that range and returns +// all those minimal flat ranges. +// +// @param {Array.} ranges Ranges to process. +// @returns {Iterator.} Minimal flat ranges of given `ranges`. +function* convertToMinimalFlatRanges( ranges ) { + for ( const range of ranges ) { + yield* range.getMinimalFlatRanges(); + } +} diff --git a/src/model/selection.js b/src/model/selection.js index 4a2bf2c29..8e1c989ac 100644 --- a/src/model/selection.js +++ b/src/model/selection.js @@ -431,7 +431,21 @@ export default class Selection { // Check whether there is any range in new ranges set that is different than all already added ranges. const anyNewRange = newRanges.some( newRange => { if ( !( newRange instanceof Range ) ) { - throw new CKEditorError( 'model-selection-added-not-range: Trying to add an object that is not an instance of Range.' ); + /** + * Selection range set to an object that is not an instance of {@link module:engine/model/range~Range}. + * + * Only {@link module:engine/model/range~Range} instances can be used to set a selection. + * Common mistakes leading to this error are: + * + * * using DOM `Range` object, + * * incorrect CKEditor 5 installation with multiple `ckeditor5-engine` packages having different versions. + * + * @error model-selection-set-ranges-not-range + */ + throw new CKEditorError( + 'model-selection-set-ranges-not-range: ' + + 'Selection range set to an object that is not an instance of model.Range.' + ); } return this._ranges.every( oldRange => { diff --git a/src/model/writer.js b/src/model/writer.js index 41000fa43..8b6e00043 100644 --- a/src/model/writer.js +++ b/src/model/writer.js @@ -12,7 +12,6 @@ import DetachOperation from './operation/detachoperation'; import InsertOperation from './operation/insertoperation'; import MarkerOperation from './operation/markeroperation'; import MoveOperation from './operation/moveoperation'; -import RemoveOperation from './operation/removeoperation'; import RenameOperation from './operation/renameoperation'; import RootAttributeOperation from './operation/rootattributeoperation'; import SplitOperation from './operation/splitoperation'; @@ -1330,7 +1329,7 @@ function applyMarkerOperation( writer, name, oldRange, newRange, affectsData ) { model.applyOperation( operation ); } -// Creates `RemoveOperation` or `DetachOperation` that removes `howMany` nodes starting from `position`. +// Creates `MoveOperation` or `DetachOperation` that removes `howMany` nodes starting from `position`. // The operation will be applied on given model instance and added to given operation instance. // // @private @@ -1345,7 +1344,7 @@ function applyRemoveOperation( position, howMany, batch, model ) { const doc = model.document; const graveyardPosition = new Position( doc.graveyard, [ 0 ] ); - operation = new RemoveOperation( position, howMany, graveyardPosition, doc.version ); + operation = new MoveOperation( position, howMany, graveyardPosition, doc.version ); } else { operation = new DetachOperation( position, howMany ); } diff --git a/src/view/selection.js b/src/view/selection.js index bd132dac8..e8b00e75a 100644 --- a/src/view/selection.js +++ b/src/view/selection.js @@ -643,7 +643,15 @@ export default class Selection { */ _addRange( range, isBackward = false ) { if ( !( range instanceof Range ) ) { - throw new CKEditorError( 'view-selection-invalid-range: Invalid Range.' ); + /** + * Selection range set to an object that is not an instance of {@link module:engine/view/range~Range}. + * + * @error view-selection-add-range-not-range + */ + throw new CKEditorError( + 'view-selection-add-range-not-range: ' + + 'Selection range set to an object that is not an instance of view.Range' + ); } this._pushRange( range ); diff --git a/tests/dev-utils/enableenginedebug.js b/tests/dev-utils/enableenginedebug.js index 4b586ac48..b646f3d6f 100644 --- a/tests/dev-utils/enableenginedebug.js +++ b/tests/dev-utils/enableenginedebug.js @@ -21,8 +21,6 @@ import MoveOperation from '../../src/model/operation/moveoperation'; import NoOperation from '../../src/model/operation/nooperation'; import RenameOperation from '../../src/model/operation/renameoperation'; import RootAttributeOperation from '../../src/model/operation/rootattributeoperation'; -import RemoveOperation from '../../src/model/operation/removeoperation'; -import transform from '../../src/model/operation/transform'; import Model from '../../src/model/model'; import ModelDocumentFragment from '../../src/model/documentfragment'; @@ -532,8 +530,8 @@ describe( 'debug tools', () => { model.applyOperation( insert ); const graveyard = modelDoc.graveyard; - const remove = new RemoveOperation( ModelPosition.createAt( modelRoot, 1 ), 2, ModelPosition.createAt( graveyard, 0 ), 1 ); - model.applyOperation( remove ); + const move = new MoveOperation( ModelPosition.createAt( modelRoot, 1 ), 2, ModelPosition.createAt( graveyard, 0 ), 1 ); + model.applyOperation( move ); } ); log.resetHistory(); @@ -668,39 +666,6 @@ describe( 'debug tools', () => { } ); } ); - describe( 'should provide error logging for transformation', () => { - let model, document, root, otherRoot; - - beforeEach( () => { - model = new Model(); - document = model.document; - root = document.createRoot(); - otherRoot = document.createRoot( 'other', 'other' ); - } ); - - it.skip( 'with more important operation A', () => { - const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 ); - const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 ); - - expect( () => { - transform.transform( opA, opB, { isStrong: true } ); - } ).to.throw( Error ); - expect( error.calledWith( opA.toString() + ' (important)' ) ).to.be.true; - expect( error.calledWith( opB.toString() ) ).to.be.true; - } ); - - it.skip( 'with more important operation B', () => { - const opA = new MoveOperation( ModelPosition.createAt( root, 4 ), 4, ModelPosition.createAt( otherRoot, 4 ), 0 ); - const opB = new InsertOperation( ModelPosition.createAt( root, 0 ), new ModelText( 'a' ), 0 ); - - testUtils.sinon.stub( transform, 'transform' ).throws( new Error() ); - - expect( () => transform.transform( opA, opB, { isStrong: true } ) ).to.throw( Error ); - expect( error.calledWith( opA.toString() ) ).to.be.true; - expect( error.calledWith( opB.toString() + ' (important)' ) ).to.be.true; - } ); - } ); - function expectLog( expectedLogMsg ) { expect( log.calledWithExactly( expectedLogMsg ) ).to.be.true; log.resetHistory(); diff --git a/tests/model/differ.js b/tests/model/differ.js index d04e3b54c..549146cae 100644 --- a/tests/model/differ.js +++ b/tests/model/differ.js @@ -10,7 +10,6 @@ import Position from '../../src/model/position'; import Range from '../../src/model/range'; import InsertOperation from '../../src/model/operation/insertoperation'; -import RemoveOperation from '../../src/model/operation/removeoperation'; import MoveOperation from '../../src/model/operation/moveoperation'; import RenameOperation from '../../src/model/operation/renameoperation'; import AttributeOperation from '../../src/model/operation/attributeoperation'; @@ -1617,7 +1616,7 @@ describe( 'Differ', () => { function remove( sourcePosition, howMany ) { const targetPosition = Position.createAt( doc.graveyard, doc.graveyard.maxOffset ); - const operation = new RemoveOperation( sourcePosition, howMany, targetPosition, doc.version ); + const operation = new MoveOperation( sourcePosition, howMany, targetPosition, doc.version ); model.applyOperation( operation ); } diff --git a/tests/model/documentselection.js b/tests/model/documentselection.js index 8a5a89852..a81f594a0 100644 --- a/tests/model/documentselection.js +++ b/tests/model/documentselection.js @@ -13,7 +13,6 @@ import LiveRange from '../../src/model/liverange'; import DocumentSelection from '../../src/model/documentselection'; import InsertOperation from '../../src/model/operation/insertoperation'; import MoveOperation from '../../src/model/operation/moveoperation'; -import RemoveOperation from '../../src/model/operation/removeoperation'; import AttributeOperation from '../../src/model/operation/attributeoperation'; import SplitOperation from '../../src/model/operation/splitoperation'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; @@ -334,7 +333,7 @@ describe( 'DocumentSelection', () => { it( 'should throw an error when range is invalid', () => { expect( () => { selection._setTo( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, /model-selection-added-not-range/ ); + } ).to.throw( CKEditorError, /model-selection-set-ranges-not-range/ ); } ); it( 'should log a warning when trying to set selection to the graveyard', () => { @@ -1131,12 +1130,12 @@ describe( 'DocumentSelection', () => { } ); } ); - describe( 'RemoveOperation', () => { + describe( 'MoveOperation to graveyard', () => { it( 'fix selection range if it ends up in graveyard #1', () => { selection._setTo( new Position( root, [ 1, 3 ] ) ); model.applyOperation( - new RemoveOperation( + new MoveOperation( new Position( root, [ 1, 2 ] ), 2, new Position( doc.graveyard, [ 0 ] ), @@ -1151,7 +1150,7 @@ describe( 'DocumentSelection', () => { selection._setTo( [ new Range( new Position( root, [ 1, 2 ] ), new Position( root, [ 1, 4 ] ) ) ] ); model.applyOperation( - new RemoveOperation( + new MoveOperation( new Position( root, [ 1, 2 ] ), 2, new Position( doc.graveyard, [ 0 ] ), @@ -1166,7 +1165,7 @@ describe( 'DocumentSelection', () => { selection._setTo( [ new Range( new Position( root, [ 1, 1 ] ), new Position( root, [ 1, 2 ] ) ) ] ); model.applyOperation( - new RemoveOperation( + new MoveOperation( new Position( root, [ 1 ] ), 2, new Position( doc.graveyard, [ 0 ] ), @@ -1179,7 +1178,7 @@ describe( 'DocumentSelection', () => { it( 'fix selection range if it ends up in graveyard #4 - whole content removed', () => { model.applyOperation( - new RemoveOperation( + new MoveOperation( new Position( root, [ 0 ] ), 3, new Position( doc.graveyard, [ 0 ] ), diff --git a/tests/model/liverange.js b/tests/model/liverange.js index 1c3a9ea92..dd74cf780 100644 --- a/tests/model/liverange.js +++ b/tests/model/liverange.js @@ -3,13 +3,14 @@ * 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'; import LiveRange from '../../src/model/liverange'; import Range from '../../src/model/range'; import Text from '../../src/model/text'; +import MoveOperation from '../../src/model/operation/moveoperation'; +import MergeOperation from '../../src/model/operation/mergeoperation'; import { stringify, setData } from '../../src/dev-utils/model'; describe( 'LiveRange', () => { @@ -90,7 +91,7 @@ describe( 'LiveRange', () => { range.detach(); } ); - it( 'should fire change:range event with proper data when its boundaries are changed', () => { + it( 'should fire change:range event with when its boundaries are changed', () => { const live = new LiveRange( new Position( root, [ 0, 1, 4 ] ), new Position( root, [ 0, 2, 2 ] ) ); const copy = Range.createFromRange( live ); @@ -99,15 +100,11 @@ describe( 'LiveRange', () => { const sourcePosition = new Position( root, [ 2 ] ); const targetPosition = new Position( root, [ 0 ] ); - const batch = new Batch(); - let op = null; - model.enqueueChange( batch, writer => { + model.change( writer => { const sourceRange = Range.createFromPositionAndShift( sourcePosition, 1 ); writer.move( sourceRange, targetPosition ); - - op = batch.operations[ 0 ]; } ); expect( spy.calledOnce ).to.be.true; @@ -115,11 +112,11 @@ describe( 'LiveRange', () => { // First parameter available in event should be a range that is equal to the live range before the live range changed. expect( spy.args[ 0 ][ 1 ].isEqual( copy ) ).to.be.true; - // Second parameter is the operation that changed the range. - expect( spy.args[ 0 ][ 2 ] ).to.equal( op ); + // Second parameter is null for operations that did not move the range into graveyard. + expect( spy.args[ 0 ][ 2 ].deletionPosition ).to.be.null; } ); - it( 'should fire change:content event with proper data when content inside the range has changed', () => { + it( 'should fire change:content event when content inside the range has changed', () => { const live = new LiveRange( new Position( root, [ 0, 1 ] ), new Position( root, [ 0, 3 ] ) ); const spy = sinon.spy(); @@ -127,15 +124,11 @@ describe( 'LiveRange', () => { const sourcePosition = new Position( root, [ 0, 2, 0 ] ); const targetPosition = new Position( root, [ 0, 4, 0 ] ); - const batch = new Batch(); - let op = null; - model.enqueueChange( batch, writer => { + model.change( writer => { const sourceRange = Range.createFromPositionAndShift( sourcePosition, 2 ); writer.move( sourceRange, targetPosition ); - - op = batch.operations[ 0 ]; } ); expect( spy.calledOnce ).to.be.true; @@ -143,8 +136,62 @@ describe( 'LiveRange', () => { // First parameter available in event should be a range that is equal to the live range before the live range changed. expect( spy.args[ 0 ][ 1 ].isEqual( live ) ).to.be.true; - // Second parameter is the operation that changed the range. - expect( spy.args[ 0 ][ 2 ] ).to.equal( op ); + // Second parameter is null for operations that did not move the range into graveyard. + expect( spy.args[ 0 ][ 2 ].deletionPosition ).to.be.null; + } ); + + it( 'should pass deletion position if range was removed (remove)', () => { + const live = new LiveRange( new Position( root, [ 0, 2 ] ), new Position( root, [ 0, 4 ] ) ); + + const spy = sinon.spy(); + live.on( 'change:range', spy ); + + const sourcePosition = new Position( root, [ 0, 0 ] ); + + model.change( writer => { + writer.remove( Range.createFromPositionAndShift( sourcePosition, 6 ) ); + } ); + + // Second parameter is deletion position. + expect( spy.args[ 0 ][ 2 ].deletionPosition.isEqual( sourcePosition ) ).to.be.true; + } ); + + // This scenario is hypothetically possible during OT if the element to merge-into was removed. + // In that case a live range inside the merged element will be merged into an element which is in graveyard. + // Because it may happen only in OT, in the test below we will generate operations by hand. + it( 'should pass deletion position if range was removed (merge)', () => { + const live = new LiveRange( new Position( root, [ 1, 0 ] ), new Position( root, [ 1, 1 ] ) ); + + const spy = sinon.spy(); + live.on( 'change:range', spy ); + + model.change( writer => { + const batch = writer.batch; + const gy = model.document.graveyard; + + const remove = new MoveOperation( + new Position( root, [ 0 ] ), + 1, + new Position( gy, [ 0 ] ), + model.document.version + ); + + const merge = new MergeOperation( + new Position( root, [ 0, 0 ] ), + new Position( gy, [ 0, 0 ] ), + new Position( gy, [ 0 ] ), + model.document.version + 1 + ); + + batch.addOperation( remove ); + model.applyOperation( remove ); + + batch.addOperation( merge ); + model.applyOperation( merge ); + } ); + + // Second parameter is deletion position. + expect( spy.args[ 1 ][ 2 ].deletionPosition.isEqual( new Position( root, [ 0 ] ) ) ).to.be.true; } ); describe( 'should get transformed and fire change:range if', () => { diff --git a/tests/model/operation/insertoperation.js b/tests/model/operation/insertoperation.js index 6f59cf3dc..92d7749a1 100644 --- a/tests/model/operation/insertoperation.js +++ b/tests/model/operation/insertoperation.js @@ -7,7 +7,7 @@ import Model from '../../../src/model/model'; import NodeList from '../../../src/model/nodelist'; import Element from '../../../src/model/element'; import InsertOperation from '../../../src/model/operation/insertoperation'; -import RemoveOperation from '../../../src/model/operation/removeoperation'; +import MoveOperation from '../../../src/model/operation/moveoperation'; import Position from '../../../src/model/position'; import Text from '../../../src/model/text'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; @@ -107,7 +107,7 @@ describe( 'InsertOperation', () => { expect( root.getChild( 0 ).data ).to.equal( 'fooxbar' ); } ); - it( 'should create a RemoveOperation as a reverse', () => { + it( 'should create a MoveOperation as a reverse', () => { const position = new Position( root, [ 0 ] ); const operation = new InsertOperation( position, @@ -117,7 +117,7 @@ describe( 'InsertOperation', () => { const reverse = operation.getReversed(); - expect( reverse ).to.be.an.instanceof( RemoveOperation ); + expect( reverse ).to.be.an.instanceof( MoveOperation ); expect( reverse.baseVersion ).to.equal( 1 ); expect( reverse.sourcePosition.isEqual( position ) ).to.be.true; expect( reverse.howMany ).to.equal( 7 ); diff --git a/tests/model/operation/moveoperation.js b/tests/model/operation/moveoperation.js index d2ff1edde..6e3a86a00 100644 --- a/tests/model/operation/moveoperation.js +++ b/tests/model/operation/moveoperation.js @@ -12,23 +12,27 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import { jsonParseStringify } from '../../../tests/model/_utils/utils'; describe( 'MoveOperation', () => { - let model, doc, root; + let model, doc, root, gy; beforeEach( () => { model = new Model(); doc = model.document; root = doc.createRoot(); + gy = doc.graveyard; } ); it( 'should have proper type', () => { - const op = new MoveOperation( - new Position( root, [ 0, 0 ] ), - 1, - new Position( root, [ 1, 0 ] ), - doc.version - ); + const move = new MoveOperation( new Position( root, [ 0, 0 ] ), 1, new Position( root, [ 1, 0 ] ), 0 ); + expect( move.type ).to.equal( 'move' ); + + const remove1 = new MoveOperation( new Position( root, [ 0, 0 ] ), 1, new Position( gy, [ 0 ] ), 0 ); + expect( remove1.type ).to.equal( 'remove' ); + + const remove2 = new MoveOperation( new Position( gy, [ 0 ] ), 1, new Position( gy, [ 1 ] ), 0 ); + expect( remove2.type ).to.equal( 'remove' ); - expect( op.type ).to.equal( 'move' ); + const reinsert = new MoveOperation( new Position( gy, [ 0 ] ), 1, new Position( root, [ 0, 0 ] ), 0 ); + expect( reinsert.type ).to.equal( 'reinsert' ); } ); it( 'should move from one node to another', () => { diff --git a/tests/model/operation/reinsertoperation.js b/tests/model/operation/reinsertoperation.js deleted file mode 100644 index da927dfc2..000000000 --- a/tests/model/operation/reinsertoperation.js +++ /dev/null @@ -1,157 +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 ReinsertOperation from '../../../src/model/operation/reinsertoperation'; -import RemoveOperation from '../../../src/model/operation/removeoperation'; -import MoveOperation from '../../../src/model/operation/moveoperation'; -import Position from '../../../src/model/position'; -import DocumentFragment from '../../../src/model/documentfragment'; -import Element from '../../../src/model/element'; -import Text from '../../../src/model/text'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -import { jsonParseStringify } from '../../../tests/model/_utils/utils'; - -describe( 'ReinsertOperation', () => { - let model, doc, root, graveyard, operation, graveyardPosition, rootPosition; - - beforeEach( () => { - model = new Model(); - doc = model.document; - root = doc.createRoot(); - graveyard = doc.graveyard; - - graveyardPosition = new Position( graveyard, [ 0 ] ); - rootPosition = new Position( root, [ 0 ] ); - - operation = new ReinsertOperation( - graveyardPosition, - 2, - rootPosition, - doc.version - ); - } ); - - it( 'should have position property equal to the position where node will be reinserted', () => { - expect( operation.position.isEqual( rootPosition ) ).to.be.true; - - // Setting also works: - operation.position = new Position( root, [ 1 ] ); - expect( operation.position.isEqual( new Position( root, [ 1 ] ) ) ).to.be.true; - } ); - - it( 'should have proper type', () => { - expect( operation.type ).to.equal( 'reinsert' ); - } ); - - it( 'should extend MoveOperation class', () => { - expect( operation ).to.be.instanceof( MoveOperation ); - } ); - - it( 'should create ReinsertOperation with same parameters when cloned', () => { - const clone = operation.clone(); - - expect( clone ).to.be.instanceof( ReinsertOperation ); - expect( clone.sourcePosition.isEqual( operation.sourcePosition ) ).to.be.true; - expect( clone.targetPosition.isEqual( operation.targetPosition ) ).to.be.true; - expect( clone.howMany ).to.equal( operation.howMany ); - expect( clone.baseVersion ).to.equal( operation.baseVersion ); - } ); - - it( 'should create RemoveOperation as a reverse', () => { - graveyard._appendChild( new Element( 'x' ) ); - - const reverse = operation.getReversed(); - - expect( reverse ).to.be.an.instanceof( RemoveOperation ); - expect( reverse.baseVersion ).to.equal( 1 ); - expect( reverse.howMany ).to.equal( 2 ); - expect( reverse.sourcePosition.isEqual( rootPosition ) ).to.be.true; - expect( reverse.targetPosition.isEqual( graveyardPosition ) ).to.be.true; - } ); - - it( 'should create correct RemoveOperation when reversed if target position was in graveyard', () => { - const operation = new ReinsertOperation( new Position( doc.graveyard, [ 0 ] ), 1, new Position( doc.graveyard, [ 3 ] ), 0 ); - const reverse = operation.getReversed(); - - expect( reverse.sourcePosition.path ).to.deep.equal( [ 2 ] ); - expect( reverse.targetPosition.path ).to.deep.equal( [ 0 ] ); - } ); - - it( 'should undo reinsert set of nodes by applying reverse operation', () => { - const reverse = operation.getReversed(); - - graveyard._insertChild( 0, new Text( 'xx' ) ); - - model.applyOperation( operation ); - - expect( doc.version ).to.equal( 1 ); - expect( root.maxOffset ).to.equal( 2 ); - expect( graveyard.maxOffset ).to.equal( 0 ); - - model.applyOperation( reverse ); - - expect( doc.version ).to.equal( 2 ); - expect( root.maxOffset ).to.equal( 0 ); - expect( graveyard.maxOffset ).to.equal( 2 ); - } ); - - describe( '_validate()', () => { - it( 'should throw when target position is not in the document', () => { - const docFrag = new DocumentFragment(); - - graveyard._insertChild( 0, new Text( 'xx' ) ); - - operation = new ReinsertOperation( - graveyardPosition, - 1, - Position.createAt( docFrag ), - doc.version - ); - - expect( () => { - operation._validate(); - } ).to.throw( CKEditorError, /^reinsert-operation-to-detached-parent/ ); - } ); - - it( 'should throw when source position is not in the document', () => { - const docFrag = new DocumentFragment( new Text( 'xx' ) ); - - operation = new ReinsertOperation( - Position.createAt( docFrag ), - 1, - rootPosition, - doc.version - ); - - expect( () => { - operation._validate(); - } ).to.throw( CKEditorError, /^reinsert-operation-on-detached-item/ ); - } ); - } ); - - describe( 'toJSON', () => { - it( 'should create proper json object', () => { - const serialized = jsonParseStringify( operation ); - - expect( serialized ).to.deep.equal( { - __className: 'engine.model.operation.ReinsertOperation', - baseVersion: 0, - howMany: 2, - sourcePosition: jsonParseStringify( operation.sourcePosition ), - targetPosition: jsonParseStringify( operation.targetPosition ) - } ); - } ); - } ); - - describe( 'fromJSON', () => { - it( 'should create proper ReinsertOperation from json object', () => { - const serialized = jsonParseStringify( operation ); - const deserialized = ReinsertOperation.fromJSON( serialized, doc ); - - expect( deserialized ).to.deep.equal( operation ); - } ); - } ); -} ); diff --git a/tests/model/operation/removeoperation.js b/tests/model/operation/removeoperation.js deleted file mode 100644 index da9bf72b1..000000000 --- a/tests/model/operation/removeoperation.js +++ /dev/null @@ -1,191 +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 ReinsertOperation from '../../../src/model/operation/reinsertoperation'; -import RemoveOperation from '../../../src/model/operation/removeoperation'; -import MoveOperation from '../../../src/model/operation/moveoperation'; -import Position from '../../../src/model/position'; -import DocumentFragment from '../../../src/model/documentfragment'; -import Element from '../../../src/model/element'; -import Text from '../../../src/model/text'; -import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; -import { jsonParseStringify } from '../../../tests/model/_utils/utils'; - -describe( 'RemoveOperation', () => { - let model, doc, root, graveyard; - - beforeEach( () => { - model = new Model(); - doc = model.document; - root = doc.createRoot(); - graveyard = doc.graveyard; - } ); - - it( 'should have proper type', () => { - const op = new RemoveOperation( - new Position( root, [ 2 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ); - - expect( op.type ).to.equal( 'remove' ); - } ); - - it( 'should extend MoveOperation class', () => { - const operation = new RemoveOperation( - new Position( root, [ 2 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ); - - expect( operation ).to.be.instanceof( MoveOperation ); - } ); - - it( 'should be able to remove set of nodes and append them to graveyard root', () => { - root._insertChild( 0, new Text( 'fozbar' ) ); - - model.applyOperation( - new RemoveOperation( - new Position( root, [ 2 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ) - ); - - expect( doc.version ).to.equal( 1 ); - expect( root.maxOffset ).to.equal( 4 ); - expect( root.getChild( 0 ).data ).to.equal( 'foar' ); - - expect( graveyard.maxOffset ).to.equal( 2 ); - expect( graveyard.getChild( 0 ).data ).to.equal( 'zb' ); - } ); - - it( 'should create RemoveOperation with same parameters when cloned', () => { - const pos = new Position( root, [ 2 ] ); - - const operation = new RemoveOperation( pos, 2, new Position( doc.graveyard, [ 0 ] ), doc.version ); - const clone = operation.clone(); - - expect( clone ).to.be.instanceof( RemoveOperation ); - expect( clone.sourcePosition.isEqual( pos ) ).to.be.true; - expect( clone.targetPosition.isEqual( operation.targetPosition ) ).to.be.true; - expect( clone.howMany ).to.equal( operation.howMany ); - expect( clone.baseVersion ).to.equal( operation.baseVersion ); - } ); - - it( 'should create ReinsertOperation when reversed', () => { - const position = new Position( root, [ 0 ] ); - const operation = new RemoveOperation( position, 2, new Position( doc.graveyard, [ 0 ] ), 0 ); - const reverse = operation.getReversed(); - - expect( reverse ).to.be.an.instanceof( ReinsertOperation ); - expect( reverse.baseVersion ).to.equal( 1 ); - expect( reverse.howMany ).to.equal( 2 ); - expect( reverse.sourcePosition.isEqual( operation.targetPosition ) ).to.be.true; - expect( reverse.targetPosition.isEqual( position ) ).to.be.true; - } ); - - it( 'should create correct ReinsertOperation when reversed if source range was in graveyard', () => { - const operation = new RemoveOperation( new Position( doc.graveyard, [ 2 ] ), 1, new Position( doc.graveyard, [ 0 ] ), 0 ); - const reverse = operation.getReversed(); - - expect( reverse.sourcePosition.path ).to.deep.equal( [ 0 ] ); - expect( reverse.targetPosition.path ).to.deep.equal( [ 3 ] ); - } ); - - it( 'should undo remove set of nodes by applying reverse operation', () => { - const position = new Position( root, [ 0 ] ); - const operation = new RemoveOperation( position, 3, new Position( doc.graveyard, [ 0 ] ), 0 ); - const reverse = operation.getReversed(); - - root._insertChild( 0, new Text( 'bar' ) ); - - model.applyOperation( operation ); - - expect( doc.version ).to.equal( 1 ); - expect( root.maxOffset ).to.equal( 0 ); - - model.applyOperation( reverse ); - - expect( doc.version ).to.equal( 2 ); - expect( root.maxOffset ).to.equal( 3 ); - expect( root.getChild( 0 ).data ).to.equal( 'bar' ); - } ); - - it( 'should properly remove a node that is already in a graveyard', () => { - doc.graveyard._appendChild( [ new Element( 'x' ), new Element( 'y' ), new Element( 'z' ) ] ); - - const position = new Position( doc.graveyard, [ 2 ] ); - const operation = new RemoveOperation( position, 1, new Position( doc.graveyard, [ 0 ] ), 0 ); - - model.applyOperation( operation ); - - expect( doc.graveyard.childCount ).to.equal( 3 ); - expect( doc.graveyard.getChild( 0 ).name ).to.equal( 'z' ); - expect( doc.graveyard.getChild( 1 ).name ).to.equal( 'x' ); - expect( doc.graveyard.getChild( 2 ).name ).to.equal( 'y' ); - } ); - - describe( '_validate()', () => { - it( 'should throw when is executed on detached item', () => { - const docFrag = new DocumentFragment(); - const item = new Element( 'foo' ); - - docFrag._appendChild( [ item ] ); - - const op = new RemoveOperation( - new Position( docFrag, [ 0 ] ), - 1, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ); - - expect( () => { - op._validate(); - } ).to.throw( CKEditorError, /^remove-operation-on-detached-item/ ); - } ); - } ); - - describe( 'toJSON', () => { - it( 'should create proper json object', () => { - const op = new RemoveOperation( - new Position( root, [ 2 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ); - - const serialized = jsonParseStringify( op ); - - expect( serialized ).to.deep.equal( { - __className: 'engine.model.operation.RemoveOperation', - baseVersion: 0, - howMany: 2, - sourcePosition: jsonParseStringify( op.sourcePosition ), - targetPosition: jsonParseStringify( op.targetPosition ) - } ); - } ); - } ); - - describe( 'fromJSON', () => { - it( 'should create proper RemoveOperation from json object', () => { - const op = new RemoveOperation( - new Position( root, [ 2 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - doc.version - ); - - const serialized = jsonParseStringify( op ); - const deserialized = RemoveOperation.fromJSON( serialized, doc ); - - expect( deserialized ).to.deep.equal( op ); - } ); - } ); -} ); diff --git a/tests/model/operation/transform.js b/tests/model/operation/transform.js index fc1f222f6..f7903230c 100644 --- a/tests/model/operation/transform.js +++ b/tests/model/operation/transform.js @@ -3,7 +3,7 @@ * For licensing, see LICENSE.md. */ -import transform from '../../../src/model/operation/transform'; +import { transform } from '../../../src/model/operation/transform'; import Model from '../../../src/model/model'; import RootElement from '../../../src/model/rootelement'; @@ -16,10 +16,11 @@ import AttributeOperation from '../../../src/model/operation/attributeoperation' import RootAttributeOperation from '../../../src/model/operation/rootattributeoperation'; import MarkerOperation from '../../../src/model/operation/markeroperation'; import MoveOperation from '../../../src/model/operation/moveoperation'; -import RemoveOperation from '../../../src/model/operation/removeoperation'; import RenameOperation from '../../../src/model/operation/renameoperation'; import NoOperation from '../../../src/model/operation/nooperation'; +import log from '@ckeditor/ckeditor5-utils/src/log'; + describe( 'transform', () => { let model, doc, root, op, nodeA, nodeB, expected; @@ -53,6 +54,39 @@ describe( 'transform', () => { } } + const strongContext = { + aIsStrong: true + }; + + it( 'error logging', () => { + const spy = sinon.spy( log, 'error' ); + + const nodeA = new Node(); + const nodeB = new Node(); + + const position = new Position( root, [ 0 ] ); + + const a = new InsertOperation( position, [ nodeA ], 0 ); + const b = new InsertOperation( position, [ nodeB ], 0 ); + + // Modify an operation so it will throw an error. + a.position = null; + + expect( () => { + transform( a, b, { + aIsStrong: true, + aWasUndone: false, + bWasUndone: false, + abRelation: null, + baRelation: null + } ); + } ).to.throw(); + + sinon.assert.called( spy ); + + log.error.restore(); + } ); + describe( 'InsertOperation', () => { let nodeC, nodeD, position; @@ -78,7 +112,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -91,7 +125,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -105,7 +139,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -119,7 +153,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -132,7 +166,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -146,7 +180,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -159,7 +193,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.path[ 1 ] += 2; expect( transOp.length ).to.equal( 1 ); @@ -173,7 +207,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -190,7 +224,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -207,7 +241,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -223,7 +257,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -237,7 +271,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset--; expect( transOp.length ).to.equal( 1 ); @@ -252,7 +286,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -266,7 +300,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -281,7 +315,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -295,7 +329,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -310,7 +344,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expected.position.offset += 2; expect( transOp.length ).to.equal( 1 ); @@ -325,7 +359,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.path[ 1 ] -= 1; expect( transOp.length ).to.equal( 1 ); @@ -340,7 +374,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -354,7 +388,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.path[ 1 ] += 2; expect( transOp.length ).to.equal( 1 ); @@ -369,7 +403,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -383,7 +417,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.path = [ 1, 2, 2 ]; expect( transOp.length ).to.equal( 1 ); @@ -398,7 +432,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.position.path = [ 1, 2 ]; expect( transOp.length ).to.equal( 1 ); @@ -410,7 +444,7 @@ describe( 'transform', () => { it( 'no operation update', () => { const transformBy = new NoOperation( 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -421,7 +455,7 @@ describe( 'transform', () => { it( 'no position update', () => { const transformBy = new RenameOperation( new Position( root, [ 0, 2, 0 ] ), 'oldName', 'newName', 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -433,7 +467,7 @@ describe( 'transform', () => { const newRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 4 ] ) ); const transformBy = new MarkerOperation( 'name', null, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -469,7 +503,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.offset += 2; expected.range.end.offset += 2; @@ -485,7 +519,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.offset += 2; expected.range.end.offset += 2; @@ -504,7 +538,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.offset--; expected.range.end.offset--; @@ -521,7 +555,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.offset += 2; expected.range.end.offset += 2; @@ -538,7 +572,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -560,7 +594,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -583,7 +617,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.path = [ 2, 4, 2, 1 ]; expected.range.end.path = [ 2, 4, 2, 4 ]; @@ -600,7 +634,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 3 ); @@ -628,7 +662,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.range.start.path = [ 2, 4, 1 ]; expected.range.end.path = [ 2, 4, 4 ]; @@ -645,7 +679,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -667,7 +701,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 3 ); @@ -676,8 +710,8 @@ describe( 'transform', () => { expectOperation( transOp[ 0 ], expected ); - expected.range.start.path = [ 0, 2, 1 ]; - expected.range.end.path = [ 0, 2, 2 ]; + expected.range.start.path = [ 0, 2, 3 ]; + expected.range.end.path = [ 0, 2, 4 ]; expectOperation( transOp[ 1 ], expected ); @@ -687,56 +721,6 @@ describe( 'transform', () => { expectOperation( transOp[ 2 ], expected ); } ); } ); - - describe( 'by RemoveOperation', () => { - beforeEach( () => { - start = new Position( doc.graveyard, [ 2, 0 ] ); - end = new Position( doc.graveyard, [ 2, 4 ] ); - - range = new Range( start, end ); - - op = new AttributeOperation( range, 'foo', 'abc', 'bar', 0 ); - - expected.range = new Range( start, end ); - } ); - - it( 'remove operation inserted elements before attribute operation range: increment path', () => { - const transformBy = new RemoveOperation( - new Position( root, [ 0 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - 0 - ); - - transformBy.targetPosition.path = [ 0 ]; - - const transOp = transform.transform( op, transformBy ); - - expect( transOp.length ).to.equal( 1 ); - - expected.range.start.path = [ 4, 0 ]; - expected.range.end.path = [ 4, 4 ]; - - expectOperation( transOp[ 0 ], expected ); - } ); - - it( 'remove operation inserted elements after attribute operation range: do nothing', () => { - const transformBy = new RemoveOperation( - new Position( root, [ 0 ] ), - 2, - new Position( doc.graveyard, [ 0 ] ), - 0 - ); - - transformBy.targetPosition.path = [ 4 ]; - - const transOp = transform.transform( op, transformBy ); - - expect( transOp.length ).to.equal( 1 ); - - expectOperation( transOp[ 0 ], expected ); - } ); - } ); } ); describe( 'RootAttributeOperation', () => { @@ -761,7 +745,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -781,7 +765,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -798,7 +782,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -813,7 +797,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -828,7 +812,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -845,7 +829,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -862,7 +846,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expected.oldValue = 'xyz'; @@ -880,7 +864,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -891,7 +875,7 @@ describe( 'transform', () => { it( 'no operation update', () => { const transformBy = new NoOperation( 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -902,7 +886,7 @@ describe( 'transform', () => { it( 'no position update', () => { const transformBy = new RenameOperation( new Position( root, [ 0 ] ), 'oldName', 'newName', 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -914,7 +898,7 @@ describe( 'transform', () => { const newRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 8 ] ) ); const transformBy = new MarkerOperation( 'name', null, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -951,7 +935,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -964,7 +948,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -977,7 +961,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.offset += 2; @@ -992,7 +976,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1005,7 +989,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.path[ 1 ] += 2; @@ -1020,7 +1004,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1033,7 +1017,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.offset += 2; @@ -1048,7 +1032,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1061,7 +1045,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.path[ 1 ] += 2; @@ -1076,7 +1060,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1089,7 +1073,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1102,7 +1086,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1115,7 +1099,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -1131,7 +1115,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1148,7 +1132,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1165,7 +1149,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1181,7 +1165,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1195,7 +1179,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.offset += 2; @@ -1211,7 +1195,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1225,7 +1209,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.offset -= 2; @@ -1241,7 +1225,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1255,7 +1239,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.path[ 1 ] += 2; @@ -1271,7 +1255,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1285,7 +1269,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.path[ 1 ] -= 1; @@ -1301,7 +1285,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1315,7 +1299,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.offset += 2; @@ -1331,7 +1315,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1345,7 +1329,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.offset -= 2; @@ -1361,7 +1345,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1375,7 +1359,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.path[ 1 ] += 2; @@ -1391,7 +1375,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1405,7 +1389,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.path[ 1 ] -= 2; @@ -1421,7 +1405,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1435,7 +1419,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -1455,7 +1439,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -1472,7 +1456,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1486,7 +1470,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -1500,7 +1484,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.path = [ 4, 3, 4 ]; @@ -1516,7 +1500,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.targetPosition.path = [ 0, 2, 3 ]; @@ -1532,7 +1516,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); const reversed = transformBy.getReversed(); expected.sourcePosition = reversed.sourcePosition; @@ -1551,7 +1535,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -1567,7 +1551,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expected.sourcePosition.path = [ 4, 1, 0 ]; @@ -1583,7 +1567,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -1599,7 +1583,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expected.sourcePosition.path = [ 4, 1, 1 ]; @@ -1617,7 +1601,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -1637,7 +1621,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); @@ -1655,7 +1639,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.sourcePosition.path = [ 2, 2, 3 ]; expected.howMany = 1; @@ -1675,7 +1659,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 2 ); @@ -1698,7 +1682,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.howMany = 1; @@ -1714,7 +1698,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 2 ); @@ -1739,7 +1723,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -1765,7 +1749,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 2 ); @@ -1789,7 +1773,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.howMany = 1; @@ -1805,7 +1789,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 2 ); @@ -1830,7 +1814,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 2 ); @@ -1856,7 +1840,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -1877,7 +1861,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 2 ); @@ -1901,7 +1885,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 3 ); @@ -1933,7 +1917,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -1952,7 +1936,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); @@ -1962,11 +1946,11 @@ describe( 'transform', () => { } ); } ); - describe( 'by RemoveOperation', () => { + describe( 'by MoveOperation to graveyard', () => { let transformBy; beforeEach( () => { - transformBy = new RemoveOperation( + transformBy = new MoveOperation( Position.createFromPosition( op.sourcePosition ), op.howMany, new Position( doc.graveyard, [ 0 ] ), @@ -1974,8 +1958,8 @@ describe( 'transform', () => { ); } ); - it( 'should skip context.aIsStrong and be less important than RemoveOperation', () => { - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + it( 'should skip context.aIsStrong and be less important than MoveOperation', () => { + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); @@ -1989,7 +1973,7 @@ describe( 'transform', () => { it( 'no operation update', () => { const transformBy = new NoOperation( 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2000,7 +1984,7 @@ describe( 'transform', () => { it( 'no position update', () => { const transformBy = new RenameOperation( new Position( root, [ 2, 2, 4 ] ), 'oldName', 'newName', 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2012,7 +1996,7 @@ describe( 'transform', () => { const newRange = new Range( new Position( root, [ 2, 2, 3 ] ), new Position( root, [ 2, 2, 8 ] ) ); const transformBy = new MarkerOperation( 'name', null, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2020,10 +2004,10 @@ describe( 'transform', () => { } ); } ); - describe( 'RemoveOperation', () => { + describe( 'MoveOperation to graveyard', () => { describe( 'by MoveOperation', () => { it( 'should force removing content even if was less important', () => { - const op = new RemoveOperation( new Position( root, [ 8 ] ), 2, new Position( doc.graveyard, [ 0 ] ), 0 ); + const op = new MoveOperation( new Position( root, [ 8 ] ), 2, new Position( doc.graveyard, [ 0 ] ), 0 ); const targetPosition = Position.createFromPosition( op.targetPosition ); @@ -2031,12 +2015,12 @@ describe( 'transform', () => { const sourcePosition = Position.createFromPosition( transformBy.targetPosition ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { - type: RemoveOperation, + type: MoveOperation, howMany: 2, sourcePosition, targetPosition @@ -2062,7 +2046,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2082,7 +2066,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2099,7 +2083,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2115,7 +2099,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2126,7 +2110,7 @@ describe( 'transform', () => { it( 'no operation update', () => { const transformBy = new NoOperation( 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2137,7 +2121,7 @@ describe( 'transform', () => { it( 'no position update', () => { const transformBy = new RenameOperation( new Position( root, [ 0, 2, 0 ] ), 'oldName', 'newName', 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2149,7 +2133,7 @@ describe( 'transform', () => { const newRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 8 ] ) ); const transformBy = new MarkerOperation( 'name', null, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2178,7 +2162,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2194,7 +2178,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2207,7 +2191,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2223,7 +2207,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2243,7 +2227,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2260,7 +2244,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2272,7 +2256,7 @@ describe( 'transform', () => { const newRange = new Range( new Position( root, [ 0, 2, 0 ] ), new Position( root, [ 0, 2, 8 ] ) ); const transformBy = new MarkerOperation( 'name', null, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2288,7 +2272,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2302,7 +2286,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -2318,7 +2302,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expect( transOp.length ).to.equal( 1 ); @@ -2336,7 +2320,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2353,7 +2337,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2370,7 +2354,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2387,7 +2371,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2404,7 +2388,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2421,7 +2405,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); @@ -2453,7 +2437,7 @@ describe( 'transform', () => { op.newRange = null; const transformBy = new InsertOperation( Position.createAt( root, 0 ), [ nodeA, nodeB ], 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.newRange = null; expected.oldRange.start.offset = 3; @@ -2468,7 +2452,7 @@ describe( 'transform', () => { op.oldRange = null; const transformBy = new InsertOperation( Position.createAt( root, 8 ), [ nodeA, nodeB ], 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.oldRange = null; expected.newRange.start.offset = 12; @@ -2492,7 +2476,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2505,7 +2489,7 @@ describe( 'transform', () => { op.newRange = null; const transformBy = new MoveOperation( Position.createAt( root, 0 ), 1, Position.createAt( root, 20 ), 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.newRange = null; expected.oldRange.start.offset = 0; @@ -2517,7 +2501,7 @@ describe( 'transform', () => { it( 'moved range contains oldRange and is before newRange: update oldRange and newRange', () => { const transformBy = new MoveOperation( Position.createAt( root, 2 ), 2, Position.createAt( root, 20 ), 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.oldRange.start.offset = 1; expected.oldRange.end.offset = 2; @@ -2533,7 +2517,7 @@ describe( 'transform', () => { op.oldRange = null; const transformBy = new MoveOperation( Position.createAt( root, 20 ), 2, Position.createAt( root, 11 ), 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.oldRange = null; expected.newRange.start.offset = 10; @@ -2545,7 +2529,7 @@ describe( 'transform', () => { it( 'target position is inside oldRange and before newRange: update oldRange and newRange', () => { const transformBy = new MoveOperation( Position.createAt( root, 20 ), 4, Position.createAt( root, 2 ), 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expected.oldRange.start.offset = 1; expected.oldRange.end.offset = 8; @@ -2567,7 +2551,7 @@ describe( 'transform', () => { 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2578,7 +2562,7 @@ describe( 'transform', () => { it( 'no operation update', () => { const transformBy = new RenameOperation( new Position( root, [ 1 ] ), 'oldName', 'newName', 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2589,7 +2573,7 @@ describe( 'transform', () => { it( 'different marker name: no operation update', () => { const transformBy = new MarkerOperation( 'otherName', oldRange, newRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], expected ); @@ -2599,7 +2583,7 @@ describe( 'transform', () => { const anotherRange = Range.createFromParentsAndOffsets( root, 2, root, 2 ); const transformBy = new MarkerOperation( 'name', oldRange, anotherRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy ); + const transOp = transform( op, transformBy ); expect( transOp.length ).to.equal( 1 ); expectOperation( transOp[ 0 ], { @@ -2611,7 +2595,7 @@ describe( 'transform', () => { const anotherRange = Range.createFromParentsAndOffsets( root, 2, root, 2 ); const transformBy = new MarkerOperation( 'name', oldRange, anotherRange, model.markers, 0 ); - const transOp = transform.transform( op, transformBy, { aIsStrong: true } ); + const transOp = transform( op, transformBy, strongContext ); expected.oldRange = anotherRange; diff --git a/tests/model/operation/transform/attribute.js b/tests/model/operation/transform/attribute.js index 6a5d0f775..f67e237e1 100644 --- a/tests/model/operation/transform/attribute.js +++ b/tests/model/operation/transform/attribute.js @@ -325,7 +325,7 @@ describe( 'transform', () => { kate.setData( 'F[oo] Bar' ); john.setAttribute( 'bold', true ); - kate.move( [ 0, 7 ], [ 0, 1 ], [ 0, 3 ] ); + kate.move( [ 0, 7 ] ); syncClients(); @@ -795,7 +795,7 @@ describe( 'transform', () => { expectClients( 'FooB<$text bold="true">ar' ); } ); - it.skip( 'element into paragraph #2, then undo', () => { + it( 'element into paragraph #2, then undo', () => { john.setData( 'Foo[Bar]' ); kate.setData( 'Foo[]Bar' ); diff --git a/tests/model/operation/transform/insert.js b/tests/model/operation/transform/insert.js index 635116e14..3eb36286b 100644 --- a/tests/model/operation/transform/insert.js +++ b/tests/model/operation/transform/insert.js @@ -811,7 +811,7 @@ describe( 'transform', () => { expectClients( 'FooBarAbc' ); } ); - it.skip( 'element, then add marker, split and undo with type #2', () => { + it( 'element, then add marker, split and undo with type #2', () => { john.setData( 'Foo[]' ); kate.setData( '[Foo]' ); @@ -845,8 +845,6 @@ describe( 'transform', () => { syncClients(); - // Actual content: - // Ba expectClients( 'FooBarAbc' ); } ); } ); diff --git a/tests/model/operation/transform/marker.js b/tests/model/operation/transform/marker.js index 07b4a2f2b..c9d404ad3 100644 --- a/tests/model/operation/transform/marker.js +++ b/tests/model/operation/transform/marker.js @@ -183,7 +183,6 @@ describe( 'transform', () => { kate.remove(); syncClients(); - expectClients( '' ); john.undo(); @@ -191,8 +190,7 @@ describe( 'transform', () => { syncClients(); - // Actual result for Kate: - // Foo Bar + // Wrong markers transforming. expectClients( '' + 'Foo Bar' + @@ -789,6 +787,41 @@ describe( 'transform', () => { '' ); } ); + + it( 'only marker end is inside merged element #1', () => { + john.setData( 'Foo[B]ar' ); + kate.setData( 'Foo[]Bar' ); + + john.setMarker( 'm1' ); + kate.merge(); + + syncClients(); + + expectClients( 'FooBar' ); + } ); + + it( 'only marker end is inside merged element #2', () => { + john.setData( 'Foo[]Bar' ); + kate.setData( 'Foo[]Bar' ); + + kate.split(); + + syncClients(); + expectClients( 'FooBar' ); + + john.setSelection( [ 1 ] ); + john.insert( 'Xyz' ); + + syncClients(); + expectClients( 'FooXyzBar' ); + + john.setSelection( [ 1 ], [ 2, 1 ] ); + john.setMarker( 'm1' ); + kate.undo(); + + syncClients(); + expectClients( 'FooBarXyz' ); + } ); } ); describe( 'by rename', () => { diff --git a/tests/model/operation/transform/move.js b/tests/model/operation/transform/move.js index f54108ef8..a7868903b 100644 --- a/tests/model/operation/transform/move.js +++ b/tests/model/operation/transform/move.js @@ -335,7 +335,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'text in other user\'s selection', () => { + it( 'text in other user\'s selection', () => { john.setData( '[Foo] Bar' ); kate.setData( 'F[]oo Bar' ); @@ -366,7 +366,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'inside moved text', () => { + it( 'inside moved text', () => { john.setData( 'F[oo Ba]rAbc' ); kate.setData( 'Foo[] BarAbc' ); @@ -375,10 +375,6 @@ describe( 'transform', () => { syncClients(); - // Actual result for Kate: - // Fr BaooAbc - // Move operations have wrong targets when they target at same place. - expectClients( 'F' + 'r' + diff --git a/tests/model/operation/transform/remove.js b/tests/model/operation/transform/remove.js index 92cbc090b..75635562f 100644 --- a/tests/model/operation/transform/remove.js +++ b/tests/model/operation/transform/remove.js @@ -158,13 +158,30 @@ describe( 'transform', () => { kate.wrap( 'blockQuote' ); syncClients(); + expectClients( 'Foo' ); john.undo(); syncClients(); - // Actual content: - // FooBar
+ expectClients( + 'Foo' + + '
' + + 'Bar' + + '
' + ); + } ); + + it( 'element while removing, then undo #2', () => { + john.setData( 'Foo[Bar]' ); + kate.setData( 'Foo[Bar]' ); + + john.remove(); + john.undo(); + kate.wrap( 'blockQuote' ); + + syncClients(); + expectClients( 'Foo' + '
' + @@ -218,7 +235,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'element while removing, then undo', () => { + it( 'element while removing, then undo', () => { john.setData( 'Foo
[Bar]
' ); kate.setData( 'Foo
[]Bar
' ); @@ -226,15 +243,11 @@ describe( 'transform', () => { kate.unwrap(); syncClients(); - expectClients( 'Foo' ); john.undo(); syncClients(); - - // Actual result: - // Foo
expectClients( 'Foo' + 'Bar' @@ -443,7 +456,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'element into paragraph, then undo', () => { + it( 'element into paragraph, then undo', () => { john.setData( 'F[oo]Bar' ); kate.setData( 'Foo[]Bar' ); @@ -451,15 +464,11 @@ describe( 'transform', () => { kate.merge(); syncClients(); - expectClients( 'FBar' ); john.undo(); syncClients(); - - // Actual result: - // FoBar expectClients( 'FooBar' ); } ); diff --git a/tests/model/operation/transform/split.js b/tests/model/operation/transform/split.js index 7f02b915c..e1b46a707 100644 --- a/tests/model/operation/transform/split.js +++ b/tests/model/operation/transform/split.js @@ -90,7 +90,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'intersecting wrap', () => { + it( 'intersecting wrap', () => { john.setData( 'Fo[]o' ); kate.setData( 'F[oo]' ); @@ -98,10 +98,9 @@ describe( 'transform', () => { kate.wrap( 'div' ); syncClients(); - expectClients( - 'F' + - '
oo
' + 'Fo' + + 'o' ); } ); } ); @@ -122,7 +121,7 @@ describe( 'transform', () => { ); } ); - it.skip( 'element in same position', () => { + it( 'element in same position', () => { john.setData( '
[]Foo
' ); kate.setData( '
[]Foo
' ); @@ -130,7 +129,6 @@ describe( 'transform', () => { kate.unwrap(); syncClients(); - expectClients( '
Foo
' ); } ); @@ -285,7 +283,7 @@ describe( 'transform', () => { expectClients( 'FooBar' ); } ); - it.skip( 'element into paragraph #3', () => { + it( 'element into paragraph #3', () => { john.setData( 'Foo[]Bar' ); kate.setData( 'Foo[]Bar' ); @@ -299,13 +297,10 @@ describe( 'transform', () => { kate.undo(); syncClients(); - // Actual content for Kate: - // FooBar - // There is a problem in Merge x Split transform in undo case. expectClients( 'FooBar' ); } ); - it.skip( 'element into paragraph #4', () => { + it( 'element into paragraph #4', () => { john.setData( 'FooB[]ar' ); kate.setData( 'Foo[]Bar' ); @@ -313,14 +308,16 @@ describe( 'transform', () => { kate.merge(); syncClients(); + expectClients( + 'FooB' + + 'ar' + ); john.undo(); kate.undo(); - expectClients( - 'Fo' + - 'oBar' - ); + syncClients(); + expectClients( 'FooBar' ); } ); } ); diff --git a/tests/model/operation/transform/unwrap.js b/tests/model/operation/transform/unwrap.js index 0b14ddd1b..8cba9be85 100644 --- a/tests/model/operation/transform/unwrap.js +++ b/tests/model/operation/transform/unwrap.js @@ -50,7 +50,7 @@ describe( 'transform', () => { expectClients( 'Foo' ); } ); - it.skip( 'the same element, then undo', () => { + it( 'the same element, then undo', () => { john.setData( '
[]Foo
' ); kate.setData( '
[]Foo
' ); @@ -63,9 +63,6 @@ describe( 'transform', () => { john.undo(); syncClients(); - // Actual content for Kate: - // Foo - // Kate has a different order of nodes in graveyard after syncing. expectClients( '
Foo
' ); } ); } ); diff --git a/tests/model/operation/transform/utils.js b/tests/model/operation/transform/utils.js index 8a0557151..12cccb7bb 100644 --- a/tests/model/operation/transform/utils.js +++ b/tests/model/operation/transform/utils.js @@ -7,7 +7,7 @@ import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting'; import BlockQuoteEditing from '@ckeditor/ckeditor5-block-quote/src/blockquoteediting'; import { getData, parse } from '../../../../src/dev-utils/model'; -import transform from '../../../../src/model/operation/transform'; +import { transformSets } from '../../../../src/model/operation/transform'; import Position from '../../../../src/model/position'; import Range from '../../../../src/model/range'; import OperationFactory from '../../../../src/model/operation/operationfactory'; @@ -253,16 +253,34 @@ function bufferOperations( operations, client ) { } export function syncClients() { - for ( const client of clients ) { - for ( const item of bufferedOperations ) { - const remoteOperations = item.operations.map( op => OperationFactory.fromJSON( JSON.parse( op ), client.document ) ); - const remoteClient = item.client; + const clientsOperations = {}; + + // For each client, flatten all buffered operations into one set. + for ( const item of bufferedOperations ) { + const name = item.client.name; + + if ( !clientsOperations[ name ] ) { + clientsOperations[ name ] = []; + } + + clientsOperations[ name ].push( ...item.operations ); + } - if ( remoteClient == client ) { + for ( const localClient of clients ) { + for ( const remoteClient of clients ) { + if ( remoteClient == localClient ) { continue; } - const clientOperations = Array.from( client.document.history.getOperations( client.syncedVersion ) ); + const remoteOperationsJson = clientsOperations[ remoteClient.name ]; + + if ( !remoteOperationsJson ) { + continue; + } + + const remoteOperations = remoteOperationsJson.map( op => OperationFactory.fromJSON( JSON.parse( op ), localClient.document ) ); + + const localOperations = Array.from( localClient.document.history.getOperations( localClient.syncedVersion ) ); let remoteOperationsTransformed = null; @@ -271,21 +289,21 @@ export function syncClients() { padWithNoOps: true }; - if ( client.orderNumber < remoteClient.orderNumber ) { - remoteOperationsTransformed = transform.transformSets( clientOperations, remoteOperations, options ).operationsB; + if ( localClient.orderNumber < remoteClient.orderNumber ) { + remoteOperationsTransformed = transformSets( localOperations, remoteOperations, options ).operationsB; } else { - remoteOperationsTransformed = transform.transformSets( remoteOperations, clientOperations, options ).operationsA; + remoteOperationsTransformed = transformSets( remoteOperations, localOperations, options ).operationsA; } - client.editor.model.enqueueChange( 'transparent', writer => { + localClient.editor.model.enqueueChange( 'transparent', writer => { for ( const operation of remoteOperationsTransformed ) { writer.batch.addOperation( operation ); - client.editor.model.applyOperation( operation ); + localClient.editor.model.applyOperation( operation ); } } ); } - client.syncedVersion = client.document.version; + localClient.syncedVersion = localClient.document.version; } bufferedOperations.clear(); diff --git a/tests/model/operation/transform/wrap.js b/tests/model/operation/transform/wrap.js index f7e125980..e03efc0e5 100644 --- a/tests/model/operation/transform/wrap.js +++ b/tests/model/operation/transform/wrap.js @@ -63,19 +63,24 @@ describe( 'transform', () => { ); } ); - it.skip( 'intersecting wrap #2', () => { - john.setData( '[Foo]' ); - kate.setData( 'F[o]o' ); + it( 'intersecting wrap #2', () => { + john.setData( '[FooBarAbc]' ); + kate.setData( 'Foo[Bar]Abc' ); - john.wrap( 'div' ); + john.wrap( 'blockQuote' ); kate.wrap( 'div' ); syncClients(); - - expectClients( '
Foo
' ); + expectClients( + '
' + + 'Foo' + + 'Bar' + + 'Abc' + + '
' + ); } ); - it.skip( 'intersecting wrap, then undo #1', () => { + it( 'intersecting wrap, then undo #1', () => { john.setData( '[FooBar]Abc' ); kate.setData( 'Foo[BarAbc]' ); @@ -83,12 +88,20 @@ describe( 'transform', () => { kate.wrap( 'div' ); syncClients(); + expectClients( + '
' + + 'Foo' + + 'Bar' + + '
' + + '
' + + 'Abc' + + '
' + ); john.undo(); kate.undo(); syncClients(); - expectClients( 'Foo' + 'Bar' + @@ -96,21 +109,30 @@ describe( 'transform', () => { ); } ); - it.skip( 'intersecting wrap, then undo #2', () => { - john.setData( '[Foo]' ); - kate.setData( 'F[o]o' ); + it( 'intersecting wrap, then undo #2', () => { + john.setData( '[FooBar]Abc' ); + kate.setData( 'Foo[BarAbc]' ); - john.wrap( 'div' ); + john.wrap( 'blockQuote' ); kate.wrap( 'div' ); syncClients(); + expectClients( + '
' + + 'Foo' + + 'Bar' + + '
' + + '
' + + 'Abc' + + '
' + ); john.undo(); kate.undo(); syncClients(); - expectClients( '
Foo
' ); + expectClients( 'FooBarAbc' ); } ); it( 'element and text', () => { @@ -189,6 +211,102 @@ describe( 'transform', () => { } ); } ); + describe( 'by remove', () => { + it( 'remove the only wrapped element', () => { + john.setData( '[Foo]Bar' ); + kate.setData( '[Foo]Bar' ); + + john.wrap( 'blockQuote' ); + kate.remove(); + + syncClients(); + + expectClients( 'Bar' ); + } ); + + it( 'remove one of two wrapped elements', () => { + john.setData( '[FooBar]' ); + kate.setData( '[Foo]Bar' ); + + john.wrap( 'blockQuote' ); + kate.remove(); + + syncClients(); + + expectClients( '
Bar
' ); + } ); + + it( 'remove all wrapped elements', () => { + john.setData( '[FooBar]Xyz' ); + kate.setData( '[Foo]BarXyz' ); + + john.wrap( 'blockQuote' ); + + kate.remove(); + kate.setSelection( [ 0 ], [ 1 ] ); + kate.remove(); + + syncClients(); + + expectClients( 'Xyz' ); + } ); + + it( 'remove the only wrapped element with undo', () => { + john.setData( '[Foo]Bar' ); + kate.setData( '[Foo]Bar' ); + + john.wrap( 'blockQuote' ); + kate.remove(); + + syncClients(); + expectClients( 'Bar' ); + + john.undo(); + kate.undo(); + + syncClients(); + expectClients( 'FooBar' ); + } ); + + it( 'remove one of two wrapped elements with undo', () => { + john.setData( '[FooBar]' ); + kate.setData( '[Foo]Bar' ); + + john.wrap( 'blockQuote' ); + kate.remove(); + + syncClients(); + expectClients( '
Bar
' ); + + john.undo(); + kate.undo(); + + syncClients(); + expectClients( 'FooBar' ); + } ); + + it( 'remove all wrapped elements with undo', () => { + john.setData( '[FooBar]Xyz' ); + kate.setData( '[Foo]BarXyz' ); + + john.wrap( 'blockQuote' ); + + kate.remove(); + kate.setSelection( [ 0 ], [ 1 ] ); + kate.remove(); + + syncClients(); + expectClients( 'Xyz' ); + + john.undo(); + kate.undo(); + kate.undo(); + + syncClients(); + expectClients( 'FooBarXyz' ); + } ); + } ); + describe( 'by merge', () => { it( 'element into paragraph #1', () => { john.setData( '[Foo]Bar' ); @@ -219,7 +337,7 @@ describe( 'transform', () => { expectClients( 'FooBar' ); } ); - it.skip( 'element into paragraph, then undo', () => { + it( 'element into paragraph, then undo', () => { john.setData( 'Foo[Bar]' ); kate.setData( 'Foo[]Bar' ); @@ -227,14 +345,12 @@ describe( 'transform', () => { kate.merge(); syncClients(); - - expectClients( '
FooBar
' ); + expectClients( 'FooBar' ); john.undo(); kate.undo(); syncClients(); - expectClients( 'FooBar' ); } ); } ); diff --git a/tests/model/range.js b/tests/model/range.js index feea61f39..2f7bfbcac 100644 --- a/tests/model/range.js +++ b/tests/model/range.js @@ -12,7 +12,6 @@ import TreeWalker from '../../src/model/treewalker'; 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 RenameOperation from '../../src/model/operation/renameoperation'; import MergeOperation from '../../src/model/operation/mergeoperation'; import SplitOperation from '../../src/model/operation/splitoperation'; @@ -898,10 +897,10 @@ describe( 'Range', () => { } ); } ); - describe( 'by RemoveOperation', () => { + describe( 'by MoveOperation to graveyard', () => { it( 'remove before range', () => { const start = new Position( root, [ 0 ] ); - const op = new RemoveOperation( start, 2, gyPos, 1 ); + const op = new MoveOperation( start, 2, gyPos, 1 ); const transformed = range.getTransformedByOperation( op ); @@ -910,7 +909,7 @@ describe( 'Range', () => { it( 'remove intersecting with range', () => { const start = new Position( root, [ 4 ] ); - const op = new RemoveOperation( start, 2, gyPos, 1 ); + const op = new MoveOperation( start, 2, gyPos, 1 ); const transformed = range.getTransformedByOperation( op ); @@ -922,7 +921,7 @@ describe( 'Range', () => { it( 'remove inside the range', () => { const start = new Position( root, [ 3 ] ); - const op = new RemoveOperation( start, 2, gyPos, 1 ); + const op = new MoveOperation( start, 2, gyPos, 1 ); const transformed = range.getTransformedByOperation( op ); diff --git a/tests/model/schema.js b/tests/model/schema.js index 55c01041b..b02a606c8 100644 --- a/tests/model/schema.js +++ b/tests/model/schema.js @@ -1106,8 +1106,7 @@ describe( 'Schema', () => { } ); describe( 'getValidRanges()', () => { - const attribute = 'bold'; - let model, doc, root, schema, ranges; + let model, doc, root, schema; beforeEach( () => { model = new Model(); @@ -1116,112 +1115,145 @@ describe( 'Schema', () => { root = doc.createRoot(); schema.register( 'p', { inheritAllFrom: '$block' } ); - schema.register( 'h1', { inheritAllFrom: '$block' } ); - schema.register( 'img', { - allowWhere: '$text' - } ); + schema.register( 'img', { allowWhere: '$text' } ); - schema.addAttributeCheck( ( ctx, attributeName ) => { - // Allow 'bold' on p>$text. - if ( ctx.endsWith( 'p $text' ) && attributeName == 'bold' ) { - return true; - } + // This is a "hack" to allow setting any ranges in `setData` util. + schema.extend( '$text', { allowIn: '$root' } ); + } ); - // Allow 'bold' on $root>p. - if ( ctx.endsWith( '$root p' ) && attributeName == 'bold' ) { - return true; - } - } ); + function testValidRangesForAttribute( input, attribute, output ) { + setData( model, input ); - // Parse data string to model. - const parsedModel = parse( '

foobar

', model.schema, { context: [ root.name ] } ); + const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute ); + const sel = new Selection( validRanges ); - // Set parsed model data to prevent selection post-fixer from running. - model.change( writer => { - writer.insert( parsedModel, root ); - } ); + expect( stringify( root, sel ) ).to.equal( output ); + } + + it( 'should return a range with p for an attribute allowed only on p', () => { + schema.extend( 'p', { allowAttributes: 'foo' } ); - ranges = [ Range.createOn( root.getChild( 0 ) ) ]; + testValidRangesForAttribute( + '[

foobar

]', + 'foo', + '[

foobar

]' + ); } ); - it( 'should return unmodified ranges when attribute is allowed on each item (text is not allowed in img)', () => { - schema.extend( 'img', { allowAttributes: 'bold' } ); + it( 'should return ranges on text nodes for an attribute allowed only on text', () => { + schema.extend( '$text', { allowAttributes: 'bold' } ); - expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges ); + testValidRangesForAttribute( + '[

foobar

]', + 'bold', + '

[foo][bar]

' + ); } ); - it( 'should return unmodified ranges when attribute is allowed on each item (text is allowed in img)', () => { - schema.extend( 'img', { allowAttributes: 'bold' } ); - schema.extend( '$text', { allowIn: 'img' } ); + it( 'should return a range on img for an attribute allowed only on img', () => { + schema.extend( 'img', { allowAttributes: 'src' } ); - expect( schema.getValidRanges( ranges, attribute ) ).to.deep.equal( ranges ); + testValidRangesForAttribute( + '[

foobar

]', + 'src', + '

foo[]bar

' + ); } ); - it( 'should return two ranges when attribute is not allowed on one item', () => { + it( 'should return a range containing all children for an attribute allowed on all children', () => { + schema.extend( '$text', { allowAttributes: 'bold' } ); schema.extend( 'img', { allowAttributes: 'bold' } ); - schema.extend( '$text', { allowIn: 'img' } ); - setData( model, '

[fooxxxbar]

' ); + testValidRangesForAttribute( + '[

foobar

]', + 'bold', + '

[foobar]

' + ); + } ); - const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute ); - const sel = new Selection( validRanges ); + it( 'should return a range with p and a range on all children for an attribute allowed on p and its children', () => { + schema.extend( 'p', { allowAttributes: 'foo' } ); + schema.extend( '$text', { allowAttributes: 'foo' } ); + schema.extend( 'img', { allowAttributes: 'foo' } ); - expect( stringify( root, sel ) ).to.equal( '

[foo]xxx[bar]

' ); - } ); + setData( model, '[

foobar

]' ); - it( 'should return three ranges when attribute is not allowed on one element but is allowed on its child', () => { - schema.extend( '$text', { allowIn: 'img' } ); + const validRanges = Array.from( schema.getValidRanges( doc.selection.getRanges(), 'foo' ) ); - schema.addAttributeCheck( ( ctx, attributeName ) => { - // Allow 'bold' on img>$text. - if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) { - return true; - } - } ); + expect( validRanges.length ).to.equal( 2 ); - setData( model, '

[fooxxxbar]

' ); + expect( validRanges[ 0 ].start.path ).to.deep.equal( [ 0, 0 ] ); + expect( validRanges[ 0 ].end.path ).to.deep.equal( [ 0, 7 ] ); - const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute ); - const sel = new Selection( validRanges ); + expect( validRanges[ 1 ].start.path ).to.deep.equal( [ 0 ] ); + expect( validRanges[ 1 ].end.path ).to.deep.equal( [ 1 ] ); + } ); + + it( 'should not break a range if children are not allowed to have the attribute', () => { + schema.extend( 'p', { allowAttributes: 'foo' } ); - expect( stringify( root, sel ) ).to.equal( '

[foo][xxx][bar]

' ); + testValidRangesForAttribute( + '[

foo

bar

]', + 'foo', + '[

foo

bar

]' + ); } ); - it( 'should not leak beyond the given ranges', () => { - setData( model, '

[foobar]x[barfoo]

' ); + it( 'should search deeply', () => { + schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } ); - const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute ); - const sel = new Selection( validRanges ); + testValidRangesForAttribute( + '[

fooxxxbar

]', + 'bold', + '

[foo][xxx][bar]

' + ); + } ); + + it( 'should work with multiple ranges', () => { + schema.extend( '$text', { allowAttributes: 'bold' } ); - expect( stringify( root, sel ) ).to.equal( '

[foo][bar]x[bar][foo]

' ); + testValidRangesForAttribute( + '[

a

b

]

c

[d]

', + 'bold', + '

[a]

[b]

c

[d]

' + ); } ); - it( 'should correctly handle a range which ends in a disallowed position', () => { - schema.extend( '$text', { allowIn: 'img' } ); + it( 'should work with non-flat ranges', () => { + schema.extend( '$text', { allowAttributes: 'bold' } ); - setData( model, '

[foobar]bom

' ); + testValidRangesForAttribute( + '[

a

b

c]

d

', + 'bold', + '

[a]

[b]

[c]

d

' + ); + } ); - const validRanges = schema.getValidRanges( doc.selection.getRanges(), attribute ); - const sel = new Selection( validRanges ); + it( 'should not leak beyond the given ranges', () => { + schema.extend( '$text', { allowAttributes: 'bold' } ); - expect( stringify( root, sel ) ).to.equal( '

[foo]barbom

' ); + testValidRangesForAttribute( + '[

foo

b]a[r

x]yz

', + 'bold', + '

[foo]

[b]a[r]

[x]yz

' + ); } ); - it( 'should split range into two ranges and omit disallowed element', () => { + it( 'should correctly handle a range which ends in a disallowed position', () => { + schema.extend( '$text', { allowAttributes: 'bold', allowIn: 'img' } ); + + // Disallow bold on text inside image. schema.addAttributeCheck( ( ctx, attributeName ) => { - // Disallow 'bold' on p>img. - if ( ctx.endsWith( 'p img' ) && attributeName == 'bold' ) { + if ( ctx.endsWith( 'img $text' ) && attributeName == 'bold' ) { return false; } } ); - const result = schema.getValidRanges( ranges, attribute ); - - expect( result ).to.length( 2 ); - expect( result[ 0 ].start.path ).to.members( [ 0 ] ); - expect( result[ 0 ].end.path ).to.members( [ 0, 3 ] ); - expect( result[ 1 ].start.path ).to.members( [ 0, 4 ] ); - expect( result[ 1 ].end.path ).to.members( [ 1 ] ); + testValidRangesForAttribute( + '[

fooxx]xbar

', + 'bold', + '

[foo]xxxbar

' + ); } ); } ); diff --git a/tests/model/selection.js b/tests/model/selection.js index 4439e9012..630420c5c 100644 --- a/tests/model/selection.js +++ b/tests/model/selection.js @@ -633,7 +633,7 @@ describe( 'Selection', () => { it( 'should throw an error when range is invalid', () => { expect( () => { selection._setRanges( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, /model-selection-added-not-range/ ); + } ).to.throw( CKEditorError, /model-selection-set-ranges-not-range/ ); } ); it( 'should remove all ranges and add given ranges', () => { diff --git a/tests/model/writer.js b/tests/model/writer.js index e31a5d0a4..fd466b90e 100644 --- a/tests/model/writer.js +++ b/tests/model/writer.js @@ -1545,7 +1545,7 @@ describe( 'Writer', () => { expect( batch.operations.length ).to.equal( 2 ); } ); - it( 'should use RemoveOperation', () => { + it( 'should use MoveOperation to graveyard', () => { batch = new Batch(); remove( div ); diff --git a/tests/view/documentselection.js b/tests/view/documentselection.js index 4525b544e..78ca8a11d 100644 --- a/tests/view/documentselection.js +++ b/tests/view/documentselection.js @@ -113,7 +113,7 @@ describe( 'DocumentSelection', () => { expect( () => { // eslint-disable-next-line no-new new DocumentSelection( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' ); + } ).to.throw( CKEditorError, /view-selection-add-range-not-range/ ); } ); it( 'should throw an error when ranges intersects', () => { @@ -1004,7 +1004,7 @@ describe( 'DocumentSelection', () => { it( 'should throw an error when range is invalid', () => { expect( () => { documentSelection._setTo( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' ); + } ).to.throw( CKEditorError, /view-selection-add-range-not-range/ ); } ); it( 'should throw when range is intersecting with already added range', () => { diff --git a/tests/view/selection.js b/tests/view/selection.js index 910f20123..6c503e9d1 100644 --- a/tests/view/selection.js +++ b/tests/view/selection.js @@ -113,7 +113,7 @@ describe( 'Selection', () => { expect( () => { // eslint-disable-next-line no-new new Selection( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' ); + } ).to.throw( CKEditorError, /view-selection-add-range-not-range/ ); } ); it( 'should throw an error when ranges intersects', () => { @@ -878,7 +878,7 @@ describe( 'Selection', () => { it( 'should throw an error when range is invalid', () => { expect( () => { selection.setTo( [ { invalid: 'range' } ] ); - } ).to.throw( CKEditorError, 'view-selection-invalid-range: Invalid Range.' ); + } ).to.throw( CKEditorError, /view-selection-add-range-not-range/ ); } ); it( 'should throw when range is intersecting with already added range', () => {