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

Commit 97a4f4b

Browse files
authoredSep 15, 2017
Merge pull request #1133 from ckeditor/t/1132
Fix: Fixed a bug in `Range#getTransformedByDelta()` that caused editor to crash after some `MergeDelta`s were transformed. Closes #1132.
2 parents 2bdd99e + df51d0d commit 97a4f4b

File tree

2 files changed

+42
-2
lines changed

2 files changed

+42
-2
lines changed
 

‎src/model/range.js

+10-2
Original file line numberDiff line numberDiff line change
@@ -492,8 +492,16 @@ export default class Range {
492492
// Collapsed range is in merged element, at the beginning or at the end of it.
493493
// Without fix, the range would end up in the graveyard, together with removed element.
494494
// <p>foo</p><p>[]bar</p> -> <p>foobar</p><p>[]</p> -> <p>foobar</p> -> <p>foo[]bar</p>
495-
// <p>foo</p><p>bar[]</p>
496-
return [ new Range( targetPosition.getShiftedBy( this.start.offset ) ) ];
495+
// <p>foo</p><p>bar[]</p> -> <p>foobar</p><p>[]</p> -> <p>foobar</p> -> <p>foobar[]</p>
496+
//
497+
// In most cases, `sourceRange.start.offset` for merge delta's move operation would be 0,
498+
// so this formula might look overcomplicated.
499+
// However in some scenarios, after operational transformation, move operation might not
500+
// in fact start from 0 and we need to properly count new offset.
501+
// https://github.com/ckeditor/ckeditor5-engine/pull/1133#issuecomment-329080668.
502+
const offset = this.start.offset - sourceRange.start.offset;
503+
504+
return [ new Range( targetPosition.getShiftedBy( offset ) ) ];
497505
}
498506
//
499507
// Other edge cases:

‎tests/model/range.js

+32
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import Element from '../../src/model/element';
99
import Text from '../../src/model/text';
1010
import Document from '../../src/model/document';
1111
import TreeWalker from '../../src/model/treewalker';
12+
import MergeDelta from '../../src/model/delta/mergedelta';
13+
import MoveOperation from '../../src/model/operation/moveoperation';
14+
import RemoveOperation from '../../src/model/operation/removeoperation';
1215
import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
1316
import { jsonParseStringify } from '../../tests/model/_utils/utils';
1417

@@ -1056,6 +1059,35 @@ describe( 'Range', () => {
10561059
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 0, 1 ] );
10571060
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 1, 1 ] );
10581061
} );
1062+
1063+
// #1132.
1064+
it( 'merge delta has move operation that does not start from offset 0', () => {
1065+
// This scenario is a test for a rare situation, that after some OT, a move operation in
1066+
// merge delta does not start from 0 offset.
1067+
//
1068+
// It happens that move operation in merge delta becomes "do nothing move operation", something like:
1069+
//
1070+
// move range [ a, x ] - [ a, y ] to [ a, x ]
1071+
// for example: move [ 0, 3 ] - [ 0, 6 ] -> [ 0, 3 ]
1072+
//
1073+
// This is a result of valid transformation and we need to check if range is properly transformed
1074+
// when such unusual delta is generated.
1075+
// For more see: https://github.com/ckeditor/ckeditor5-engine/pull/1133#issuecomment-329080668.
1076+
//
1077+
// For this test scenario assume: <p>foobar[]</p>, "bar" is moved between "o" and "b".
1078+
// Expect state after transformation is that nothing has changed.
1079+
const range = new Range( new Position( root, [ 0, 6 ] ), new Position( root, [ 0, 6 ] ) );
1080+
1081+
const delta = new MergeDelta();
1082+
delta.addOperation( new MoveOperation( new Position( root, [ 0, 3 ] ), 3, new Position( root, [ 0, 3 ] ), 0 ) );
1083+
delta.addOperation( new RemoveOperation( new Position( root, [ 1 ] ), 1, new Position( doc.graveyard, [ 0 ] ), 1 ) );
1084+
1085+
const transformed = range.getTransformedByDelta( delta );
1086+
1087+
expect( transformed.length ).to.equal( 1 );
1088+
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 0, 6 ] );
1089+
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 0, 6 ] );
1090+
} );
10591091
} );
10601092

10611093
describe( 'by WrapDelta', () => {

0 commit comments

Comments
 (0)
This repository has been archived.