From 83f791197e7ad68b6593d03c9174134b02f6a6fc Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Thu, 4 Jun 2020 17:44:38 +0200 Subject: [PATCH 1/3] Trimming pasted content for table to table pasting. --- .../ckeditor5-table/src/tableclipboard.js | 35 ++++- .../tests/tableclipboard-paste.js | 125 ++++++++++++++++++ 2 files changed, 155 insertions(+), 5 deletions(-) diff --git a/packages/ckeditor5-table/src/tableclipboard.js b/packages/ckeditor5-table/src/tableclipboard.js index e89179c14ee..a8fc7f8ccf2 100644 --- a/packages/ckeditor5-table/src/tableclipboard.js +++ b/packages/ckeditor5-table/src/tableclipboard.js @@ -120,7 +120,7 @@ export default class TableClipboard extends Plugin { } // We might need to crop table before inserting so reference might change. - let pastedTable = getTableIfOnlyTableInContent( content ); + let pastedTable = getTableIfOnlyTableInContent( content, model ); if ( !pastedTable ) { return; @@ -336,7 +336,7 @@ function expandTableSize( table, expectedHeight, expectedWidth, writer, tableUti } } -function getTableIfOnlyTableInContent( content ) { +function getTableIfOnlyTableInContent( content, model ) { // Table passed directly. if ( content.is( 'table' ) ) { return content; @@ -344,11 +344,36 @@ function getTableIfOnlyTableInContent( content ) { // We do not support mixed content when pasting table into table. // See: https://github.com/ckeditor/ckeditor5/issues/6817. - if ( content.childCount != 1 || !content.getChild( 0 ).is( 'table' ) ) { - return null; + if ( content.childCount == 1 && content.getChild( 0 ).is( 'table' ) ) { + return content.getChild( 0 ); } - return content.getChild( 0 ); + // If there are only whitespaces around a table then use that table for pasting. + + const contentRange = model.createRangeIn( content ); + + for ( const element of contentRange.getItems() ) { + if ( element.is( 'table' ) ) { + // Stop checking if there is some content before table. + const rangeBefore = model.createRange( contentRange.start, model.createPositionBefore( element ) ); + + if ( model.hasContent( rangeBefore, { ignoreWhitespaces: true } ) ) { + return null; + } + + // Stop checking if there is some content after table. + const rangeAfter = model.createRange( model.createPositionAfter( element ), contentRange.end ); + + if ( model.hasContent( rangeAfter, { ignoreWhitespaces: true } ) ) { + return null; + } + + // There wasn't any content neither before nor after. + return element; + } + } + + return null; } // Returns two-dimensional array that is addressed by [ row ][ column ] that stores cells anchored at given location. diff --git a/packages/ckeditor5-table/tests/tableclipboard-paste.js b/packages/ckeditor5-table/tests/tableclipboard-paste.js index 84ef499821d..1c2fa8599ef 100644 --- a/packages/ckeditor5-table/tests/tableclipboard-paste.js +++ b/packages/ckeditor5-table/tests/tableclipboard-paste.js @@ -248,6 +248,131 @@ describe( 'table clipboard', () => { ] ) ); } ); + it( 'should alter model.insertContent if mixed content is pasted (table + empty paragraph)', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + const table = viewTable( [ + [ 'aa', 'ab' ], + [ 'ba', 'bb' ] ] ); + + const data = { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + data.dataTransfer.setData( 'text/html', `${ table }

 

` ); + viewDocument.fire( 'paste', data ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'aa', 'ab', '02', '03' ], + [ 'ba', 'bb', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + + it( 'should alter model.insertContent if mixed content is pasted (table + br)', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + const table = viewTable( [ + [ 'aa', 'ab' ], + [ 'ba', 'bb' ] ] ); + + const data = { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + data.dataTransfer.setData( 'text/html', `${ table }
` ); + viewDocument.fire( 'paste', data ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'aa', 'ab', '02', '03' ], + [ 'ba', 'bb', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + + it( 'should alter model.insertContent if mixed content is pasted (empty paragraph + table)', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + const table = viewTable( [ + [ 'aa', 'ab' ], + [ 'ba', 'bb' ] ] ); + + const data = { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + data.dataTransfer.setData( 'text/html', `

 

${ table }` ); + viewDocument.fire( 'paste', data ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'aa', 'ab', '02', '03' ], + [ 'ba', 'bb', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + + it( 'should alter model.insertContent if mixed content is pasted (br + table)', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + const table = viewTable( [ + [ 'aa', 'ab' ], + [ 'ba', 'bb' ] ] ); + + const data = { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + data.dataTransfer.setData( 'text/html', `
${ table }` ); + viewDocument.fire( 'paste', data ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'aa', 'ab', '02', '03' ], + [ 'ba', 'bb', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + + it( 'should not alter model.insertContent if element other than a table is passed directly', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + model.change( writer => { + const element = writer.createElement( 'paragraph' ); + + writer.insertText( 'foo', element, 0 ); + model.insertContent( element ); + } ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'foo', '', '02', '03' ], + [ '', '', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + it( 'should alter model.insertContent if selectable is a document selection', () => { tableSelection.setCellSelection( modelRoot.getNodeByPath( [ 0, 0, 0 ] ), From c253d51ff96e80390085a350bd2bb545670f34ef Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 5 Jun 2020 11:24:36 +0200 Subject: [PATCH 2/3] Fixed tests code style. --- .../tests/tableclipboard-paste.js | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/packages/ckeditor5-table/tests/tableclipboard-paste.js b/packages/ckeditor5-table/tests/tableclipboard-paste.js index 1c2fa8599ef..bc5dc379720 100644 --- a/packages/ckeditor5-table/tests/tableclipboard-paste.js +++ b/packages/ckeditor5-table/tests/tableclipboard-paste.js @@ -178,7 +178,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -204,7 +205,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -230,7 +232,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -256,7 +259,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -282,7 +286,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -308,7 +313,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), @@ -334,7 +340,8 @@ describe( 'table clipboard', () => { const table = viewTable( [ [ 'aa', 'ab' ], - [ 'ba', 'bb' ] ] ); + [ 'ba', 'bb' ] + ] ); const data = { dataTransfer: createDataTransfer(), From 1c371f1b9418191946d71e268a6a18467bd14fed Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Fri, 5 Jun 2020 11:42:24 +0200 Subject: [PATCH 3/3] Added test for multiple empty elements around a table while pasting it. --- .../tests/tableclipboard-paste.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/ckeditor5-table/tests/tableclipboard-paste.js b/packages/ckeditor5-table/tests/tableclipboard-paste.js index bc5dc379720..b14e9335204 100644 --- a/packages/ckeditor5-table/tests/tableclipboard-paste.js +++ b/packages/ckeditor5-table/tests/tableclipboard-paste.js @@ -359,6 +359,33 @@ describe( 'table clipboard', () => { ] ) ); } ); + it( 'should alter model.insertContent if mixed content is pasted (p + p + table + p + br)', () => { + tableSelection.setCellSelection( + modelRoot.getNodeByPath( [ 0, 0, 0 ] ), + modelRoot.getNodeByPath( [ 0, 1, 1 ] ) + ); + + const table = viewTable( [ + [ 'aa', 'ab' ], + [ 'ba', 'bb' ] + ] ); + + const data = { + dataTransfer: createDataTransfer(), + preventDefault: sinon.spy(), + stopPropagation: sinon.spy() + }; + data.dataTransfer.setData( 'text/html', `

 

 

${ table }

 


` ); + viewDocument.fire( 'paste', data ); + + assertEqualMarkup( getModelData( model, { withoutSelection: true } ), modelTable( [ + [ 'aa', 'ab', '02', '03' ], + [ 'ba', 'bb', '12', '13' ], + [ '20', '21', '22', '23' ], + [ '30', '31', '32', '33' ] + ] ) ); + } ); + it( 'should not alter model.insertContent if element other than a table is passed directly', () => { tableSelection.setCellSelection( modelRoot.getNodeByPath( [ 0, 0, 0 ] ),