8
0
Просмотр исходного кода

Merge branch 'master' into t/ckeditor5-dev/278

Piotrek Koszuliński 8 лет назад
Родитель
Сommit
17b20c191b

+ 1 - 1
packages/ckeditor5-engine/package.json

@@ -9,7 +9,7 @@
     "@ckeditor/ckeditor5-utils": "^0.10.0"
   },
   "devDependencies": {
-    "@ckeditor/ckeditor5-dev-lint": "^3.1.0",
+    "@ckeditor/ckeditor5-dev-lint": "^3.1.1",
     "@ckeditor/ckeditor5-basic-styles": "^0.9.0",
     "@ckeditor/ckeditor5-core": "^0.9.0",
     "@ckeditor/ckeditor5-editor-classic": "^0.8.0",

+ 11 - 0
packages/ckeditor5-engine/src/model/documentselection.js

@@ -119,6 +119,17 @@ export default class DocumentSelection extends Selection {
 		return this._ranges.length ? this._ranges.length : 1;
 	}
 
+	/**
+	 * Describes whether `DocumentSelection` has own range(s) set, or if it is defaulted to
+	 * {@link module:engine/model/document~Document#_getDefaultRange document's default range}.
+	 *
+	 * @readonly
+	 * @type {Boolean}
+	 */
+	get hasOwnRange() {
+		return this._ranges.length > 0;
+	}
+
 	/**
 	 * Unbinds all events previously bound by document selection.
 	 */

+ 20 - 2
packages/ckeditor5-engine/src/model/range.js

@@ -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:

+ 12 - 0
packages/ckeditor5-engine/tests/model/documentselection.js

@@ -181,6 +181,18 @@ describe( 'DocumentSelection', () => {
 		} );
 	} );
 
+	describe( 'hasOwnRange', () => {
+		it( 'should return true if selection has any ranges set', () => {
+			selection.addRange( new Range( new Position( root, [ 0 ] ), new Position( root, [ 0 ] ) ) );
+
+			expect( selection.hasOwnRange ).to.be.true;
+		} );
+
+		it( 'should return false if selection has a default range', () => {
+			expect( selection.hasOwnRange ).to.be.false;
+		} );
+	} );
+
 	describe( 'addRange()', () => {
 		it( 'should convert added Range to LiveRange', () => {
 			selection.addRange( range );

+ 45 - 0
packages/ckeditor5-engine/tests/model/range.js

@@ -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';
 
@@ -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', () => {
@@ -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', () => {