Explorar el Código

Merge branch 'master' into t/1049

Szymon Cofalik hace 8 años
padre
commit
441eeb4864

+ 2 - 1
packages/ckeditor5-engine/README.md

@@ -1,6 +1,7 @@
-CKEditor 5 Editing Engine
+CKEditor 5 editing engine
 ========================================
 
+[![Join the chat at https://gitter.im/ckeditor/ckeditor5](https://badges.gitter.im/ckeditor/ckeditor5.svg)](https://gitter.im/ckeditor/ckeditor5?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge)
 [![npm version](https://badge.fury.io/js/%40ckeditor%2Fckeditor5-engine.svg)](https://www.npmjs.com/package/@ckeditor/ckeditor5-engine)
 [![Build Status](https://travis-ci.org/ckeditor/ckeditor5-engine.svg?branch=master)](https://travis-ci.org/ckeditor/ckeditor5-engine)
 [![Test Coverage](https://codeclimate.com/github/ckeditor/ckeditor5-engine/badges/coverage.svg)](https://codeclimate.com/github/ckeditor/ckeditor5-engine/coverage)

+ 21 - 15
packages/ckeditor5-engine/src/model/delta/basic-transformations.js

@@ -348,15 +348,16 @@ addTransformationCase( WrapDelta, SplitDelta, ( a, b, context ) => {
 
 // Add special case for RenameDelta x SplitDelta transformation.
 addTransformationCase( RenameDelta, SplitDelta, ( a, b, context ) => {
-	const splitPosition = new Position( b.position.root, b.position.path.slice( 0, -1 ) );
+	const undoMode = context.aWasUndone || context.bWasUndone;
+	const posBeforeSplitParent = new Position( b.position.root, b.position.path.slice( 0, -1 ) );
 
 	const deltas = defaultTransform( a, b, context );
 
-	if ( a.operations[ 0 ].position.isEqual( splitPosition ) ) {
+	if ( !undoMode && a.operations[ 0 ].position.isEqual( posBeforeSplitParent ) ) {
 		// If a node that has been split has it's name changed, we should also change name of
 		// the node created during splitting.
 		const additionalRenameDelta = a.clone();
-		additionalRenameDelta.operations[ 0 ].position = a.operations[ 0 ].position.getShiftedBy( 1 );
+		additionalRenameDelta.operations[ 0 ].position = posBeforeSplitParent.getShiftedBy( 1 );
 
 		deltas.push( additionalRenameDelta );
 	}
@@ -365,20 +366,25 @@ addTransformationCase( RenameDelta, SplitDelta, ( a, b, context ) => {
 } );
 
 // Add special case for SplitDelta x RenameDelta transformation.
-addTransformationCase( SplitDelta, RenameDelta, ( a, b ) => {
-	a = a.clone();
-
-	const splitPosition = new Position( a.position.root, a.position.path.slice( 0, -1 ) );
-
-	if ( a._cloneOperation instanceof InsertOperation ) {
-		// If element to split had it's name changed, we have to reflect this change in an element
-		// that is in SplitDelta's InsertOperation.
-		if ( b.operations[ 0 ].position.isEqual( splitPosition ) ) {
-			a._cloneOperation.nodes.getNode( 0 ).name = b.operations[ 0 ].newName;
-		}
+addTransformationCase( SplitDelta, RenameDelta, ( a, b, context ) => {
+	const undoMode = context.aWasUndone || context.bWasUndone;
+	const posBeforeSplitParent = new Position( a.position.root, a.position.path.slice( 0, -1 ) );
+
+	// If element to split had it's name changed, we have to reflect this by creating additional rename operation.
+	if ( !undoMode && b.operations[ 0 ].position.isEqual( posBeforeSplitParent ) ) {
+		const additionalRenameDelta = b.clone();
+		additionalRenameDelta.operations[ 0 ].position = posBeforeSplitParent.getShiftedBy( 1 );
+
+		// `nodes` is a property that is available only if `SplitDelta` `a` has `InsertOperation`.
+		// `SplitDelta` may have `ReinsertOperation` instead of `InsertOperation`.
+		// However, such delta is only created when `MergeDelta` is reversed.
+		// So if this is not undo mode, it means that `SplitDelta` has `InsertOperation`.
+		additionalRenameDelta.operations[ 0 ].oldName = a._cloneOperation.nodes.getNode( 0 ).name;
+
+		return [ a.clone(), additionalRenameDelta ];
 	}
 
-	return [ a ];
+	return [ a.clone() ];
 } );
 
 // Add special case for RemoveDelta x SplitDelta transformation.

+ 20 - 9
packages/ckeditor5-engine/src/model/delta/transform.js

@@ -262,14 +262,18 @@ const transform = {
 							insertBefore: contextAB.insertBefore,
 							forceNotSticky: contextAB.forceNotSticky,
 							isStrong: contextAB.isStrong,
-							forceWeakRemove: contextAB.forceWeakRemove
+							forceWeakRemove: contextAB.forceWeakRemove,
+							aWasUndone: false,
+							bWasUndone: contextAB.bWasUndone
 						} );
 
 						const resultBA = transform.transform( deltaB[ l ], deltaA[ k ], {
 							insertBefore: !contextAB.insertBefore,
 							forceNotSticky: contextAB.forceNotSticky,
 							isStrong: !contextAB.isStrong,
-							forceWeakRemove: contextAB.forceWeakRemove
+							forceWeakRemove: contextAB.forceWeakRemove,
+							aWasUndone: contextAB.bWasUndone,
+							bWasUndone: false
 						} );
 
 						if ( useAdditionalContext ) {
@@ -350,10 +354,21 @@ function padWithNoOps( deltas, howMany ) {
 // Using data given in `context` object, sets `context.insertBefore` and `context.forceNotSticky` flags.
 // Also updates `context.wasAffected`.
 function _setContext( a, b, context ) {
+	_setBWasUndone( b, context );
 	_setWasAffected( a, b, context );
 	_setInsertBeforeContext( a, b, context );
 	_setForceWeakRemove( b, context );
-	_setForceNotSticky( b, context );
+	_setForceNotSticky( context );
+}
+
+// Sets `context.bWasUndone` basing on `context.document` history for `b` delta.
+//
+// `context.bWasUndone` is set to `true` if the (originally transformed) `b` delta was undone or was undoing delta.
+function _setBWasUndone( b, context ) {
+	const originalDelta = context.originalDelta.get( b );
+	const history = context.document.history;
+
+	context.bWasUndone = history.isUndoneDelta( originalDelta ) || history.isUndoingDelta( originalDelta );
 }
 
 // Sets `context.insertBefore` basing on `context.document` history for `a` by `b` transformation.
@@ -417,12 +432,8 @@ function _setInsertBeforeContext( a, b, context ) {
 // if delta `b` was already undone or if delta `b` is an undoing delta.
 //
 // This affects `MoveOperation` (and its derivatives).
-function _setForceNotSticky( b, context ) {
-	const history = context.document.history;
-	const originalB = context.originalDelta.get( b );
-
-	// If `b` delta is undoing or undone delta, stickiness should not be taken into consideration.
-	if ( history.isUndoingDelta( originalB ) || history.isUndoneDelta( originalB ) ) {
+function _setForceNotSticky( context ) {
+	if ( context.bWasUndone ) {
 		context.forceNotSticky = true;
 	}
 }

+ 1 - 1
packages/ckeditor5-engine/src/model/documentfragment.js

@@ -186,7 +186,7 @@ export default class DocumentFragment {
 		let node = this; // eslint-disable-line consistent-this
 
 		for ( const index of relativePath ) {
-			node = node.getChild( index );
+			node = node.getChild( node.offsetToIndex( index ) );
 		}
 
 		return node;

+ 1 - 1
packages/ckeditor5-engine/src/model/element.js

@@ -241,7 +241,7 @@ export default class Element extends Node {
 		let node = this; // eslint-disable-line consistent-this
 
 		for ( const index of relativePath ) {
-			node = node.getChild( index );
+			node = node.getChild( node.offsetToIndex( index ) );
 		}
 
 		return node;

+ 3 - 3
packages/ckeditor5-engine/src/view/node.js

@@ -222,7 +222,7 @@ export default class Node {
  * Change event is bubbled – it is fired on all ancestors.
  *
  * @event change:children
- * @param {module:engine/view/node~Node} Changed node.
+ * @param {module:engine/view/node~Node} changedNode
  */
 
 /**
@@ -231,7 +231,7 @@ export default class Node {
  * Change event is bubbled – it is fired on all ancestors.
  *
  * @event change:attributes
- * @param {module:engine/view/node~Node} Changed node.
+ * @param {module:engine/view/node~Node} changedNode
  */
 
 /**
@@ -240,7 +240,7 @@ export default class Node {
  * Change event is bubbled – it is fired on all ancestors.
  *
  * @event change:text
- * @param {module:engine/view/node~Node} Changed node.
+ * @param {module:engine/view/node~Node} changedNode
  */
 
 /**

+ 20 - 0
packages/ckeditor5-engine/tests/controller/getselectedcontent.js

@@ -309,6 +309,26 @@ describe( 'Delete utils', () => {
 				expect( content )
 					.to.equal( '<quote><heading1>bc</heading1></quote><heading1>x</heading1><quote><heading1>de</heading1></quote>' );
 			} );
+
+			// See: https://github.com/ckeditor/ckeditor5-engine/pull/1043#issuecomment-318012286
+			it( 'ensures that elements are retrieved by indexes instead of offsets', () => {
+				doc.schema.allow( { name: '$text', inside: '$root' } );
+				doc.schema.allow( { name: '$text', inside: 'quote' } );
+
+				setData( doc,
+					'foo' +
+					'<quote>' +
+						'<paragraph>' +
+							'b[ar' +
+						'</paragraph>' +
+						'bo]m' +
+					'</quote>'
+				);
+
+				const content = stringify( getSelectedContent( doc.selection ) );
+				expect( content )
+					.to.equal( '<paragraph>ar</paragraph>bo' );
+			} );
 		} );
 	} );
 } );

+ 15 - 3
packages/ckeditor5-engine/tests/model/delta/transform/splitdelta.js

@@ -625,14 +625,14 @@ describe( 'transform', () => {
 			it( 'renamed split element', () => {
 				const renameDelta = new RenameDelta();
 				renameDelta.addOperation( new RenameOperation(
-					new Position( root, [ 3, 3, 3 ] ), 'p', 'li', baseVersion
+					new Position( root, [ 3, 3, 3 ] ), 'h2', 'li', baseVersion
 				) );
 
 				const transformed = transform( splitDelta, renameDelta, context );
 
 				baseVersion = renameDelta.operations.length;
 
-				expect( transformed.length ).to.equal( 1 );
+				expect( transformed.length ).to.equal( 2 );
 
 				expectDelta( transformed[ 0 ], {
 					type: SplitDelta,
@@ -652,7 +652,18 @@ describe( 'transform', () => {
 					]
 				} );
 
-				expect( transformed[ 0 ].operations[ 0 ].nodes.getNode( 0 ).name ).to.equal( 'li' );
+				expectDelta( transformed[ 1 ], {
+					type: RenameDelta,
+					operations: [
+						{
+							type: RenameOperation,
+							position: new Position( root, [ 3, 3, 4 ] ),
+							oldName: 'p', // `oldName` taken from SplitDelta.
+							newName: 'li',
+							baseVersion: baseVersion + 2
+						}
+					]
+				} );
 			} );
 
 			it( 'split element is different than renamed element', () => {
@@ -699,6 +710,7 @@ describe( 'transform', () => {
 					new Position( root, [ 3, 3, 3 ] ), 'p', 'li', baseVersion
 				) );
 
+				context.aWasUndone = true;
 				const transformed = transform( splitDelta, renameDelta, context );
 
 				baseVersion = renameDelta.operations.length;

+ 50 - 2
packages/ckeditor5-engine/tests/model/documentfragment.js

@@ -309,16 +309,64 @@ describe( 'DocumentFragment', () => {
 		} );
 
 		it( 'should return a descendant of this node', () => {
+			const foo = new Text( 'foo' );
 			const image = new Element( 'image' );
 			const element = new Element( 'elem', [], [
 				new Element( 'elem', [], [
-					new Text( 'foo' ),
+					foo,
 					image
 				] )
 			] );
 			const frag = new DocumentFragment( element );
 
-			expect( frag.getNodeByPath( [ 0, 0, 1 ] ) ).to.equal( image );
+			expect( frag.getNodeByPath( [ 0, 0, 0 ] ) ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 0, 0, 1 ] ) ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 0, 0, 2 ] ) ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 0, 0, 3 ] ) ).to.equal( image );
+		} );
+
+		it( 'works fine with offsets', () => {
+			const abc = new Text( 'abc' );
+			const xyz = new Text( 'xyz' );
+			const bar = new Text( 'bar' );
+			const foo = new Text( 'foo' );
+			const bom = new Text( 'bom' );
+			const bold = new Element( 'b', [], [
+				bar
+			] );
+			const paragraph = new Element( 'paragraph', [], [
+				foo,
+				bold,
+				bom
+			] );
+			const frag = new DocumentFragment( [
+				abc,
+				paragraph,
+				xyz
+			] );
+
+			// abc<paragraph>foo<bold>bar</bold>bom</paragraph>xyz
+
+			expect( frag.getNodeByPath( [ 0 ] ), 1 ).to.equal( abc );
+			expect( frag.getNodeByPath( [ 1 ] ), 2 ).to.equal( abc );
+			expect( frag.getNodeByPath( [ 2 ] ), 3 ).to.equal( abc );
+			expect( frag.getNodeByPath( [ 3 ] ), 4 ).to.equal( paragraph );
+			expect( frag.getNodeByPath( [ 3, 0 ] ), 5 ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 3, 1 ] ), 6 ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 3, 2 ] ), 7 ).to.equal( foo );
+			expect( frag.getNodeByPath( [ 3, 3 ] ), 8 ).to.equal( bold );
+			expect( frag.getNodeByPath( [ 3, 3, 0 ] ), 9 ).to.equal( bar );
+			expect( frag.getNodeByPath( [ 3, 3, 1 ] ), 10 ).to.equal( bar );
+			expect( frag.getNodeByPath( [ 3, 3, 2 ] ), 11 ).to.equal( bar );
+			expect( frag.getNodeByPath( [ 3, 3, 3 ] ), 12 ).to.equal( null );
+			expect( frag.getNodeByPath( [ 3, 4 ] ), 13 ).to.equal( bom );
+			expect( frag.getNodeByPath( [ 3, 5 ] ), 14 ).to.equal( bom );
+			expect( frag.getNodeByPath( [ 3, 6 ] ), 15 ).to.equal( bom );
+			expect( frag.getNodeByPath( [ 3, 7 ] ), 16 ).to.equal( null );
+			expect( frag.getNodeByPath( [ 4 ] ), 17 ).to.equal( xyz );
+			expect( frag.getNodeByPath( [ 5 ] ), 18 ).to.equal( xyz );
+			expect( frag.getNodeByPath( [ 6 ] ), 19 ).to.equal( xyz );
+			expect( frag.getNodeByPath( [ 7 ] ), 20 ).to.equal( null );
 		} );
 	} );
 } );

+ 35 - 2
packages/ckeditor5-engine/tests/model/element.js

@@ -176,15 +176,48 @@ describe( 'Element', () => {
 		} );
 
 		it( 'should return a descendant of this node', () => {
+			const foo = new Text( 'foo' );
 			const image = new Element( 'image' );
 			const element = new Element( 'elem', [], [
 				new Element( 'elem', [], [
-					new Text( 'foo' ),
+					foo,
 					image
 				] )
 			] );
 
-			expect( element.getNodeByPath( [ 0, 1 ] ) ).to.equal( image );
+			expect( element.getNodeByPath( [ 0, 0 ] ) ).to.equal( foo );
+			expect( element.getNodeByPath( [ 0, 1 ] ) ).to.equal( foo );
+			expect( element.getNodeByPath( [ 0, 2 ] ) ).to.equal( foo );
+			expect( element.getNodeByPath( [ 0, 3 ] ) ).to.equal( image );
+		} );
+
+		it( 'works fine with offsets', () => {
+			const bar = new Text( 'bar' );
+			const foo = new Text( 'foo' );
+			const bom = new Text( 'bom' );
+			const bold = new Element( 'b', [], [
+				bar
+			] );
+			const paragraph = new Element( 'paragraph', [], [
+				foo,
+				bold,
+				bom
+			] );
+
+			// <paragraph>foo<bold>bar</bold>bom</paragraph>
+
+			expect( paragraph.getNodeByPath( [ 0 ] ) ).to.equal( foo );
+			expect( paragraph.getNodeByPath( [ 1 ] ) ).to.equal( foo );
+			expect( paragraph.getNodeByPath( [ 2 ] ) ).to.equal( foo );
+			expect( paragraph.getNodeByPath( [ 3 ] ) ).to.equal( bold );
+			expect( paragraph.getNodeByPath( [ 3, 0 ] ) ).to.equal( bar );
+			expect( paragraph.getNodeByPath( [ 3, 1 ] ) ).to.equal( bar );
+			expect( paragraph.getNodeByPath( [ 3, 2 ] ) ).to.equal( bar );
+			expect( paragraph.getNodeByPath( [ 3, 3 ] ) ).to.equal( null );
+			expect( paragraph.getNodeByPath( [ 4 ] ) ).to.equal( bom );
+			expect( paragraph.getNodeByPath( [ 5 ] ) ).to.equal( bom );
+			expect( paragraph.getNodeByPath( [ 6 ] ) ).to.equal( bom );
+			expect( paragraph.getNodeByPath( [ 7 ] ) ).to.equal( null );
 		} );
 	} );