diff --git a/src/model/document.js b/src/model/document.js index 0739bdfe1..307c0f957 100644 --- a/src/model/document.js +++ b/src/model/document.js @@ -161,7 +161,7 @@ export default class Document { // Buffer marker changes. // This is not covered in buffering operations because markers may change outside of them (when they // are modified using `model.markers` collection, not through `MarkerOperation`). - this.listenTo( model.markers, 'add', ( evt, marker ) => { + this.listenTo( model.markers, 'set', ( evt, marker ) => { // TODO: Should filter out changes of markers that are not in document. // Whenever a new marker is added, buffer that change. this.differ.bufferMarkerChange( marker.name, null, marker.getRange() ); diff --git a/src/model/markercollection.js b/src/model/markercollection.js index f07cd526a..dcad1746a 100644 --- a/src/model/markercollection.js +++ b/src/model/markercollection.js @@ -15,14 +15,19 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; import mix from '@ckeditor/ckeditor5-utils/src/mix'; /** - * Creates, stores and manages {@link ~Marker markers}. + * The collection of all {@link module:engine/model/markercollection~Marker markers} attached to the document. + * It lets you {@link module:engine/model/markercollection~MarkerCollection#get get} markers or track them using + * {@link module:engine/model/markercollection~MarkerCollection#event:set} and + * {@link module:engine/model/markercollection~MarkerCollection#event:remove} events. * - * Markers are created by {@link ~MarkerCollection#set setting} a name for a {@link module:engine/model/liverange~LiveRange live range} - * in `MarkerCollection`. Name is used to group and identify markers. Names have to be unique, but markers can be grouped by - * using common prefixes, separated with `:`, for example: `user:john` or `search:3`. + * To create, change or remove makers use {@link module:engine/model/writer~Writer model writers'} methods: + * {@link module:engine/model/writer~Writer#setMarker} or {@link module:engine/model/writer~Writer#removeMarker}. Since + * the writer is the only proper way to change the data model it is not possible to change markers directly using this + * collection. All markers created by the writer will be automatically added to this collection. * - * Since markers are based on {@link module:engine/model/liverange~LiveRange live ranges}, for efficiency reasons, it's - * best to create and keep at least markers as possible. + * By default there is one marker collection available as {@link module:engine/model/model~Model#markers model property}. + * + * @see module:engine/model/markercollection~Marker */ export default class MarkerCollection { /** @@ -78,31 +83,33 @@ export default class MarkerCollection { * set is different, the marker in collection is removed and then new marker is added. If the range was same, nothing * happens and `false` is returned. * - * @fires module:engine/model/markercollection~MarkerCollection#event:add + * @protected + * @fires module:engine/model/markercollection~MarkerCollection#event:set * @fires module:engine/model/markercollection~MarkerCollection#event:remove - * @param {String|module:engine/model/markercollection~Marker} markerOrName Name of marker to add or Marker instance to update. + * @param {String|module:engine/model/markercollection~Marker} markerOrName Name of marker to set or marker instance to update. * @param {module:engine/model/range~Range} range Marker range. + * @param {Boolean} managedUsingOperations Specifies whether the marker is managed using operations. * @returns {module:engine/model/markercollection~Marker} `Marker` instance added to the collection. */ - set( markerOrName, range ) { + _set( markerOrName, range, managedUsingOperations ) { const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName; const oldMarker = this._markers.get( markerName ); if ( oldMarker ) { const oldRange = oldMarker.getRange(); - if ( oldRange.isEqual( range ) ) { + if ( oldRange.isEqual( range ) && managedUsingOperations === oldMarker.managedUsingOperations ) { return oldMarker; } - this.remove( markerName ); + this._remove( markerName ); } const liveRange = LiveRange.createFromRange( range ); - const marker = new Marker( markerName, liveRange ); + const marker = new Marker( markerName, liveRange, managedUsingOperations ); this._markers.set( markerName, marker ); - this.fire( 'add:' + markerName, marker ); + this.fire( 'set:' + markerName, marker ); return marker; } @@ -110,10 +117,11 @@ export default class MarkerCollection { /** * Removes given {@link ~Marker marker} or a marker with given name from the `MarkerCollection`. * + * @protected * @param {String} markerOrName Marker or name of a marker to remove. * @returns {Boolean} `true` if marker was found and removed, `false` otherwise. */ - remove( markerOrName ) { + _remove( markerOrName ) { const markerName = markerOrName instanceof Marker ? markerOrName.name : markerOrName; const oldMarker = this._markers.get( markerName ); @@ -190,10 +198,10 @@ export default class MarkerCollection { } /** - * Fired whenever marker is added to `MarkerCollection`. + * Fired whenever marker is added or updated in `MarkerCollection`. * - * @event add - * @param {module:engine/model/markercollection~Marker} The added marker. + * @event set + * @param {module:engine/model/markercollection~Marker} The set marker. */ /** @@ -209,30 +217,64 @@ mix( MarkerCollection, EmitterMixin ); /** * `Marker` is a continuous parts of model (like a range), is named and represent some kind of information about marked * part of model document. In contrary to {@link module:engine/model/node~Node nodes}, which are building blocks of - * model document tree, markers are not stored directly in document tree. Still, they are document data, by giving + * model document tree, markers are not stored directly in document tree but in + * {@link module:engine/model/model~Model#markers model markers' collection}. Still, they are document data, by giving * additional meaning to the part of a model document between marker start and marker end. * * In this sense, markers are similar to adding and converting attributes on nodes. The difference is that attribute is * connected with a given node (e.g. a character is bold no matter if it gets moved or content around it changes). - * Markers on the other hand are continuous ranges and are characterised by their start and end position. This means that + * Markers on the other hand are continuous ranges and are characterized by their start and end position. This means that * any character in the marker is marked by the marker. For example, if a character is moved outside of marker it stops being * "special" and the marker is shrunk. Similarly, when a character is moved into the marker from other place in document * model, it starts being "special" and the marker is enlarged. * - * Since markers are based on {@link module:engine/model/liverange~LiveRange live ranges}, for efficiency reasons, it's - * best to create and keep at least markers as possible. + * Another upside of markers is that finding marked part of document is fast and easy. Using attributes to mark some nodes + * and then trying to find that part of document would require traversing whole document tree. Marker gives instant access + * to the range which it is marking at the moment. + * + * Markers are built from a name and a range. + * + * Range of the marker is updated automatically when document changes, using + * {@link module:engine/model/liverange~LiveRange live range} mechanism. + * + * Name is used to group and identify markers. Names have to be unique, but markers can be grouped by + * using common prefixes, separated with `:`, for example: `user:john` or `search:3`. That's useful in term of creating + * namespaces for custom elements (e.g. comments, highlights). You can use this prefixes in + * {@link module:engine/model/markercollection~MarkerCollection#event:set} listeners to listen on changes in a group of markers. + * For instance: `model.markers.on( 'set:user', callback );` will be called whenever any `user:*` markers changes. + * + * There are two types of markers. + * + * 1. Markers managed directly, without using operations. They are added directly by {@link module:engine/model/writer~Writer} + * to the {@link module:engine/model/markercollection~MarkerCollection} without any additional mechanism. They can be used + * as bookmarks or visual markers. They are great for showing results of the find, or select link when the focus is in the input. + * + * 1. Markers managed using operations. These markers are also stored in {@link module:engine/model/markercollection~MarkerCollection} + * but changes in these markers is managed the same way all other changes in the model structure - using operations. + * Therefore, they are handled in the undo stack and synchronized between clients if the collaboration plugin is enabled. + * This type of markers is useful for solutions like spell checking or comments. + * + * Both type of them should be added / updated by {@link module:engine/model/writer~Writer#setMarker} + * and removed by {@link module:engine/model/writer~Writer#removeMarker} methods. + * + * model.change( ( writer ) => { + * const marker = writer.setMarker( name, range, { usingOperation: true } ); + * + * // ... + * + * writer.removeMarker( marker ); + * } ); + * + * See {@link module:engine/model/writer~Writer} to find more examples. + * + * Since markers need to track change in the document, for efficiency reasons, it is best to create and keep as little + * markers as possible and remove them as soon as they are not needed anymore. * * Markers can be converted to view by adding appropriate converters for * {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher#event:addMarker} and * {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher#event:removeMarker} * events, or by building converters for {@link module:engine/conversion/modelconversiondispatcher~ModelConversionDispatcher} * using {@link module:engine/conversion/buildmodelconverter~buildModelConverter model converter builder}. - * - * Another upside of markers is that finding marked part of document is fast and easy. Using attributes to mark some nodes - * and then trying to find that part of document would require traversing whole document tree. Marker gives instant access - * to the range which it is marking at the moment. - * - * `Marker` instances are created and destroyed only by {@link ~MarkerCollection MarkerCollection}. */ class Marker { /** @@ -241,20 +283,29 @@ class Marker { * @param {String} name Marker name. * @param {module:engine/model/liverange~LiveRange} liveRange Range marked by the marker. */ - constructor( name, liveRange ) { + constructor( name, liveRange, managedUsingOperations ) { /** * Marker's name. * * @readonly - * @member {String} #name + * @type {String} */ this.name = name; + /** + * Flag indicates if the marker is managed using operations or not. See {@link ~Marker marker class description} + * to learn more about marker types. See {@link module:engine/model/writer~Writer#setMarker}. + * + * @readonly + * @type {Boolean} + */ + this.managedUsingOperations = managedUsingOperations; + /** * Range marked by the marker. * * @protected - * @member {module:engine/model/liverange~LiveRange} #_liveRange + * @type {module:engine/model/liverange~LiveRange} */ this._liveRange = liveRange; diff --git a/src/model/operation/markeroperation.js b/src/model/operation/markeroperation.js index 381e866f5..5bc3bccad 100644 --- a/src/model/operation/markeroperation.js +++ b/src/model/operation/markeroperation.js @@ -87,9 +87,9 @@ export default class MarkerOperation extends Operation { * @inheritDoc */ _execute() { - const type = this.newRange ? 'set' : 'remove'; + const type = this.newRange ? '_set' : '_remove'; - this._markers[ type ]( this.name, this.newRange ); + this._markers[ type ]( this.name, this.newRange, true ); } /** diff --git a/src/model/writer.js b/src/model/writer.js index 8283d7092..4675098fb 100644 --- a/src/model/writer.js +++ b/src/model/writer.js @@ -40,6 +40,7 @@ import DocumentSelection from './documentselection'; import toMap from '@ckeditor/ckeditor5-utils/src/tomap'; import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror'; +import uid from '@ckeditor/ckeditor5-utils/src/uid'; /** * Model writer it the proper way of modifying model. It should be used whenever you wants to create node, modify @@ -194,7 +195,7 @@ export default class Writer { markerRange.end._getCombined( rangeRootPosition, position ) ); - this.setMarker( markerName, range ); + this.setMarker( markerName, range, { usingOperation: true } ); } } } @@ -471,7 +472,7 @@ export default class Writer { const delta = new RemoveDelta(); this.batch.addDelta( delta ); - addRemoveOperation( position, howMany, delta, this.model ); + applyRemoveOperation( position, howMany, delta, this.model ); }; if ( itemOrRange instanceof Range ) { @@ -539,7 +540,7 @@ export default class Writer { delta.addOperation( move ); this.model.applyOperation( move ); - addRemoveOperation( position, 1, delta, this.model ); + applyRemoveOperation( position, 1, delta, this.model ); } /** @@ -756,37 +757,99 @@ export default class Writer { delta.addOperation( move ); this.model.applyOperation( move ); - addRemoveOperation( Position.createBefore( element ), 1, delta, this.model ); + applyRemoveOperation( Position.createBefore( element ), 1, delta, this.model ); } /** - * Adds or updates {@link module:engine/model/markercollection~Marker marker} with given name to given `range`. + * Adds or updates {@link module:engine/model/markercollection~Marker marker}. * - * If passed name is a name of already existing marker (or {@link module:engine/model/markercollection~Marker Marker} instance - * is passed), `range` parameter may be omitted. In this case marker will not be updated in - * {@link module:engine/model/model~Model#markers document marker collection}. However the marker will be added to - * the document history. This may be important for other features, like undo. From document history point of view, it will - * look like the marker was created and added to the document at the moment when it is set using this method. + * As the first parameter you can set marker name or instance. If none of them is provided, new marker, with a unique + * name is created and returned. * - * This is useful if the marker is created before it can be added to document history (e.g. a feature creating the marker - * is waiting for additional data, etc.). In this case, the marker may be first created directly through - * {@link module:engine/model/markercollection~MarkerCollection MarkerCollection API} and only later added using `Batch` API. + * Using this method you can change markers range or define if the marker is managed by operation or not. * - * @param {module:engine/model/markercollection~Marker|String} markerOrName Marker or marker name to add or update. - * @param {module:engine/model/range~Range} [newRange] Marker range. + * Marker tracks changes is the document and updates the range automatically, so you need to update the range only + * when it changes directly. You do not need to update it after each document change. + * + * The option parameter let you decide if the marker should be managed by operations or not. See + * {@link module:engine/model/markercollection~Marker marker class description} to learn about the difference between + * markers managed by operation and managed directly. You can change this option for existing marker. This is + * useful if a marker have been created earlier and need to be added to the document history later. + * + * Update marker using operation: + * + * setMarker( marker, range, { usingOperation: true } ); + * + * Create/update marker directly base on marker's name: + * + * setMarker( markerName, range ); + * + * Create marker with a unique id using operation: + * + * setMarker( range, { usingOperation: true } ); + * + * Create marker directly with a unique name: + * + * setMarker( range ) + * + * Change marker's option (start using operations to manage it): + * + * setMarker( marker, { usingOperation: true } ); + * + * Note: For efficiency reasons, it's best to create and keep as little markers as possible. + * + * @see module:engine/model/markercollection~Marker + * @param {module:engine/model/markercollection~Marker|String} [markerOrName=uid()] + * Name of marker to add, Marker instance to update or range for the marker with a unique name. + * @param {module:engine/model/range~Range|Object} [range] Marker range or options. + * @param {Object} [options] + * @param {Boolean} [options.usingOperation=false] Flag indicated whether the marker should be added by MarkerOperation. + * See {@link module:engine/model/markercollection~Marker#managedUsingOperations}. + * @returns {module:engine/model/markercollection~Marker} Marker that was set. */ - setMarker( markerOrName, newRange ) { + setMarker( markerOrNameOrRange, rangeOrOptions, options ) { this._assertWriterUsedCorrectly(); - const name = typeof markerOrName == 'string' ? markerOrName : markerOrName.name; - const currentMarker = this.model.markers.get( name ); + let markerName, newRange, usingOperation; + + if ( markerOrNameOrRange instanceof Range ) { + markerName = uid(); + newRange = markerOrNameOrRange; + usingOperation = !!rangeOrOptions && !!rangeOrOptions.usingOperation; + } else { + markerName = typeof markerOrNameOrRange === 'string' ? markerOrNameOrRange : markerOrNameOrRange.name; + + if ( rangeOrOptions instanceof Range ) { + newRange = rangeOrOptions; + usingOperation = !!options && !!options.usingOperation; + } else { + newRange = null; + usingOperation = !!rangeOrOptions && !!rangeOrOptions.usingOperation; + } + } + + const currentMarker = this.model.markers.get( markerName ); + + if ( !usingOperation ) { + if ( !newRange ) { + /** + * Range parameter is required when adding a new marker. + * + * @error writer-setMarker-no-range + */ + throw new CKEditorError( 'writer-setMarker-no-range: Range parameter is required when adding a new marker.' ); + } + + // If marker changes to marker that do not use operations then we need to create additional operation + // that removes that marker first. + if ( currentMarker && currentMarker.managedUsingOperations && !usingOperation ) { + applyMarkerOperation( this, markerName, currentMarker.getRange(), null ); + } + + return this.model.markers._set( markerName, newRange, usingOperation ); + } if ( !newRange && !currentMarker ) { - /** - * Range parameter is required when adding a new marker. - * - * @error writer-setMarker-no-range - */ throw new CKEditorError( 'writer-setMarker-no-range: Range parameter is required when adding a new marker.' ); } @@ -795,17 +858,22 @@ export default class Writer { if ( !newRange ) { // If `newRange` is not given, treat this as synchronizing existing marker. // Create `MarkerOperation` with `oldRange` set to `null`, so reverse operation will remove the marker. - addMarkerOperation( this, name, null, currentRange ); + applyMarkerOperation( this, markerName, null, currentRange ); } else { // Just change marker range. - addMarkerOperation( this, name, currentRange, newRange ); + applyMarkerOperation( this, markerName, currentRange, newRange ); } + + return this.model.markers.get( markerName ); } /** * Removes given {@link module:engine/model/markercollection~Marker marker} or marker with given name. + * The marker is removed accordingly to how it has been created, so if the marker was created using operation, + * it will be destroyed using operation. * * @param {module:engine/model/markercollection~Marker|String} markerOrName Marker or marker name to remove. + * @param {Object} [options] */ removeMarker( markerOrName ) { this._assertWriterUsedCorrectly(); @@ -821,9 +889,17 @@ export default class Writer { throw new CKEditorError( 'writer-removeMarker-no-marker: Trying to remove marker which does not exist.' ); } - const oldRange = this.model.markers.get( name ).getRange(); + const marker = this.model.markers.get( name ); + + if ( !marker.managedUsingOperations ) { + this.model.markers._remove( name ); + + return; + } + + const oldRange = marker.getRange(); - addMarkerOperation( this, name, oldRange, null ); + applyMarkerOperation( this, name, oldRange, null ); } /** @@ -1114,14 +1190,14 @@ function setAttributeOnItem( writer, key, value, item ) { } } -// Creates and adds marker operation to {@link module:engine/model/delta/delta~Delta delta}. +// Creates and applies marker operation to {@link module:engine/model/delta/delta~Delta delta}. // // @private // @param {module:engine/model/writer~Writer} writer // @param {String} name Marker name. // @param {module:engine/model/range~Range} oldRange Marker range before the change. // @param {module:engine/model/range~Range} newRange Marker range after the change. -function addMarkerOperation( writer, name, oldRange, newRange ) { +function applyMarkerOperation( writer, name, oldRange, newRange ) { const model = writer.model; const doc = model.document; const delta = new MarkerDelta(); @@ -1141,7 +1217,7 @@ function addMarkerOperation( writer, name, oldRange, newRange ) { // @param {Number} howMany Number of nodes to remove. // @param {module:engine/model/delta~Delta} delta Delta to add new operation to. // @param {module:engine/model/model~Model} model Model instance on which operation will be applied. -function addRemoveOperation( position, howMany, delta, model ) { +function applyRemoveOperation( position, howMany, delta, model ) { let operation; if ( position.root.document ) { diff --git a/tests/controller/datacontroller.js b/tests/controller/datacontroller.js index 341a8c71e..577950384 100644 --- a/tests/controller/datacontroller.js +++ b/tests/controller/datacontroller.js @@ -321,7 +321,7 @@ describe( 'DataController', () => { model.change( writer => { writer.insert( modelElement, modelRoot, 0 ); - writer.setMarker( 'marker:a', Range.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ) ); + writer.setMarker( 'marker:a', Range.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ), { usingOperation: true } ); } ); const viewDocumentFragment = data.toView( modelElement ); @@ -343,8 +343,8 @@ describe( 'DataController', () => { model.change( writer => { writer.insert( modelElement, modelRoot, 0 ); - writer.setMarker( 'marker:a', Range.createFromParentsAndOffsets( modelP1, 1, modelP1, 3 ) ); - writer.setMarker( 'marker:b', Range.createFromParentsAndOffsets( modelP2, 0, modelP2, 2 ) ); + writer.setMarker( 'marker:a', Range.createFromParentsAndOffsets( modelP1, 1, modelP1, 3 ), { usingOperation: true } ); + writer.setMarker( 'marker:b', Range.createFromParentsAndOffsets( modelP2, 0, modelP2, 2 ), { usingOperation: true } ); } ); const viewDocumentFragment = data.toView( modelP1 ); diff --git a/tests/controller/editingcontroller.js b/tests/controller/editingcontroller.js index baa866a3f..87452a126 100644 --- a/tests/controller/editingcontroller.js +++ b/tests/controller/editingcontroller.js @@ -232,8 +232,8 @@ describe( 'EditingController', () => { it( 'should convert adding marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ) @@ -243,12 +243,12 @@ describe( 'EditingController', () => { it( 'should convert removing marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ) @@ -258,14 +258,14 @@ describe( 'EditingController', () => { it( 'should convert changing marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); const range2 = new ModelRange( new ModelPosition( modelRoot, [ 0, 0 ] ), new ModelPosition( modelRoot, [ 0, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range2 ); + model.change( writer => { + writer.setMarker( 'marker', range2 ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ) @@ -275,11 +275,8 @@ describe( 'EditingController', () => { it( 'should convert insertion into marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); - } ); - model.change( writer => { + writer.setMarker( 'marker', range ); writer.insertText( 'xyz', new ModelPosition( modelRoot, [ 1, 0 ] ) ); } ); @@ -290,8 +287,8 @@ describe( 'EditingController', () => { it( 'should convert move to marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); model.change( writer => { @@ -308,8 +305,8 @@ describe( 'EditingController', () => { it( 'should convert move from marker', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 2, 2 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); model.change( writer => { @@ -326,8 +323,8 @@ describe( 'EditingController', () => { it( 'should convert the whole marker move', () => { const range = new ModelRange( new ModelPosition( modelRoot, [ 0, 1 ] ), new ModelPosition( modelRoot, [ 0, 3 ] ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); model.change( writer => { @@ -386,7 +383,7 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it will be affected by insert operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -404,7 +401,7 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it will be affected by remove operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -422,7 +419,7 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it will be affected by move operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -442,7 +439,11 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it will be affected by rename operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ) ); + writer.setMarker( + 'marker', + ModelRange.createFromParentsAndOffsets( modelRoot, 0, modelRoot, 1 ), + { usingOperation: true } + ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -460,7 +461,7 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it will be affected by marker operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -472,7 +473,7 @@ describe( 'EditingController', () => { model.change( writer => { const p2 = p1.nextSibling; - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p2, 1, p2, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p2, 1, p2, 2 ), { usingOperation: true } ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ).to.equal( '

foo

bar

' ); @@ -480,7 +481,7 @@ describe( 'EditingController', () => { it( 'should remove marker from view if it is removed through marker collection', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -489,8 +490,8 @@ describe( 'EditingController', () => { expect( getViewData( editing.view, { withoutSelection: true } ) ).to.equal( '

foo

bar

' ); }, { priority: 'low' } ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ).to.equal( '

foo

bar

' ); @@ -498,7 +499,7 @@ describe( 'EditingController', () => { it( 'should not remove marker if applied operation is an attribute operation', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); // Adding with 'high' priority, because `applyOperation` is decorated - its default callback is fired with 'normal' priority. @@ -516,7 +517,7 @@ describe( 'EditingController', () => { it( 'should not crash if multiple operations affect a marker', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); model.change( writer => { @@ -530,12 +531,12 @@ describe( 'EditingController', () => { it( 'should not crash if marker is removed, added and removed #1', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); model.change( writer => { writer.insertText( 'a', p1, 0 ); - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 3, p1, 4 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 3, p1, 4 ), { usingOperation: true } ); writer.insertText( 'a', p1, 0 ); } ); @@ -544,13 +545,13 @@ describe( 'EditingController', () => { it( 'should not crash if marker is removed, added and removed #2', () => { model.change( writer => { - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ) ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 1, p1, 2 ), { usingOperation: true } ); } ); model.change( writer => { - writer.removeMarker( 'marker' ); - writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 0, p1, 1 ) ); - writer.removeMarker( 'marker' ); + writer.removeMarker( 'marker', { usingOperation: true } ); + writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( p1, 0, p1, 1 ), { usingOperation: true } ); + writer.removeMarker( 'marker', { usingOperation: true } ); } ); expect( getViewData( editing.view, { withoutSelection: true } ) ).to.equal( '

foo

bar

' ); diff --git a/tests/conversion/buildmodelconverter.js b/tests/conversion/buildmodelconverter.js index db7c18c68..ceec5b6c0 100644 --- a/tests/conversion/buildmodelconverter.js +++ b/tests/conversion/buildmodelconverter.js @@ -348,8 +348,8 @@ describe( 'Model converter builder', () => { attributes: { title: 'highlight title' } } ); - model.change( () => { - model.markers.set( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); + model.change( writer => { + writer.setMarker( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -363,8 +363,8 @@ describe( 'Model converter builder', () => { expect( viewRoot.getChild( 0 ).getChild( 1 ).priority ).to.equal( 3 ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -377,8 +377,8 @@ describe( 'Model converter builder', () => { attributes: { title: 'highlight title' } } ) ); - model.change( () => { - model.markers.set( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); + model.change( writer => { + writer.setMarker( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -392,8 +392,8 @@ describe( 'Model converter builder', () => { expect( viewRoot.getChild( 0 ).getChild( 1 ).priority ).to.equal( 12 ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -404,14 +404,14 @@ describe( 'Model converter builder', () => { class: 'highlight' } ); - model.change( () => { - model.markers.set( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 2 ) ); + model.change( writer => { + writer.setMarker( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 2 ) ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -421,8 +421,8 @@ describe( 'Model converter builder', () => { buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toHighlight( { class: 'highlight' } ); buildModelConverter().for( dispatcher ).fromMarker( 'search' ).withPriority( 'high' ).toHighlight( { class: 'override' } ); - model.change( () => { - model.markers.set( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); + model.change( writer => { + writer.setMarker( 'search', ModelRange.createFromParentsAndOffsets( modelElement, 2, modelElement, 4 ) ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -468,14 +468,14 @@ describe( 'Model converter builder', () => { it( 'using passed view element name', () => { buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toElement( 'span' ); - model.change( () => { - model.markers.set( 'search', range ); + model.change( writer => { + writer.setMarker( 'search', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -485,14 +485,14 @@ describe( 'Model converter builder', () => { const viewElement = new ViewUIElement( 'span', { class: 'search' } ); buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toElement( viewElement ); - model.change( () => { - model.markers.set( 'search', range ); + model.change( writer => { + writer.setMarker( 'search', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -505,14 +505,14 @@ describe( 'Model converter builder', () => { return new ViewUIElement( 'span', { class: className } ); } ); - model.change( () => { - model.markers.set( 'search:red', range ); + model.change( writer => { + writer.setMarker( 'search:red', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search:red' ); + model.change( writer => { + writer.removeMarker( 'search:red' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -527,14 +527,14 @@ describe( 'Model converter builder', () => { it( 'using passed view element name', () => { buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toElement( 'span' ); - model.change( () => { - model.markers.set( 'search', range ); + model.change( writer => { + writer.setMarker( 'search', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -544,16 +544,16 @@ describe( 'Model converter builder', () => { const viewElement = new ViewUIElement( 'span', { class: 'search' } ); buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toElement( viewElement ); - model.change( () => { - model.markers.set( 'search', range ); + model.change( writer => { + writer.setMarker( 'search', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search' ); + model.change( writer => { + writer.removeMarker( 'search' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -566,16 +566,16 @@ describe( 'Model converter builder', () => { return new ViewUIElement( 'span', { class: className } ); } ); - model.change( () => { - model.markers.set( 'search:red', range ); + model.change( writer => { + writer.setMarker( 'search:red', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'search:red' ); + model.change( writer => { + writer.removeMarker( 'search:red' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -588,8 +588,8 @@ describe( 'Model converter builder', () => { buildModelConverter().for( dispatcher ).fromMarker( 'search' ).toElement( 'normal' ); buildModelConverter().for( dispatcher ).fromMarker( 'search' ).withPriority( 'high' ).toElement( 'high' ); - model.change( () => { - model.markers.set( 'search', range ); + model.change( writer => { + writer.setMarker( 'search', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); diff --git a/tests/conversion/model-selection-to-view-converters.js b/tests/conversion/model-selection-to-view-converters.js index e09ac0ca5..29a12e2be 100644 --- a/tests/conversion/model-selection-to-view-converters.js +++ b/tests/conversion/model-selection-to-view-converters.js @@ -176,6 +176,8 @@ describe( 'model-selection-to-view-converters', () => { } ); describe( 'collapsed selection', () => { + let marker; + it( 'in container', () => { test( [ 1, 1 ], @@ -194,9 +196,9 @@ describe( 'model-selection-to-view-converters', () => { it( 'in attribute and marker', () => { setModelData( model, 'fo<$text bold="true">obar' ); - const marker = model.markers.set( 'marker', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); model.change( writer => { + marker = writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); writer.setSelection( new ModelRange( ModelPosition.createAt( modelRoot, 3 ) ) ); } ); @@ -218,9 +220,9 @@ describe( 'model-selection-to-view-converters', () => { it( 'in attribute and marker - no attribute', () => { setModelData( model, 'fo<$text bold="true">obar' ); - const marker = model.markers.set( 'marker', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); model.change( writer => { + marker = writer.setMarker( 'marker', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); writer.setSelection( new ModelRange( ModelPosition.createAt( modelRoot, 3 ) ) ); writer.removeSelectionAttribute( 'bold' ); } ); @@ -246,9 +248,9 @@ describe( 'model-selection-to-view-converters', () => { ) ); setModelData( model, 'foobar' ); - const marker = model.markers.set( 'marker2', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); model.change( writer => { + marker = writer.setMarker( 'marker2', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); writer.setSelection( new ModelRange( ModelPosition.createAt( modelRoot, 3 ) ) ); } ); @@ -271,9 +273,9 @@ describe( 'model-selection-to-view-converters', () => { dispatcher.on( 'addMarker:marker3', highlightText( () => null ) ); setModelData( model, 'foobar' ); - const marker = model.markers.set( 'marker3', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); model.change( writer => { + marker = writer.setMarker( 'marker3', ModelRange.createFromParentsAndOffsets( modelRoot, 1, modelRoot, 5 ) ); writer.setSelection( new ModelRange( ModelPosition.createAt( modelRoot, 3 ) ) ); } ); diff --git a/tests/conversion/model-to-view-converters.js b/tests/conversion/model-to-view-converters.js index a0a82eea3..25dc83f44 100644 --- a/tests/conversion/model-to-view-converters.js +++ b/tests/conversion/model-to-view-converters.js @@ -370,14 +370,14 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -389,14 +389,14 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( () => viewUi ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( () => viewUi ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -422,14 +422,14 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( () => null ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( () => null ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -447,15 +447,15 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ) .to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -467,15 +467,15 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( viewUi ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( viewUi ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ) .to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -493,16 +493,16 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', insertUIElement( creator ) ); dispatcher.on( 'removeMarker:marker', removeUIElement( creator ) ); - model.change( () => { - model.markers.set( 'marker', range ); + model.change( writer => { + writer.setMarker( 'marker', range ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foobar

' ); @@ -726,8 +726,8 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', highlightElement( highlightDescriptor ) ); dispatcher.on( 'removeMarker:marker', removeHighlight( highlightDescriptor ) ); - model.change( () => { - model.markers.set( 'marker', markerRange ); + model.change( writer => { + writer.setMarker( 'marker', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -741,8 +741,8 @@ describe( 'model-to-view-converters', () => { '' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foo

bar

' ); @@ -759,8 +759,8 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', highlightElement( newDescriptor ), { priority: 'high' } ); dispatcher.on( 'removeMarker:marker', removeHighlight( newDescriptor ), { priority: 'high' } ); - model.change( () => { - model.markers.set( 'marker', markerRange ); + model.change( writer => { + writer.setMarker( 'marker', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -774,8 +774,8 @@ describe( 'model-to-view-converters', () => { '' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foo

bar

' ); @@ -786,14 +786,14 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', highlightElement( () => null ), { priority: 'high' } ); dispatcher.on( 'removeMarker:marker', removeHighlight( () => null ), { priority: 'high' } ); - model.change( () => { - model.markers.set( 'marker', markerRange ); + model.change( writer => { + writer.setMarker( 'marker', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foo

bar

' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '

foo

bar

' ); @@ -840,8 +840,8 @@ describe( 'model-to-view-converters', () => { } ); it( 'should use addHighlight and removeHighlight on elements and not convert children nodes', () => { - model.change( () => { - model.markers.set( 'marker', markerRange ); + model.change( writer => { + writer.setMarker( 'marker', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -852,8 +852,8 @@ describe( 'model-to-view-converters', () => { '' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '
foo
' ); @@ -866,8 +866,8 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker', highlightElement( newDescriptor ), { priority: 'high' } ); dispatcher.on( 'removeMarker:marker', removeHighlight( newDescriptor ), { priority: 'high' } ); - model.change( () => { - model.markers.set( 'marker', markerRange ); + model.change( writer => { + writer.setMarker( 'marker', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( @@ -878,8 +878,8 @@ describe( 'model-to-view-converters', () => { '' ); - model.change( () => { - model.markers.remove( 'marker' ); + model.change( writer => { + writer.removeMarker( 'marker' ); } ); expect( viewToString( viewRoot ) ).to.equal( '
foo
' ); @@ -901,8 +901,8 @@ describe( 'model-to-view-converters', () => { expect( id ).to.equal( 'marker:foo-bar-baz' ); } ); - model.change( () => { - model.markers.set( 'marker2', markerRange ); + model.change( writer => { + writer.setMarker( 'marker2', markerRange ); } ); } ); @@ -911,14 +911,14 @@ describe( 'model-to-view-converters', () => { dispatcher.on( 'addMarker:marker2', highlightElement( () => null ) ); dispatcher.on( 'removeMarker:marker2', removeHighlight( () => null ) ); - model.change( () => { - model.markers.set( 'marker2', markerRange ); + model.change( writer => { + writer.setMarker( 'marker2', markerRange ); } ); expect( viewToString( viewRoot ) ).to.equal( '
foo
' ); - model.change( () => { - model.markers.remove( 'marker2' ); + model.change( writer => { + writer.removeMarker( 'marker2' ); } ); expect( viewToString( viewRoot ) ).to.equal( '
foo
' ); diff --git a/tests/conversion/modelconversiondispatcher.js b/tests/conversion/modelconversiondispatcher.js index 287d850ec..879f5df42 100644 --- a/tests/conversion/modelconversiondispatcher.js +++ b/tests/conversion/modelconversiondispatcher.js @@ -333,10 +333,9 @@ describe( 'ModelConversionDispatcher', () => { writer.setSelection( new ModelRange( new ModelPosition( root, [ 1 ] ), new ModelPosition( root, [ 1 ] ) ) ); + writer.setMarker( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); } ); - model.markers.set( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); - sinon.spy( dispatcher, 'fire' ); const markers = Array.from( model.markers.getMarkersAtPosition( doc.selection.getFirstPosition() ) ); @@ -346,7 +345,9 @@ describe( 'ModelConversionDispatcher', () => { } ); it( 'should not fire events for markers for non-collapsed selection', () => { - model.markers.set( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); + model.change( writer => { + writer.setMarker( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); + } ); sinon.spy( dispatcher, 'fire' ); @@ -384,8 +385,8 @@ describe( 'ModelConversionDispatcher', () => { } }; - model.markers.set( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 1 ) ); model.change( writer => { + writer.setMarker( 'name', ModelRange.createFromParentsAndOffsets( root, 0, root, 1 ) ); writer.setSelection( ModelRange.createFromParentsAndOffsets( caption, 1, caption, 1 ) ); } ); sinon.spy( dispatcher, 'fire' ); @@ -402,11 +403,10 @@ describe( 'ModelConversionDispatcher', () => { writer.setSelection( new ModelRange( new ModelPosition( root, [ 1 ] ), new ModelPosition( root, [ 1 ] ) ) ); + writer.setMarker( 'foo', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); + writer.setMarker( 'bar', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); } ); - model.markers.set( 'foo', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); - model.markers.set( 'bar', ModelRange.createFromParentsAndOffsets( root, 0, root, 2 ) ); - sinon.spy( dispatcher, 'fire' ); dispatcher.on( 'addMarker:foo', ( evt, data, consumable ) => { diff --git a/tests/manual/highlight.js b/tests/manual/highlight.js index 5db788253..886e8bd30 100644 --- a/tests/manual/highlight.js +++ b/tests/manual/highlight.js @@ -103,9 +103,9 @@ ClassicEditor.create( global.document.querySelector( '#editor' ), { document.getElementById( 'remove-markers' ).addEventListener( 'mousedown', evt => { const markers = editor.model.markers; - editor.model.change( () => { + editor.model.change( writer => { for ( const marker of markers ) { - markers.remove( marker ); + writer.removeMarker( marker ); } } ); @@ -117,14 +117,14 @@ ClassicEditor.create( global.document.querySelector( '#editor' ), { } ); function addMarker( editor, color ) { - editor.model.change( () => { + editor.model.change( writer => { const range = ModelRange.createFromRange( editor.model.document.selection.getFirstRange() ); - editor.model.markers.set( 'marker:' + color, range ); + writer.setMarker( 'marker:' + color, range ); } ); } function removeMarker( editor, color ) { - editor.model.change( () => { - editor.model.markers.remove( 'marker:' + color ); + editor.model.change( writer => { + writer.removeMarker( 'marker:' + color ); } ); } diff --git a/tests/manual/markers.js b/tests/manual/markers.js index 5ba17068c..3a33fe786 100644 --- a/tests/manual/markers.js +++ b/tests/manual/markers.js @@ -73,13 +73,13 @@ ClassicEditor moveSelectionByOffset( 1 ); } ); - model.change( () => { + model.change( writer => { const root = model.document.getRoot(); const range = new Range( new Position( root, [ 0, 10 ] ), new Position( root, [ 0, 16 ] ) ); const name = 'highlight:yellow:' + uid(); markerNames.push( name ); - model.markers.set( name, range ); + writer.setMarker( name, range ); } ); } ) .catch( err => { @@ -91,17 +91,17 @@ function uid() { } function addHighlight( color ) { - model.change( () => { + model.change( writer => { const range = Range.createFromRange( model.document.selection.getFirstRange() ); const name = 'highlight:' + color + ':' + uid(); markerNames.push( name ); - model.markers.set( name, range ); + writer.setMarker( name, range ); } ); } function removeHighlight() { - model.change( () => { + model.change( writer => { const pos = model.document.selection.getFirstPosition(); for ( let i = 0; i < markerNames.length; i++ ) { @@ -110,7 +110,7 @@ function removeHighlight() { const range = marker.getRange(); if ( range.containsPosition( pos ) || range.start.isEqual( pos ) || range.end.isEqual( pos ) ) { - model.markers.remove( name ); + writer.removeMarker( name ); markerNames.splice( i, 1 ); break; diff --git a/tests/model/document.js b/tests/model/document.js index 31a90b822..ecdc4d430 100644 --- a/tests/model/document.js +++ b/tests/model/document.js @@ -237,8 +237,8 @@ describe( 'Document', () => { it( 'should buffer marker changes in differ', () => { sinon.spy( doc.differ, 'bufferMarkerChange' ); - model.change( () => { - model.markers.set( 'marker', Range.createCollapsedAt( doc.getRoot(), 0 ) ); + model.change( writer => { + writer.setMarker( 'marker', Range.createCollapsedAt( doc.getRoot(), 0 ) ); } ); expect( doc.differ.bufferMarkerChange.called ).to.be.true; diff --git a/tests/model/markercollection.js b/tests/model/markercollection.js index 802776ced..138a39083 100644 --- a/tests/model/markercollection.js +++ b/tests/model/markercollection.js @@ -26,8 +26,8 @@ describe( 'MarkerCollection', () => { describe( 'iterator', () => { it( 'should return markers added to the marker collection', () => { - markers.set( 'a', range ); - markers.set( 'b', range ); + markers._set( 'a', range ); + markers._set( 'b', range ); const markerA = markers.get( 'a' ); const markerB = markers.get( 'b' ); @@ -40,28 +40,28 @@ describe( 'MarkerCollection', () => { } ); } ); - describe( 'set', () => { - it( 'should create a marker, fire add: event and return true', () => { + describe( '_set', () => { + it( 'should create a marker, fire set: event and return true', () => { sinon.spy( markers, 'fire' ); - const result = markers.set( 'name', range ); + const result = markers._set( 'name', range ); const marker = markers.get( 'name' ); expect( result ).to.equal( marker ); expect( marker.name ).to.equal( 'name' ); expect( marker.getRange().isEqual( range ) ).to.be.true; - expect( markers.fire.calledWithExactly( 'add:name', marker ) ).to.be.true; + expect( markers.fire.calledWithExactly( 'set:name', marker ) ).to.be.true; } ); it( 'should fire remove: event, and create a new marker if marker with given name was in the collection', () => { - const marker1 = markers.set( 'name', range ); + const marker1 = markers._set( 'name', range ); sinon.spy( markers, 'fire' ); - const marker2 = markers.set( 'name', range2 ); + const marker2 = markers._set( 'name', range2 ); expect( markers.fire.calledWithExactly( 'remove:name', marker1 ) ).to.be.true; - expect( markers.fire.calledWithExactly( 'add:name', marker2 ) ).to.be.true; + expect( markers.fire.calledWithExactly( 'set:name', marker2 ) ).to.be.true; expect( marker2.name ).to.equal( 'name' ); expect( marker2.getRange().isEqual( range2 ) ).to.be.true; @@ -70,21 +70,21 @@ describe( 'MarkerCollection', () => { } ); it( 'should not fire event and return the same marker if given marker has a range equal to given range', () => { - const marker1 = markers.set( 'name', range ); + const marker1 = markers._set( 'name', range ); sinon.spy( markers, 'fire' ); - const marker2 = markers.set( 'name', range ); + const marker2 = markers._set( 'name', range ); expect( marker1 ).to.equal( marker2 ); expect( markers.fire.notCalled ).to.be.true; } ); it( 'should accept marker instance instead of name', () => { - markers.set( 'name', range ); + markers._set( 'name', range ); const marker1 = markers.get( 'name' ); - const result = markers.set( marker1, range2 ); + const result = markers._set( marker1, range2 ); const marker2 = markers.get( 'name' ); expect( result ).to.equal( marker2 ); @@ -99,7 +99,7 @@ describe( 'MarkerCollection', () => { } ); it( 'should return true if marker with given name is in the collection', () => { - markers.set( 'name', range ); + markers._set( 'name', range ); expect( markers.has( 'name' ) ).to.be.true; } ); } ); @@ -114,13 +114,13 @@ describe( 'MarkerCollection', () => { } ); } ); - describe( 'remove', () => { + describe( '_remove', () => { it( 'should remove marker, return true and fire remove: event', () => { - const marker = markers.set( 'name', range ); + const marker = markers._set( 'name', range ); sinon.spy( markers, 'fire' ); - const result = markers.remove( 'name' ); + const result = markers._remove( 'name' ); expect( result ).to.be.true; expect( markers.fire.calledWithExactly( 'remove:name', marker ) ).to.be.true; @@ -128,13 +128,13 @@ describe( 'MarkerCollection', () => { } ); it( 'should destroy marker instance', () => { - const marker = markers.set( 'name', range ); + const marker = markers._set( 'name', range ); const liveRange = marker._liveRange; sinon.spy( marker, 'stopListening' ); sinon.spy( liveRange, 'detach' ); - markers.remove( 'name' ); + markers._remove( 'name' ); expect( marker.stopListening.calledOnce ).to.be.true; expect( marker._liveRange ).to.be.null; @@ -142,22 +142,22 @@ describe( 'MarkerCollection', () => { } ); it( 'should return false if name has not been found in collection', () => { - markers.set( 'name', range ); + markers._set( 'name', range ); sinon.spy( markers, 'fire' ); - const result = markers.remove( 'other' ); + const result = markers._remove( 'other' ); expect( result ).to.be.false; expect( markers.fire.notCalled ).to.be.true; } ); it( 'should accept marker instance instead of name', () => { - const marker = markers.set( 'name', range ); + const marker = markers._set( 'name', range ); sinon.spy( markers, 'fire' ); - const result = markers.remove( marker ); + const result = markers._remove( marker ); expect( result ).to.be.true; expect( markers.fire.calledWithExactly( 'remove:name', marker ) ).to.be.true; @@ -167,10 +167,10 @@ describe( 'MarkerCollection', () => { describe( 'getMarkersGroup', () => { it( 'returns all markers which names start on given prefix', () => { - const markerFooA = markers.set( 'foo:a', range ); - const markerFooB = markers.set( 'foo:b', range ); - markers.set( 'bar:a', range ); - markers.set( 'foobar:a', range ); + const markerFooA = markers._set( 'foo:a', range ); + const markerFooB = markers._set( 'foo:b', range ); + markers._set( 'bar:a', range ); + markers._set( 'foobar:a', range ); expect( Array.from( markers.getMarkersGroup( 'foo' ) ) ).to.deep.equal( [ markerFooA, markerFooB ] ); expect( Array.from( markers.getMarkersGroup( 'a' ) ) ).to.deep.equal( [] ); @@ -179,8 +179,8 @@ describe( 'MarkerCollection', () => { describe( 'getMarkersAtPosition', () => { it( 'should return iterator iterating over all markers that contains given position', () => { - markers.set( 'a', range ); - const markerB = markers.set( 'b', range2 ); + markers._set( 'a', range ); + const markerB = markers._set( 'b', range2 ); const result = Array.from( markers.getMarkersAtPosition( Position.createAt( root, 1 ) ) ); @@ -190,8 +190,8 @@ describe( 'MarkerCollection', () => { describe( 'destroy', () => { it( 'should make MarkerCollection stop listening to all events and destroy all markers', () => { - const markerA = markers.set( 'a', range ); - const markerB = markers.set( 'b', range2 ); + const markerA = markers._set( 'a', range ); + const markerB = markers._set( 'b', range2 ); sinon.spy( markers, 'stopListening' ); sinon.spy( markerA, 'stopListening' ); @@ -221,7 +221,7 @@ describe( 'Marker', () => { root.appendChildren( new Text( 'foo' ) ); const range = Range.createFromParentsAndOffsets( root, 1, root, 2 ); - const marker = model.markers.set( 'name', range ); + const marker = model.markers._set( 'name', range ); expect( marker.getRange().isEqual( range ) ).to.be.true; expect( marker.getStart().isEqual( range.start ) ).to.be.true; @@ -240,9 +240,9 @@ describe( 'Marker', () => { it( 'should throw when using the API if marker was removed from markers collection', () => { const range = Range.createFromParentsAndOffsets( root, 1, root, 2 ); - const marker = model.markers.set( 'name', range ); + const marker = model.markers._set( 'name', range ); - model.markers.remove( 'name' ); + model.markers._remove( 'name' ); expect( () => { marker.getRange(); @@ -259,7 +259,7 @@ describe( 'Marker', () => { it( 'should delegate events from live range', () => { const range = Range.createFromParentsAndOffsets( root, 1, root, 2 ); - const marker = model.markers.set( 'name', range ); + const marker = model.markers._set( 'name', range ); const eventRange = sinon.spy(); const eventContent = sinon.spy(); diff --git a/tests/model/operation/markeroperation.js b/tests/model/operation/markeroperation.js index 88349d691..b14b55b69 100644 --- a/tests/model/operation/markeroperation.js +++ b/tests/model/operation/markeroperation.js @@ -30,14 +30,14 @@ describe( 'MarkerOperation', () => { } ); it( 'should add marker to document marker collection', () => { - sinon.spy( model.markers, 'set' ); + sinon.spy( model.markers, '_set' ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', null, range, model.markers, doc.version ) ) ); expect( doc.version ).to.equal( 1 ); - expect( model.markers.set.calledWith( 'name', matchRange( range ) ) ); + expect( model.markers._set.calledWith( 'name', matchRange( range ) ) ); expect( model.markers.get( 'name' ).getRange().isEqual( range ) ).to.be.true; } ); @@ -48,14 +48,14 @@ describe( 'MarkerOperation', () => { const range2 = Range.createFromParentsAndOffsets( root, 0, root, 3 ); - sinon.spy( model.markers, 'set' ); + sinon.spy( model.markers, '_set' ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, range2, model.markers, doc.version ) ) ); expect( doc.version ).to.equal( 2 ); - expect( model.markers.set.calledWith( 'name', matchRange( range2 ) ) ); + expect( model.markers._set.calledWith( 'name', matchRange( range2 ) ) ); expect( model.markers.get( 'name' ).getRange().isEqual( range2 ) ).to.be.true; } ); @@ -64,14 +64,14 @@ describe( 'MarkerOperation', () => { new MarkerOperation( 'name', null, range, model.markers, doc.version ) ) ); - sinon.spy( model.markers, 'remove' ); + sinon.spy( model.markers, '_remove' ); model.applyOperation( wrapInDelta( new MarkerOperation( 'name', range, null, model.markers, doc.version ) ) ); expect( doc.version ).to.equal( 2 ); - expect( model.markers.remove.calledWith( 'name' ) ); + expect( model.markers._remove.calledWith( 'name' ) ); expect( model.markers.get( 'name' ) ).to.be.null; } ); @@ -86,7 +86,9 @@ describe( 'MarkerOperation', () => { } ); it( 'should not fire document markers set event if newRange is same as current marker range', () => { - model.markers.set( 'name', range ); + model.change( writer => { + writer.setMarker( 'name', range, { usingOperation: true } ); + } ); sinon.spy( model.markers, 'fire' ); diff --git a/tests/model/writer.js b/tests/model/writer.js index 1b9650373..a411da79b 100644 --- a/tests/model/writer.js +++ b/tests/model/writer.js @@ -1970,6 +1970,30 @@ describe( 'Writer', () => { expect( model.markers.get( 'name' ).getRange().isEqual( range ) ).to.be.true; } ); + it( 'should return marker if that was set directly', () => { + const marker = setMarker( 'name', range ); + + expect( model.markers.get( 'name' ) ).to.equal( marker ); + } ); + + it( 'should return marker if that was set using operation API', () => { + const marker = setMarker( 'name', range, { usingOperation: true } ); + + expect( model.markers.get( 'name' ) ).to.equal( marker ); + } ); + + it( 'should return marker with properly set managedUsingOperations (to true)', () => { + const marker = setMarker( 'name', range, { usingOperation: true } ); + + expect( marker.managedUsingOperations ).to.equal( true ); + } ); + + it( 'should return marker with properly set managedUsingOperations (to false)', () => { + const marker = setMarker( 'name', range ); + + expect( marker.managedUsingOperations ).to.equal( false ); + } ); + it( 'should update marker in the document marker collection', () => { setMarker( 'name', range ); @@ -1980,24 +2004,58 @@ describe( 'Writer', () => { } ); it( 'should accept marker instance', () => { - const marker = model.markers.set( 'name', range ); + const marker = setMarker( 'name', range, { usingOperation: true } ); const range2 = Range.createFromParentsAndOffsets( root, 0, root, 0 ); - setMarker( marker, range2 ); - const op = batch.deltas[ 0 ].operations[ 0 ]; + setMarker( marker, range2, { usingOperation: true } ); + + 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', () => { - const marker = model.markers.set( 'name', range ); + it( 'should accept empty range parameter if marker instance is passed and usingOperation is set to true', () => { + const marker = setMarker( 'name', range, { usingOperation: true } ); const spy = sinon.spy(); model.on( 'applyOperation', spy ); - setMarker( marker ); + setMarker( marker, { usingOperation: true } ); + + const op = batch.deltas[ 0 ].operations[ 0 ]; + + expect( spy.calledOnce ).to.be.true; + expect( spy.firstCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); + expect( op.oldRange ).to.be.null; + expect( op.newRange.isEqual( range ) ).to.be.true; + } ); + + it( 'should create a unique id if the first param is type of range', () => { + const marker = setMarker( range ); + + expect( marker.name ).to.be.a( 'string' ); + } ); + + it( 'should create a unique id if the first param is type of range when usingOperation is set to true', () => { + const spy = sinon.spy(); + model.on( 'applyOperation', spy ); + + const marker = setMarker( range, { usingOperation: true } ); + + expect( marker.name ).to.be.a( 'string' ); + expect( spy.calledOnce ).to.be.true; + } ); + + it( 'should use operations when having set usingOperation to true', () => { + const spy = sinon.spy(); + + model.on( 'applyOperation', spy ); + + setMarker( 'name', range, { usingOperation: true } ); const op = batch.deltas[ 0 ].operations[ 0 ]; expect( spy.calledOnce ).to.be.true; @@ -2007,17 +2065,58 @@ describe( 'Writer', () => { } ); it( 'should throw if marker with given name does not exist and range is not passed', () => { + expect( () => { + setMarker( 'name', { usingOperation: true } ); + } ).to.throw( CKEditorError, /^writer-setMarker-no-range/ ); + } ); + + it( 'should throw if marker is set directly and range is not passed', () => { expect( () => { setMarker( 'name' ); } ).to.throw( CKEditorError, /^writer-setMarker-no-range/ ); } ); + it( 'should create additional operation when marker type changes to not managed using operation', () => { + const spy = sinon.spy(); + model.on( 'applyOperation', spy ); + + setMarker( 'name', range, { usingOperation: true } ); + const marker = setMarker( 'name', range ); + const op1 = batch.deltas[ 0 ].operations[ 0 ]; + const op2 = batch.deltas[ 1 ].operations[ 0 ]; + + expect( spy.calledTwice ).to.be.true; + expect( spy.firstCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); + expect( spy.secondCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); + + expect( op1.oldRange ).to.be.null; + expect( op1.newRange.isEqual( range ) ).to.be.true; + + expect( op2.oldRange.isEqual( range ) ).to.be.true; + expect( op2.newRange ).to.be.null; + + expect( marker.managedUsingOperations ).to.be.false; + } ); + + it( 'should enable changing marker to be not managed using operation', () => { + const spy = sinon.spy(); + model.on( 'applyOperation', spy ); + + setMarker( 'name', range ); + const marker = setMarker( 'name', range, { usingOperation: true } ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.firstCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); + + expect( marker.managedUsingOperations ).to.be.true; + } ); + it( 'should throw when trying to use detached writer', () => { - const marker = model.markers.set( 'name', range ); + const marker = setMarker( 'name', range ); const writer = new Writer( model, batch ); expect( () => { - writer.setMarker( marker ); + writer.setMarker( marker, null, { usingOperation: true } ); } ).to.throw( CKEditorError, /^writer-incorrect-use/ ); } ); } ); @@ -2060,6 +2159,21 @@ describe( 'Writer', () => { expect( model.markers.get( 'name' ) ).to.be.null; } ); + + it( 'should use MarkerOperation when marker was created using operation', () => { + setMarker( 'name', range, { usingOperation: true } ); + + const marker = model.markers.get( 'name' ); + const spy = sinon.spy(); + + model.on( 'applyOperation', spy ); + + removeMarker( marker ); + + expect( spy.calledOnce ).to.be.true; + expect( spy.firstCall.args[ 1 ][ 0 ].type ).to.equal( 'marker' ); + expect( model.markers.get( 'name' ) ).to.be.null; + } ); } ); describe( 'setSelection()', () => { @@ -2353,15 +2467,19 @@ describe( 'Writer', () => { } ); } - function setMarker( markerOrName, newRange ) { + function setMarker( markerOrNameOrRange, rangeOrManagedUsingOperations, options ) { + let marker; + model.enqueueChange( batch, writer => { - writer.setMarker( markerOrName, newRange ); + marker = writer.setMarker( markerOrNameOrRange, rangeOrManagedUsingOperations, options ); } ); + + return marker; } - function removeMarker( markerOrName ) { + function removeMarker( markerOrName, options ) { model.enqueueChange( batch, writer => { - writer.removeMarker( markerOrName ); + writer.removeMarker( markerOrName, options ); } ); }