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 #1223 from ckeditor/t/1212
Browse files Browse the repository at this point in the history
Other: Prevented `Writer` from usage outside of the `change` block. Closes #1212.
  • Loading branch information
Piotr Jasiun authored Jan 5, 2018
2 parents 9664b5d + 949e9df commit 2592bf1
Show file tree
Hide file tree
Showing 2 changed files with 760 additions and 384 deletions.
43 changes: 43 additions & 0 deletions src/model/writer.js
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ export default class Writer {
* second parameter is a {@link module:engine/model/item~Item model item}.
*/
insert( item, itemOrPosition, offset ) {
this._assertWriterUsageCorrectness();

const position = Position.createAt( itemOrPosition, offset );

// For text that has no parent we need to make a WeakInsert.
Expand Down Expand Up @@ -315,6 +317,8 @@ export default class Writer {
* Model item or range on which the attribute will be set.
*/
setAttribute( key, value, itemOrRange ) {
this._assertWriterUsageCorrectness();

if ( itemOrRange instanceof Range ) {
setAttributeToRange( this, key, value, itemOrRange );
} else {
Expand Down Expand Up @@ -350,6 +354,8 @@ export default class Writer {
* Model item or range from which the attribute will be removed.
*/
removeAttribute( key, itemOrRange ) {
this._assertWriterUsageCorrectness();

if ( itemOrRange instanceof Range ) {
setAttributeToRange( this, key, null, itemOrRange );
} else {
Expand All @@ -364,6 +370,8 @@ export default class Writer {
* Model item or range from which all attributes will be removed.
*/
clearAttributes( itemOrRange ) {
this._assertWriterUsageCorrectness();

const removeAttributesFromItem = item => {
for ( const attribute of item.getAttributeKeys() ) {
this.removeAttribute( attribute, item );
Expand Down Expand Up @@ -404,6 +412,8 @@ export default class Writer {
* second parameter is a {@link module:engine/model/item~Item model item}.
*/
move( range, itemOrPosition, offset ) {
this._assertWriterUsageCorrectness();

if ( !( range instanceof Range ) ) {
/**
* Invalid range to move.
Expand Down Expand Up @@ -448,6 +458,8 @@ export default class Writer {
* @param {module:engine/model/item~Item|module:engine/model/range~Range} itemOrRange Model item or range to remove.
*/
remove( itemOrRange ) {
this._assertWriterUsageCorrectness();

const addRemoveDelta = ( position, howMany ) => {
const delta = new RemoveDelta();
this.batch.addDelta( delta );
Expand Down Expand Up @@ -489,6 +501,8 @@ export default class Writer {
* @param {module:engine/model/position~Position} position Position of merge.
*/
merge( position ) {
this._assertWriterUsageCorrectness();

const delta = new MergeDelta();
this.batch.addDelta( delta );

Expand Down Expand Up @@ -542,6 +556,8 @@ export default class Writer {
* @param {String} newName New element name.
*/
rename( element, newName ) {
this._assertWriterUsageCorrectness();

if ( !( element instanceof Element ) ) {
/**
* Trying to rename an object which is not an instance of Element.
Expand Down Expand Up @@ -570,6 +586,8 @@ export default class Writer {
* @param {module:engine/model/position~Position} position Position of split.
*/
split( position ) {
this._assertWriterUsageCorrectness();

const delta = new SplitDelta();
this.batch.addDelta( delta );

Expand Down Expand Up @@ -616,6 +634,8 @@ export default class Writer {
* @param {module:engine/model/element~Element|String} elementOrString Element or name of element to wrap the range with.
*/
wrap( range, elementOrString ) {
this._assertWriterUsageCorrectness();

if ( !range.isFlat ) {
/**
* Range to wrap is not flat.
Expand Down Expand Up @@ -670,6 +690,8 @@ export default class Writer {
* @param {module:engine/model/element~Element} element Element to unwrap.
*/
unwrap( element ) {
this._assertWriterUsageCorrectness();

if ( element.parent === null ) {
/**
* Trying to unwrap an element which has no parent.
Expand Down Expand Up @@ -722,6 +744,8 @@ export default class Writer {
* @param {module:engine/model/range~Range} [newRange] Marker range.
*/
setMarker( markerOrName, newRange ) {
this._assertWriterUsageCorrectness();

const name = typeof markerOrName == 'string' ? markerOrName : markerOrName.name;
const currentMarker = this.model.markers.get( name );

Expand Down Expand Up @@ -752,6 +776,8 @@ export default class Writer {
* @param {module:engine/model/markercollection~Marker|String} markerOrName Marker or marker name to remove.
*/
removeMarker( markerOrName ) {
this._assertWriterUsageCorrectness();

const name = typeof markerOrName == 'string' ? markerOrName : markerOrName.name;

if ( !this.model.markers.has( name ) ) {
Expand All @@ -767,6 +793,23 @@ export default class Writer {

addMarkerOperation( this, name, oldRange, null );
}

/**
* Throws `writer-detached-writer-tries-to-modify-model` error when the writer is used outside of the `change()` block.
*
* @private
*/
_assertWriterUsageCorrectness() {
/**
* Detached writer tries to modify the model. Be sure, that your Writer is used
* within the `model.change()` or `model.enqueueChange()` block.
*
* @error writer-detached-writer-tries-to-modify-model
*/
if ( this.model._currentWriter !== this ) {
throw new CKEditorError( 'writer-detached-writer-tries-to-modify-model: Detached writer tries to modify the model.' );
}
}
}

// Sets given attribute to each node in given range. When attribute value is null then attribute will be removed.
Expand Down
Loading

0 comments on commit 2592bf1

Please # to comment.