diff --git a/src/changebuffer.js b/src/changebuffer.js index f7ad9d5..ffff490 100644 --- a/src/changebuffer.js +++ b/src/changebuffer.js @@ -60,12 +60,28 @@ export default class ChangeBuffer { */ this.limit = limit; + /** + * Whether the buffer is locked. The locked buffer cannot be reset unless it gets unlocked. + * + * @readonly + * @member {Boolean} #isLocked + */ + this.isLocked = false; + this._changeCallback = ( evt, type, changes, batch ) => { this._onBatch( batch ); }; + this._selectionChangeCallback = () => { + this._reset(); + }; + doc.on( 'change', this._changeCallback ); + doc.selection.on( 'change:range', this._selectionChangeCallback ); + + doc.selection.on( 'change:attribute', this._selectionChangeCallback ); + /** * The current batch instance. * @@ -79,13 +95,20 @@ export default class ChangeBuffer { * @private * @member #_changeCallback */ + + /** + * The callback to document selection change:attribute and change:range events which resets the buffer. + * + * @private + * @member #_selectionChangeCallback + */ } /** * The current batch to which a feature should add its deltas. Once the {@link #size} * is reached or exceeds the {@link #limit}, the batch is set to a new instance and the size is reset. * - * @type {engine.treeModel.batch.Batch} + * @type {module:engine/model/batch~Batch} */ get batch() { if ( !this._batch ) { @@ -105,15 +128,31 @@ export default class ChangeBuffer { this.size += changeCount; if ( this.size >= this.limit ) { - this._reset(); + this._reset( true ); } } + /** + * Locks the buffer. + */ + lock() { + this.isLocked = true; + } + + /** + * Unlocks the buffer. + */ + unlock() { + this.isLocked = false; + } + /** * Destroys the buffer. */ destroy() { this.document.off( 'change', this._changeCallback ); + this.document.selection.off( 'change:range', this._selectionChangeCallback ); + this.document.selection.off( 'change:attribute', this._selectionChangeCallback ); } /** @@ -130,7 +169,7 @@ export default class ChangeBuffer { _onBatch( batch ) { // One operation means a newly created batch. if ( batch.type != 'transparent' && batch !== this._batch && count( batch.getOperations() ) <= 1 ) { - this._reset(); + this._reset( true ); } } @@ -138,9 +177,12 @@ export default class ChangeBuffer { * Resets the change buffer. * * @private + * @param {Boolean} [ignoreLock] Whether internal lock {@link #isLocked} should be ignored. */ - _reset() { - this._batch = null; - this.size = 0; + _reset( ignoreLock ) { + if ( !this.isLocked || ignoreLock ) { + this._batch = null; + this.size = 0; + } } } diff --git a/src/deletecommand.js b/src/deletecommand.js index 171b29b..79e00d1 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -61,6 +61,8 @@ export default class DeleteCommand extends Command { const dataController = this.editor.data; doc.enqueueChanges( () => { + this._buffer.lock(); + const selection = Selection.createFromSelection( doc.selection ); // Try to extend the selection in the specified direction. @@ -85,6 +87,8 @@ export default class DeleteCommand extends Command { this._buffer.input( changeCount ); doc.selection.setRanges( selection.getRanges(), selection.isBackward ); + + this._buffer.unlock(); } ); } } diff --git a/src/input.js b/src/input.js index 2899b4f..c1977c4 100644 --- a/src/input.js +++ b/src/input.js @@ -66,9 +66,13 @@ export default class Input extends Plugin { return; } + buffer.lock(); + doc.enqueueChanges( () => { this.editor.data.deleteContent( doc.selection, buffer.batch ); } ); + + buffer.unlock(); } /** diff --git a/src/inputcommand.js b/src/inputcommand.js index a4b6815..5ad4c7d 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -78,6 +78,8 @@ export default class InputCommand extends Command { doc.enqueueChanges( () => { const isCollapsedRange = range.isCollapsed; + this._buffer.lock(); + if ( !isCollapsedRange ) { this._buffer.batch.remove( range ); } @@ -91,6 +93,8 @@ export default class InputCommand extends Command { this.editor.data.model.selection.collapse( range.start.getShiftedBy( textInsertions ) ); } + this._buffer.unlock(); + this._buffer.input( textInsertions ); } ); } diff --git a/tests/changebuffer.js b/tests/changebuffer.js index d529851..2ee5a7a 100644 --- a/tests/changebuffer.js +++ b/tests/changebuffer.js @@ -23,6 +23,7 @@ describe( 'ChangeBuffer', () => { expect( buffer ).to.have.property( 'document', doc ); expect( buffer ).to.have.property( 'limit', CHANGE_LIMIT ); expect( buffer ).to.have.property( 'size', 0 ); + expect( buffer ).to.have.property( 'isLocked', false ); } ); it( 'sets limit property according to default value', () => { @@ -32,6 +33,26 @@ describe( 'ChangeBuffer', () => { } ); } ); + describe( 'locking', () => { + it( 'is unlocked by default', () => { + expect( buffer.isLocked ).to.be.false; + } ); + + it( 'is locked by lock method', () => { + buffer.lock(); + + expect( buffer.isLocked ).to.be.true; + } ); + + it( 'is unlocked by unlock method', () => { + buffer.isLocked = true; + + buffer.unlock(); + + expect( buffer.isLocked ).to.be.false; + } ); + } ); + describe( 'batch', () => { it( 'it is set initially', () => { expect( buffer ).to.have.property( 'batch' ); @@ -107,6 +128,106 @@ describe( 'ChangeBuffer', () => { expect( buffer.batch ).to.equal( bufferBatch ); } ); + + it( 'is not reset while locked', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( 1 ); + buffer._reset(); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); + + it( 'is reset while locked with ignoreLock used', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( 1 ); + buffer._reset( true ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset while locked and limit exceeded', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + buffer.input( CHANGE_LIMIT + 1 ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset while locked and new batch is applied', () => { + const initialBatch = buffer.batch; + + buffer.lock(); + + doc.batch().insert( Position.createAt( root, 0 ), 'a' ); + + buffer.unlock(); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset on selection change:range', () => { + const initialBatch = buffer.batch; + + doc.selection.fire( 'change:range' ); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is reset on selection change:attribute', () => { + const initialBatch = buffer.batch; + + doc.selection.fire( 'change:attribute' ); + + expect( buffer.batch ).to.not.equal( initialBatch ); + expect( buffer.size ).to.equal( 0 ); + } ); + + it( 'is not reset on selection change:range while locked', () => { + const initialBatch = buffer.batch; + buffer.size = 1; + + buffer.lock(); + + doc.selection.fire( 'change:range' ); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); + + it( 'is not reset on selection change:attribute while locked', () => { + const initialBatch = buffer.batch; + buffer.size = 1; + + buffer.lock(); + + doc.selection.fire( 'change:attribute' ); + + buffer.unlock(); + + expect( buffer.batch ).to.be.equal( initialBatch ); + expect( buffer.size ).to.equal( 1 ); + } ); } ); describe( 'destroy', () => { @@ -119,5 +240,25 @@ describe( 'ChangeBuffer', () => { expect( buffer.batch ).to.equal( batch1 ); } ); + + it( 'offs the buffer from the selection change:range', () => { + const batch1 = buffer.batch; + + buffer.destroy(); + + doc.selection.fire( 'change:attribute' ); + + expect( buffer.batch ).to.equal( batch1 ); + } ); + + it( 'offs the buffer from the selection change:attribute', () => { + const batch1 = buffer.batch; + + buffer.destroy(); + + doc.selection.fire( 'change:range' ); + + expect( buffer.batch ).to.equal( batch1 ); + } ); } ); } ); diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 8b2f879..4ca5c21 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -6,10 +6,13 @@ import ModelTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/modeltesteditor'; import DeleteCommand from '../src/deletecommand'; import { getData, setData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model'; +import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils'; describe( 'DeleteCommand', () => { let editor, doc; + testUtils.createSinonSandbox(); + beforeEach( () => { return ModelTestEditor.create( ) .then( newEditor => { @@ -33,13 +36,26 @@ describe( 'DeleteCommand', () => { it( 'uses enqueueChanges', () => { setData( doc, '
foo[]bar
' ); - const spy = sinon.spy( doc, 'enqueueChanges' ); + const spy = testUtils.sinon.spy( doc, 'enqueueChanges' ); editor.execute( 'delete' ); expect( spy.calledOnce ).to.be.true; } ); + it( 'locks buffer when executing', () => { + setData( doc, 'foo[]bar
' ); + + const buffer = editor.commands.get( 'delete' )._buffer; + const lockSpy = testUtils.sinon.spy( buffer, 'lock' ); + const unlockSpy = testUtils.sinon.spy( buffer, 'unlock' ); + + editor.execute( 'delete' ); + + expect( lockSpy.calledOnce ).to.be.true; + expect( unlockSpy.calledOnce ).to.be.true; + } ); + it( 'deletes previous character when selection is collapsed', () => { setData( doc, 'foo[]bar
' ); diff --git a/tests/input.js b/tests/input.js index 0f294a5..6b732a0 100644 --- a/tests/input.js +++ b/tests/input.js @@ -434,6 +434,48 @@ describe( 'Input feature', () => { expect( getModelData( model ) ).to.equal( '0123456789{}
' ); + + editor.execute( 'undo' ); + + expectOutput( '0123456{}
' ); + + editor.execute( 'undo' ); + + expectOutput( '0123{}
' ); + + editor.execute( 'redo' ); + + expectOutput( '0123456{}
' ); + } ); + + it( 'resets the buffer on text insertion respecting typing.undoStep', () => { + setModelData( doc, '0123456789{}
' ); + + editor.execute( 'undo' ); + + expectOutput( '012345678{}
' ); + + editor.execute( 'undo' ); + + expectOutput( '01234{}
' ); + + editor.execute( 'redo' ); + + expectOutput( '012345678{}
' ); + } ); + + it( 'resets the buffer when selection changes', () => { + setModelData( doc, 'Foo B1a2{}
' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo B1a{r}
' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo B{}ar
' ); + + editor.execute( 'redo' ); + + expectOutput( 'Foo B1{}ar
' ); + } ); + + it( 'resets the buffer when selection changes (with enter)', () => { + setModelData( doc, 'Foo1
Bar2
Baz{}
' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1
Bar2
[]
' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1
Bar2{}
' ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo1
Bar{}
' ); + + editor.execute( 'redo' ); + + expectOutput( 'Foo1
Bar2{}
' ); + } ); + + it( 'resets the buffer when attribute changes', () => { + setModelData( doc, 'Foo Baz{} Bar
' ); + + editor.execute( 'undo' ); + + expectOutput( + 'Foo Ba{} Bar
' + ); + + editor.execute( 'undo' ); + + expectOutput( 'Foo B{} Bar
' ); + } ); + } ); +} ); diff --git a/tests/inputcommand.js b/tests/inputcommand.js index 86ba6d6..cc1ec11 100644 --- a/tests/inputcommand.js +++ b/tests/inputcommand.js @@ -71,6 +71,20 @@ describe( 'InputCommand', () => { expect( spy.calledOnce ).to.be.true; } ); + it( 'should lock and unlock buffer', () => { + setData( doc, 'foo[]bar
' ); + + const spyLock = testUtils.sinon.spy( buffer, 'lock' ); + const spyUnlock = testUtils.sinon.spy( buffer, 'unlock' ); + + editor.execute( 'input', { + text: '' + } ); + + expect( spyLock.calledOnce ).to.be.true; + expect( spyUnlock.calledOnce ).to.be.true; + } ); + it( 'inserts text for collapsed range', () => { setData( doc, 'foo[]
' ); diff --git a/tests/manual/20/1.html b/tests/manual/20/1.html new file mode 100644 index 0000000..b2364bb --- /dev/null +++ b/tests/manual/20/1.html @@ -0,0 +1,4 @@ +This is an editor instance.
+This is an editor instance.
+Parag}raph
- ``` - - delete selected text. + * select a fragment of the heading and paragraph, + ```html +Parag}raph
+ ``` +* delete the selected text. Expected result: ```html