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

Commit

Permalink
Marker type shouldn't change during the runtime.
Browse files Browse the repository at this point in the history
  • Loading branch information
ma2ciek committed Feb 2, 2018
1 parent b7abc8f commit edfeb99
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 9 deletions.
10 changes: 9 additions & 1 deletion src/model/markercollection.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,15 @@ export default class MarkerCollection {
if ( oldMarker ) {
const oldRange = oldMarker.getRange();

if ( oldRange.isEqual( range ) && managedUsingOperations === oldMarker.managedUsingOperations ) {
if ( managedUsingOperations !== oldMarker.managedUsingOperations ) {
/**
* Marker's type should be consistent. Provide correct `managedUsingOperations` parameter.
*/
throw new CKEditorError( 'marker-set-incorrect-marker-type:' +
' Marker\'s type should be consistent. Provide correct `managedUsingOperations` parameter.' );
}

if ( oldRange.isEqual( range ) ) {
return oldMarker;
}

Expand Down
10 changes: 5 additions & 5 deletions src/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -815,24 +815,24 @@ export default class Writer {
* @param {Boolean} [options.usingOperation=false] Flag indicated whether the marker should be added by MarkerOperation.
* @returns {module:engine/model/markercollection~Marker} Marker that was set.
*/
setMarker( markerOrNameOrRange, rangeOrManagedUsingOperations, options ) {
setMarker( markerOrNameOrRange, rangeOrOptions, options ) {
this._assertWriterUsedCorrectly();

let markerName, newRange, usingOperation;

if ( markerOrNameOrRange instanceof Range ) {
markerName = uid();
newRange = markerOrNameOrRange;
usingOperation = !!rangeOrManagedUsingOperations && !!rangeOrManagedUsingOperations.usingOperation;
usingOperation = !!rangeOrOptions && !!rangeOrOptions.usingOperation;
} else {
markerName = typeof markerOrNameOrRange === 'string' ? markerOrNameOrRange : markerOrNameOrRange.name;

if ( rangeOrManagedUsingOperations instanceof Range ) {
newRange = rangeOrManagedUsingOperations;
if ( rangeOrOptions instanceof Range ) {
newRange = rangeOrOptions;
usingOperation = !!options && !!options.usingOperation;
} else {
newRange = null;
usingOperation = !!rangeOrManagedUsingOperations && !!rangeOrManagedUsingOperations.usingOperation;
usingOperation = !!rangeOrOptions && !!rangeOrOptions.usingOperation;
}
}

Expand Down
17 changes: 14 additions & 3 deletions tests/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -2004,19 +2004,22 @@ describe( 'Writer', () => {
} );

it( 'should accept marker instance', () => {
const marker = setMarker( 'name', range );
const marker = setMarker( 'name', range, { usingOperation: true } );
const range2 = Range.createFromParentsAndOffsets( root, 0, root, 0 );

setMarker( marker, range2, { usingOperation: true } );
const op = batch.deltas[ 0 ].operations[ 0 ];

expect( batch.deltas.length ).to.equal( 2 );

const op = batch.deltas[ 1 ].operations[ 0 ];

expect( model.markers.get( 'name' ).getRange().isEqual( range2 ) ).to.be.true;
expect( op.oldRange.isEqual( range ) ).to.be.true;
expect( op.newRange.isEqual( range2 ) ).to.be.true;
} );

it( 'should accept empty range parameter if marker instance is passed and usingOperation is set to true', () => {
const marker = setMarker( 'name', range );
const marker = setMarker( 'name', range, { usingOperation: true } );
const spy = sinon.spy();

model.on( 'applyOperation', spy );
Expand Down Expand Up @@ -2073,6 +2076,14 @@ describe( 'Writer', () => {
} ).to.throw( CKEditorError, /^writer-setMarker-no-range/ );
} );

it( 'should throw if marker is updated incorrectly', () => {
setMarker( 'name', range );

expect( () => {
setMarker( 'name', range, { usingOperation: true } );
} ).to.throw( CKEditorError, /^marker-set-incorrect-marker-type/ );
} );

it( 'should throw when trying to use detached writer', () => {
const marker = setMarker( 'name', range );
const writer = new Writer( model, batch );
Expand Down

0 comments on commit edfeb99

Please sign in to comment.