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

Commit

Permalink
Merge branch 'master' into t/1139b
Browse files Browse the repository at this point in the history
  • Loading branch information
Reinmar committed Sep 15, 2017
2 parents a3af9aa + 1be7ed1 commit 7a90bc2
Show file tree
Hide file tree
Showing 2 changed files with 65 additions and 2 deletions.
22 changes: 20 additions & 2 deletions src/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,26 @@ export default class Range {
// Collapsed range is in merged element, at the beginning or at the end of it.
// Without fix, the range would end up in the graveyard, together with removed element.
// <p>foo</p><p>[]bar</p> -> <p>foobar</p><p>[]</p> -> <p>foobar</p> -> <p>foo[]bar</p>
// <p>foo</p><p>bar[]</p>
return [ new Range( targetPosition.getShiftedBy( this.start.offset ) ) ];
// <p>foo</p><p>bar[]</p> -> <p>foobar</p><p>[]</p> -> <p>foobar</p> -> <p>foobar[]</p>
//
// In most cases, `sourceRange.start.offset` for merge delta's move operation would be 0,
// so this formula might look overcomplicated.
// However in some scenarios, after operational transformation, move operation might not
// in fact start from 0 and we need to properly count new offset.
// https://github.com/ckeditor/ckeditor5-engine/pull/1133#issuecomment-329080668.
const offset = this.start.offset - sourceRange.start.offset;

return [ new Range( targetPosition.getShiftedBy( offset ) ) ];
}
//
// Edge case for split delta.
//
if ( deltaType == 'split' && this.isCollapsed && this.end.isEqual( sourceRange.end ) ) {
// Collapsed range is at the end of split element.
// Without fix, the range would end up at the end of split (old) element instead of at the end of new element.
// That would happen because this range is not technically inside moved range. Last step below shows the fix.
// <p>foobar[]</p> -> <p>foobar[]</p><p></p> -> <p>foo[]</p><p>bar</p> -> <p>foo</p><p>bar[]</p>
return [ new Range( targetPosition.getShiftedBy( howMany ) ) ];
}
//
// Other edge cases:
Expand Down
45 changes: 45 additions & 0 deletions tests/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ import Element from '../../src/model/element';
import Text from '../../src/model/text';
import Document from '../../src/model/document';
import TreeWalker from '../../src/model/treewalker';
import MergeDelta from '../../src/model/delta/mergedelta';
import MoveOperation from '../../src/model/operation/moveoperation';
import RemoveOperation from '../../src/model/operation/removeoperation';
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
import { jsonParseStringify } from '../../tests/model/_utils/utils';

Expand Down Expand Up @@ -1024,6 +1027,19 @@ describe( 'Range', () => {
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 3 ] );
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 1, 2 ] );
} );

it( 'split element which has collapsed range at the end', () => {
range.start = new Position( root, [ 0, 6 ] );
range.end = new Position( root, [ 0, 6 ] );

const delta = getSplitDelta( new Position( root, [ 0, 3 ] ), new Element( 'p' ), 3, 1 );

const transformed = range.getTransformedByDelta( delta );

expect( transformed.length ).to.equal( 1 );
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 1, 3 ] );
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 1, 3 ] );
} );
} );

describe( 'by MergeDelta', () => {
Expand Down Expand Up @@ -1056,6 +1072,35 @@ describe( 'Range', () => {
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 0, 1 ] );
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 1, 1 ] );
} );

// #1132.
it( 'merge delta has move operation that does not start from offset 0', () => {
// This scenario is a test for a rare situation, that after some OT, a move operation in
// merge delta does not start from 0 offset.
//
// It happens that move operation in merge delta becomes "do nothing move operation", something like:
//
// move range [ a, x ] - [ a, y ] to [ a, x ]
// for example: move [ 0, 3 ] - [ 0, 6 ] -> [ 0, 3 ]
//
// This is a result of valid transformation and we need to check if range is properly transformed
// when such unusual delta is generated.
// For more see: https://github.com/ckeditor/ckeditor5-engine/pull/1133#issuecomment-329080668.
//
// For this test scenario assume: <p>foobar[]</p>, "bar" is moved between "o" and "b".
// Expect state after transformation is that nothing has changed.
const range = new Range( new Position( root, [ 0, 6 ] ), new Position( root, [ 0, 6 ] ) );

const delta = new MergeDelta();
delta.addOperation( new MoveOperation( new Position( root, [ 0, 3 ] ), 3, new Position( root, [ 0, 3 ] ), 0 ) );
delta.addOperation( new RemoveOperation( new Position( root, [ 1 ] ), 1, new Position( doc.graveyard, [ 0 ] ), 1 ) );

const transformed = range.getTransformedByDelta( delta );

expect( transformed.length ).to.equal( 1 );
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 6 ] );
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 6 ] );
} );
} );

describe( 'by WrapDelta', () => {
Expand Down

0 comments on commit 7a90bc2

Please sign in to comment.