From ddb1c9f5a2017d2af3a57e39192f3b896452e186 Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 6 Oct 2017 15:32:21 +0200 Subject: [PATCH 1/3] Fix: Prevent throwing error when logging MergeDelta with NoOperation. --- src/dev-utils/enableenginedebug.js | 5 ++++- tests/dev-utils/enableenginedebug.js | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/dev-utils/enableenginedebug.js b/src/dev-utils/enableenginedebug.js index 88231077b..24cabc8d9 100644 --- a/src/dev-utils/enableenginedebug.js +++ b/src/dev-utils/enableenginedebug.js @@ -382,7 +382,10 @@ function enableLoggingTools() { MergeDelta.prototype.toString = function() { return getClassName( this ) + `( ${ this.baseVersion } ): ` + - this.position.toString(); + ( this.position ? + this.position.toString() : + `(move from ${ this.operations[ 0 ].sourcePosition })` + ); }; MoveDelta.prototype.toString = function() { diff --git a/tests/dev-utils/enableenginedebug.js b/tests/dev-utils/enableenginedebug.js index 0e41bfffe..dc4a3323d 100644 --- a/tests/dev-utils/enableenginedebug.js +++ b/tests/dev-utils/enableenginedebug.js @@ -397,6 +397,25 @@ describe( 'debug tools', () => { expect( log.calledWithExactly( delta.toString() ) ).to.be.true; } ); + it( 'MergeDelta - NoOperation as second operation', () => { + const otherRoot = modelDoc.createRoot( '$root', 'otherRoot' ); + const firstEle = new ModelElement( 'paragraph' ); + const removedEle = new ModelElement( 'paragraph', null, [ new ModelText( 'foo' ) ] ); + + otherRoot.appendChildren( [ firstEle, removedEle ] ); + + const delta = new MergeDelta(); + const move = new MoveOperation( ModelPosition.createAt( removedEle, 0 ), 3, ModelPosition.createAt( firstEle, 0 ), 0 ); + + delta.addOperation( move ); + delta.addOperation( new NoOperation( 1 ) ); + + expect( delta.toString() ).to.equal( 'MergeDelta( 0 ): (move from otherRoot [ 1, 0 ])' ); + + delta.log(); + expect( log.calledWithExactly( delta.toString() ) ).to.be.true; + } ); + it( 'MoveDelta', () => { const delta = new MoveDelta(); const move1 = new MoveOperation( ModelPosition.createAt( modelRoot, 0 ), 1, ModelPosition.createAt( modelRoot, 3 ), 0 ); From dfd74eaa2efa0b76b9ca615a83ff8c7aa576c32b Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 6 Oct 2017 15:32:48 +0200 Subject: [PATCH 2/3] Changed: Minor refactoring making code more clear. --- src/model/operation/transform.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/model/operation/transform.js b/src/model/operation/transform.js index f5c5016a4..13cc541ef 100644 --- a/src/model/operation/transform.js +++ b/src/model/operation/transform.js @@ -342,7 +342,9 @@ const ot = { InsertOperation( a, b, context ) { // Create range from MoveOperation properties and transform it by insertion. let range = Range.createFromPositionAndShift( a.sourcePosition, a.howMany ); - range = range._getTransformedByInsertion( b.position, b.nodes.maxOffset, false, a.isSticky && !context.forceNotSticky )[ 0 ]; + const includeB = a.isSticky && !context.forceNotSticky; + + range = range._getTransformedByInsertion( b.position, b.nodes.maxOffset, false, includeB )[ 0 ]; // Check whether there is a forced order of nodes or use `context.isStrong` flag for conflict resolving. const insertBefore = context.insertBefore === undefined ? !context.isStrong : context.insertBefore; From 4d6007a00d14c4d9c1750cd100f321be62e2344e Mon Sep 17 00:00:00 2001 From: Szymon Cofalik Date: Fri, 6 Oct 2017 15:33:15 +0200 Subject: [PATCH 3/3] Fix: Handle isSticky flag properly when collapsed range is transformed by insertion. --- src/model/range.js | 2 +- tests/model/range.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/model/range.js b/src/model/range.js index 62a84b530..e8fcd56bf 100644 --- a/src/model/range.js +++ b/src/model/range.js @@ -615,7 +615,7 @@ export default class Range { } else { const range = Range.createFromRange( this ); - const insertBeforeStart = range.isCollapsed ? true : !isSticky; + const insertBeforeStart = !isSticky; const insertBeforeEnd = range.isCollapsed ? true : isSticky; range.start = range.start._getTransformedByInsertion( insertPosition, howMany, insertBeforeStart ); diff --git a/tests/model/range.js b/tests/model/range.js index 71d40e58e..adf1bb9c2 100644 --- a/tests/model/range.js +++ b/tests/model/range.js @@ -572,11 +572,11 @@ describe( 'Range', () => { expect( transformed[ 0 ].end.path ).to.deep.equal( [ 4, 4 ] ); } ); - it( 'should move after inserted nodes if the range is collapsed and isSticky is true', () => { + it( 'should expand range after inserted nodes if the range is collapsed and isSticky is true', () => { const range = new Range( new Position( root, [ 3, 2 ] ), new Position( root, [ 3, 2 ] ) ); const transformed = range._getTransformedByInsertion( new Position( root, [ 3, 2 ] ), 3, false, true ); - expect( transformed[ 0 ].start.path ).to.deep.equal( [ 3, 5 ] ); + expect( transformed[ 0 ].start.path ).to.deep.equal( [ 3, 2 ] ); expect( transformed[ 0 ].end.path ).to.deep.equal( [ 3, 5 ] ); } ); } );