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

Prevent Writer from usage outside of the change block #1223

Merged
merged 10 commits into from
Jan 5, 2018
Merged

Conversation

ma2ciek
Copy link
Contributor

@ma2ciek ma2ciek commented Jan 4, 2018

Suggested merge commit message (convention)

Other: Prevented Writer from usage outside of the change block. Closes ckeditor/ckeditor5#4224.

@ma2ciek ma2ciek requested a review from pjasiun January 4, 2018 12:17
@ma2ciek ma2ciek changed the title T/1212 Prevent Writer from usage outside of the change block Jan 4, 2018
@ma2ciek
Copy link
Contributor Author

ma2ciek commented Jan 4, 2018

Few tests for errors miss. I'm on it.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 99.809% when pulling 65d4f08 on t/1212 into 4e4f5c3 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 0c39191 on t/1212 into 4e4f5c3 on master.

@@ -150,6 +150,10 @@ export default class Writer {
* second parameter is a {@link module:engine/model/item~Item model item}.
*/
insert( item, itemOrPosition, offset ) {
if ( this.model._currentWriter !== this ) {
throw new CKEditorError( 'writer-detached-writer-tries-to-modify-model: Detached writer tries to modify the model' );
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to have it in a private method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably yes. I moved it to the separate private method.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling a81391e on t/1212 into 4e4f5c3 on master.

@ma2ciek
Copy link
Contributor Author

ma2ciek commented Jan 5, 2018

Ready for the review.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 949e9df on t/1212 into 4e4f5c3 on master.

@pjasiun pjasiun merged commit 2592bf1 into master Jan 5, 2018
@pjasiun pjasiun deleted the t/1212 branch January 5, 2018 11:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants