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

Commit

Permalink
Merge fd6dd3e into f6ad9af
Browse files Browse the repository at this point in the history
  • Loading branch information
scofalik authored Oct 5, 2018
2 parents f6ad9af + fd6dd3e commit 2dcc8b9
Show file tree
Hide file tree
Showing 5 changed files with 100 additions and 8 deletions.
38 changes: 32 additions & 6 deletions src/model/operation/transform.js
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,10 @@ class ContextFactory {
this._setRelation( opA, opB, 'mergeTargetNotMoved' );
}

if ( opA.sourcePosition.isEqual( opB.sourcePosition ) ) {
this._setRelation( opA, opB, 'mergeSameElement' );
}

break;
}
}
Expand Down Expand Up @@ -905,8 +909,10 @@ setTransformation( AttributeOperation, SplitOperation, ( a, b ) => {
// After attribute change:
// <listItem type="numbered">foo</listItem><listItem type="numbered">foo</listItem>
//
if ( a.range.end.isEqual( b.insertionPosition ) && !b.graveyardPosition ) {
a.range.end.offset++;
if ( a.range.end.isEqual( b.insertionPosition ) ) {
if ( !b.graveyardPosition ) {
a.range.end.offset++;
}

return [ a ];
}
Expand Down Expand Up @@ -1320,6 +1326,23 @@ setTransformation( MergeOperation, SplitOperation, ( a, b, context ) => {
}
}

// Case 2:
//
// Merge source is at the same position as split position. This sometimes happen during undo. This merge operation
// might have been earlier transformed by a merge operation which both merged the same element. See case in
// `MergeOperation` x `MergeOperation` transformation. In that case, if the merge operation has been undone, the special
// case is not applied.
//
// In this scenario the merge operation is now transformed by the split which has undone the previous merge operation.
// So now we are fixing situation which was skipped in `MergeOperation` x `MergeOperation` case.
//
if ( a.sourcePosition.isEqual( b.splitPosition ) && context.abRelation == 'mergeSameElement' ) {
a.sourcePosition = Position.createFromPosition( b.moveTargetPosition );
a.targetPosition = a.targetPosition._getTransformedBySplitOperation( b );

return [ a ];
}

// The default case.
//
if ( a.sourcePosition.hasSameParentAs( b.splitPosition ) ) {
Expand Down Expand Up @@ -1555,13 +1578,16 @@ setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
//
// In this case the default range transformation will not work correctly as the element created by
// split operation would be outside the range. The range to move needs to be fixed manually.
// Do it only if this is a "natural" split, not a one that comes from undo.
// TODO: this should be done on relations
//
const moveRange = Range.createFromPositionAndShift( a.sourcePosition, a.howMany );

if ( moveRange.end.isEqual( b.insertionPosition ) && !b.graveyardPosition ) {
a.howMany++;
if ( moveRange.end.isEqual( b.insertionPosition ) ) {
// Do it only if this is a "natural" split, not a one that comes from undo.
// If this is undo split, only `targetPosition` needs to be changed (if the move is a remove).
if ( !b.graveyardPosition ) {
a.howMany++;
}

a.targetPosition = newTargetPosition;

return [ a ];
Expand Down
9 changes: 8 additions & 1 deletion src/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,7 +517,14 @@ export default class Range {
*/
_getTransformedBySplitOperation( operation ) {
const start = this.start._getTransformedBySplitOperation( operation );
const end = this.end._getTransformedBySplitOperation( operation );

let end;

if ( this.end.isEqual( operation.insertionPosition ) ) {
end = this.end.getShiftedBy( 1 );
} else {
end = this.end._getTransformedBySplitOperation( operation );
}

return new Range( start, end );
}
Expand Down
46 changes: 46 additions & 0 deletions tests/model/operation/transform/undo.js
Original file line number Diff line number Diff line change
Expand Up @@ -275,4 +275,50 @@ describe( 'transform', () => {
john.redo();
expectClients( '<paragraph>Foo</paragraph>' );
} );

it( 'undo pasting', () => {
john.setData( '<paragraph>Foo[]Bar</paragraph>' );

// Below simulates pasting.
john.editor.model.change( () => {
john.split();
john.setSelection( [ 1 ] );

john.insert( '<paragraph>1</paragraph>' );
john.setSelection( [ 1 ] );
john.merge();

john.setSelection( [ 1 ] );
john.insert( '<paragraph>2</paragraph>' );
john.setSelection( [ 2 ] );
john.merge();
} );

expectClients( '<paragraph>Foo1</paragraph><paragraph>2Bar</paragraph>' );

john.undo();

expectClients( '<paragraph>FooBar</paragraph>' );
} );

it( 'selection attribute setting: split, bold, merge, undo, undo, undo', () => {
// This test is ported from undo to keep 100% CC in engine.
john.setData( '<paragraph>Foo[]</paragraph><paragraph>Bar</paragraph>' );

john.split();
john.setSelection( [ 1, 0 ] );
john._processExecute( 'bold' );
john._processExecute( 'forwardDelete' );

expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );

john.undo();
expectClients( '<paragraph>Foo</paragraph><paragraph selection:bold="true"></paragraph><paragraph>Bar</paragraph>' );

john.undo();
expectClients( '<paragraph>Foo</paragraph><paragraph></paragraph><paragraph>Bar</paragraph>' );

john.undo();
expectClients( '<paragraph>Foo</paragraph><paragraph>Bar</paragraph>' );
} );
} );
4 changes: 3 additions & 1 deletion tests/model/operation/transform/utils.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor';

import ListEditing from '@ckeditor/ckeditor5-list/src/listediting';
import BoldEditing from '@ckeditor/ckeditor5-basic-styles/src/bold/boldediting';
import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
import Typing from '@ckeditor/ckeditor5-typing/src/typing';
import UndoEditing from '@ckeditor/ckeditor5-undo/src/undoediting';
Expand Down Expand Up @@ -30,7 +31,8 @@ export class Client {
// Typing is needed for delete command.
// UndoEditing is needed for undo command.
// Block plugins are needed for proper data serializing.
plugins: [ Typing, Paragraph, ListEditing, UndoEditing, BlockQuoteEditing, HeadingEditing ]
// BoldEditing is needed for bold command.
plugins: [ Typing, Paragraph, ListEditing, UndoEditing, BlockQuoteEditing, HeadingEditing, BoldEditing ]
} ).then( editor => {
this.editor = editor;
this.document = editor.model.document;
Expand Down
11 changes: 11 additions & 0 deletions tests/model/range.js
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,17 @@ describe( 'Range', () => {
expect( transformed[ 0 ].start.path ).to.deep.equal( [ 1, 3 ] );
expect( transformed[ 0 ].end.path ).to.deep.equal( [ 1, 3 ] );
} );

it( 'split element which is the last element in the range', () => {
range = new Range( new Position( root, [ 1 ] ), new Position( root, [ 3 ] ) );

const op = new SplitOperation( new Position( root, [ 2, 0 ] ), 6, gyPos, 1 );
const transformed = range.getTransformedByOperation( op );

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

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

0 comments on commit 2dcc8b9

Please sign in to comment.