From cd455f508e1fb05e79e2d61eeb30b377a033ec84 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Mon, 2 Mar 2020 16:15:33 +0100 Subject: [PATCH 1/4] Passed the direction of the DeleteCommand to the Model#deleteContent() execution. --- src/deletecommand.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/deletecommand.js b/src/deletecommand.js index 944cb48..1db1fea 100644 --- a/src/deletecommand.js +++ b/src/deletecommand.js @@ -110,7 +110,11 @@ export default class DeleteCommand extends Command { ); } ); - model.deleteContent( selection, { doNotResetEntireContent } ); + model.deleteContent( selection, { + doNotResetEntireContent, + direction: this.direction + } ); + this._buffer.input( changeCount ); writer.setSelection( selection ); From b939683c75628dfc994877150a808c8f8d9450ff Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Tue, 3 Mar 2020 11:30:21 +0100 Subject: [PATCH 2/4] Tests: Added a test to check if the DeleteCommand always passes the direction to the Model#deleteContent() method. --- tests/deletecommand.js | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/deletecommand.js b/tests/deletecommand.js index 582c10d..c6866f8 100644 --- a/tests/deletecommand.js +++ b/tests/deletecommand.js @@ -184,6 +184,29 @@ describe( 'DeleteCommand', () => { expect( deleteOpts ).to.have.property( 'doNotResetEntireContent', false ); } ); + it( 'should pass the "direction" option to Model#deleteContent method', () => { + const spy = sinon.spy(); + const forwardCommand = new DeleteCommand( editor, 'forward' ); + editor.commands.add( 'forwardDelete', forwardCommand ); + + editor.model.on( 'deleteContent', spy ); + setData( model, 'foo[]bar' ); + + editor.execute( 'delete' ); + + expect( spy.callCount ).to.equal( 1 ); + + let deleteOpts = spy.args[ 0 ][ 1 ][ 1 ]; + expect( deleteOpts ).to.have.property( 'direction', 'backward' ); + + editor.execute( 'forwardDelete' ); + + expect( spy.callCount ).to.equal( 2 ); + + deleteOpts = spy.args[ 1 ][ 1 ][ 1 ]; + expect( deleteOpts ).to.have.property( 'direction', 'forward' ); + } ); + it( 'leaves an empty paragraph after removing the whole content from editor', () => { setData( model, '[Header 1Some text.]' ); From 0065bb2c6acb9362dc302b1e58c8cbf8716d60d8 Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Wed, 4 Mar 2020 14:20:29 +0100 Subject: [PATCH 3/4] Changed InputCommand#execute so it supports multi-range selections. --- src/inputcommand.js | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/inputcommand.js b/src/inputcommand.js index 6fe85a5..126e046 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -83,25 +83,20 @@ export default class InputCommand extends Command { const doc = model.document; const text = options.text || ''; const textInsertions = text.length; - const range = options.range || doc.selection.getFirstRange(); + const selection = options.range ? model.createSelection( options.range ) : doc.selection; const resultRange = options.resultRange; model.enqueueChange( this._buffer.batch, writer => { - const isCollapsedRange = range.isCollapsed; - this._buffer.lock(); - model.deleteContent( model.createSelection( range ) ); + model.deleteContent( selection ); if ( text ) { - model.insertContent( writer.createText( text, doc.selection.getAttributes() ), range.start ); + model.insertContent( writer.createText( text, doc.selection.getAttributes() ), selection ); } if ( resultRange ) { writer.setSelection( resultRange ); - } else if ( isCollapsedRange ) { - // If range was collapsed just shift the selection by the number of inserted characters. - writer.setSelection( range.start.getShiftedBy( textInsertions ) ); } this._buffer.unlock(); From 53ee2554ad85db81ba9457aa6cdc4592f224b84e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 9 Mar 2020 22:12:53 +0100 Subject: [PATCH 4/4] Added at least a single test and handled the case where operating on standalone selections. --- src/inputcommand.js | 2 + tests/inputcommand.js | 113 +++++++++++++++++++++++++++++++----------- 2 files changed, 85 insertions(+), 30 deletions(-) diff --git a/src/inputcommand.js b/src/inputcommand.js index 126e046..9f47346 100644 --- a/src/inputcommand.js +++ b/src/inputcommand.js @@ -97,6 +97,8 @@ export default class InputCommand extends Command { if ( resultRange ) { writer.setSelection( resultRange ); + } else if ( !selection.is( 'documentSelection' ) ) { + writer.setSelection( selection ); } this._buffer.unlock(); diff --git a/tests/inputcommand.js b/tests/inputcommand.js index 626f751..2931a34 100644 --- a/tests/inputcommand.js +++ b/tests/inputcommand.js @@ -29,8 +29,8 @@ describe( 'InputCommand', () => { buffer = inputCommand.buffer; buffer.size = 0; - model.schema.register( 'p', { inheritAllFrom: '$block' } ); - model.schema.register( 'h1', { inheritAllFrom: '$block' } ); + model.schema.register( 'paragraph', { inheritAllFrom: '$block' } ); + model.schema.register( 'heading1', { inheritAllFrom: '$block' } ); } ); } ); @@ -63,22 +63,22 @@ describe( 'InputCommand', () => { describe( 'execute()', () => { it( 'uses enqueueChange', () => { - setData( model, '

foo[]bar

' ); + setData( model, 'foo[]bar' ); model.enqueueChange( () => { editor.execute( 'input', { text: 'x' } ); // We expect that command is executed in enqueue changes block. Since we are already in // an enqueued block, the command execution will be postponed. Hence, no changes. - expect( getData( model ) ).to.be.equal( '

foo[]bar

' ); + expect( getData( model ) ).to.be.equal( 'foo[]bar' ); } ); // After all enqueued changes are done, the command execution is reflected. - expect( getData( model ) ).to.be.equal( '

foox[]bar

' ); + expect( getData( model ) ).to.be.equal( 'foox[]bar' ); } ); it( 'should lock and unlock buffer', () => { - setData( model, '

foo[]bar

' ); + setData( model, 'foo[]bar' ); const spyLock = testUtils.sinon.spy( buffer, 'lock' ); const spyUnlock = testUtils.sinon.spy( buffer, 'unlock' ); @@ -92,102 +92,102 @@ describe( 'InputCommand', () => { } ); it( 'inserts text for collapsed range', () => { - setData( model, '

foo[]

' ); + setData( model, 'foo[]' ); editor.execute( 'input', { text: 'bar', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

foobar[]

' ); + expect( getData( model ) ).to.be.equal( 'foobar[]' ); expect( buffer.size ).to.be.equal( 3 ); } ); it( 'replaces text for range within single element on the beginning', () => { - setData( model, '

[fooba]r

' ); + setData( model, '[fooba]r' ); editor.execute( 'input', { text: 'rab', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

rab[]r

' ); + expect( getData( model ) ).to.be.equal( 'rab[]r' ); expect( buffer.size ).to.be.equal( 3 ); } ); it( 'replaces text for range within single element in the middle', () => { - setData( model, '

fo[oba]r

' ); + setData( model, 'fo[oba]r' ); editor.execute( 'input', { text: 'bazz', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

fobazz[]r

' ); + expect( getData( model ) ).to.be.equal( 'fobazz[]r' ); expect( buffer.size ).to.be.equal( 4 ); } ); it( 'replaces text for range within single element on the end', () => { - setData( model, '

fooba[r]

' ); + setData( model, 'fooba[r]' ); editor.execute( 'input', { text: 'zzz', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

foobazzz[]

' ); + expect( getData( model ) ).to.be.equal( 'foobazzz[]' ); expect( buffer.size ).to.be.equal( 3 ); } ); it( 'replaces text for range within multiple elements', () => { - setData( model, '

F[OO

b]ar

' ); + setData( model, 'F[OOb]ar' ); editor.execute( 'input', { text: 'unny c', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

Funny c[]ar

' ); + expect( getData( model ) ).to.be.equal( 'Funny c[]ar' ); expect( buffer.size ).to.be.equal( 6 ); } ); it( 'uses current selection when range is not given', () => { - setData( model, '

foob[ar]

' ); + setData( model, 'foob[ar]' ); editor.execute( 'input', { text: 'az' } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

foobaz[]

' ); + expect( getData( model ) ).to.be.equal( 'foobaz[]' ); expect( buffer.size ).to.be.equal( 2 ); } ); it( 'only removes content when empty text given', () => { - setData( model, '

[fo]obar

' ); + setData( model, '[fo]obar' ); editor.execute( 'input', { text: '', range: doc.selection.getFirstRange() } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

[]obar

' ); + expect( getData( model ) ).to.be.equal( '[]obar' ); expect( buffer.size ).to.be.equal( 0 ); } ); it( 'should set selection according to passed resultRange (collapsed)', () => { - setData( model, '

[foo]bar

' ); + setData( model, '[foo]bar' ); editor.execute( 'input', { text: 'new', resultRange: editor.model.createRange( editor.model.createPositionFromPath( doc.getRoot(), [ 0, 5 ] ) ) } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

newba[]r

' ); + expect( getData( model ) ).to.be.equal( 'newba[]r' ); expect( buffer.size ).to.be.equal( 3 ); } ); it( 'should set selection according to passed resultRange (non-collapsed)', () => { - setData( model, '

[foo]bar

' ); + setData( model, '[foo]bar' ); editor.execute( 'input', { text: 'new', @@ -197,30 +197,30 @@ describe( 'InputCommand', () => { ) } ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

new[bar]

' ); + expect( getData( model ) ).to.be.equal( 'new[bar]' ); expect( buffer.size ).to.be.equal( 3 ); } ); it( 'only removes content when no text given (with default non-collapsed range)', () => { - setData( model, '

[fo]obar

' ); + setData( model, '[fo]obar' ); editor.execute( 'input' ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

[]obar

' ); + expect( getData( model ) ).to.be.equal( '[]obar' ); expect( buffer.size ).to.be.equal( 0 ); } ); it( 'does not change selection and content when no text given (with default collapsed range)', () => { - setData( model, '

fo[]obar

' ); + setData( model, 'fo[]obar' ); editor.execute( 'input' ); - expect( getData( model, { selection: true } ) ).to.be.equal( '

fo[]obar

' ); + expect( getData( model ) ).to.be.equal( 'fo[]obar' ); expect( buffer.size ).to.be.equal( 0 ); } ); it( 'does not create insert delta when no text given', () => { - setData( model, '

foo[]bar

' ); + setData( model, 'foo[]bar' ); const version = doc.version; @@ -228,9 +228,62 @@ describe( 'InputCommand', () => { expect( doc.version ).to.equal( version ); } ); + + it( 'handles multi-range selection', () => { + model.schema.register( 'object', { + allowWhere: '$block', + allowContentOf: '$block', + isObject: true + } ); + + setData( + model, + 'x' + + '[y]' + + 'y' + + '[y]' + + 'z' + ); + + // deleteContent() does not support multi-range selections yet, so we need to mock it here. + // See https://github.com/ckeditor/ckeditor5/issues/6328. + model.on( 'deleteContent', ( evt, args ) => { + const [ selection ] = args; + + if ( selection.rangeCount != 2 ) { + return; + } + + evt.stop(); + + model.change( writer => { + let rangeSelection; + + for ( const range of selection.getRanges() ) { + rangeSelection = writer.createSelection( range ); + + model.deleteContent( rangeSelection ); + } + + writer.setSelection( rangeSelection ); + } ); + }, { priority: 'high' } ); + + editor.execute( 'input', { + text: 'foo' + } ); + + expect( getData( model ) ).to.be.equal( + 'x' + + '' + + 'y' + + 'foo[]' + + 'z' + ); + } ); } ); - describe( 'destroy', () => { + describe( 'destroy()', () => { it( 'should destroy change buffer', () => { const command = editor.commands.get( 'input' ); const destroy = command._buffer.destroy = testUtils.sinon.spy();