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

Commit

Permalink
Merge pull request #1705 from ckeditor/t/ckeditor5/1645
Browse files Browse the repository at this point in the history
Fix: Attribute and remove change on intersecting ranges done in the same change block will be correctly saved in `Differ` and downcasted. Closes ckeditor/ckeditor5#1645.
  • Loading branch information
Reinmar committed Mar 28, 2019
2 parents 606827c + cb6b59c commit b2a9d86
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 2 deletions.
22 changes: 21 additions & 1 deletion src/model/differ.js
Original file line number Diff line number Diff line change
Expand Up @@ -821,6 +821,26 @@ export default class Differ {
}
}

if ( old.type == 'remove' ) {
// This is a case when attribute change "contains" remove change.
// The attribute change needs to be split into two because changes cannot intersect.
if ( inc.offset < old.offset && incEnd > old.offset ) {
const attributePart = {
type: 'attribute',
offset: old.offset,
howMany: incEnd - old.offset,
count: this._changeCount++
};

this._handleChange( attributePart, changes );

changes.push( attributePart );

inc.nodesToHandle = old.offset - inc.offset;
inc.howMany = inc.nodesToHandle;
}
}

if ( old.type == 'attribute' ) {
// There are only two conflicting scenarios possible here:
if ( inc.offset >= old.offset && incEnd <= oldEnd ) {
Expand Down Expand Up @@ -1087,7 +1107,7 @@ function _generateActionsFromChanges( oldChildrenLength, changes ) {
} else {
actions.push( ...'a'.repeat( change.howMany ).split( '' ) );

// The last handled offset isa at the position after the changed range.
// The last handled offset is at the position after the changed range.
offset = change.offset + change.howMany;
// We changed `howMany` old nodes, update `oldChildrenHandled`.
oldChildrenHandled += change.howMany;
Expand Down
42 changes: 41 additions & 1 deletion tests/model/differ.js
Original file line number Diff line number Diff line change
Expand Up @@ -998,7 +998,7 @@ describe( 'Differ', () => {
} );
} );

it( 'remove and add attribute on text', () => {
it( 'remove attribute and add attribute on text', () => {
const p = root.getChild( 1 );

p.getChild( 0 )._setAttribute( 'bold', true );
Expand Down Expand Up @@ -1283,6 +1283,46 @@ describe( 'Differ', () => {
] );
} );
} );

it( 'add attribute after some text was removed', () => {
const p = root.getChild( 0 );

const range = new Range( Position._createAt( p, 0 ), Position._createAt( p, 2 ) );
const position = Position._createAt( p, 1 );

model.change( () => {
remove( position, 1 );
attribute( range, 'a', null, true );

const type = 'attribute';
const attributeOldValue = null;
const attributeNewValue = true;

// Attribute change glueing does not work 100% correct.
expectChanges( [
{
type,
range: new Range( Position._createAt( p, 0 ), Position._createAt( p, 1 ) ),
attributeKey: 'a',
attributeOldValue,
attributeNewValue
},
{
type: 'remove',
position,
length: 1,
name: '$text'
},
{
type,
range: new Range( Position._createAt( p, 1 ), Position._createAt( p, 2 ) ),
attributeKey: 'a',
attributeOldValue,
attributeNewValue
}
] );
} );
} );
} );

describe( 'split', () => {
Expand Down

0 comments on commit b2a9d86

Please sign in to comment.