From 26c95ae963f4f8ba10e989b196974f6a34bff9a4 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 3 Jun 2020 20:28:03 +0200 Subject: [PATCH 1/3] Added Writer#cloneElement(). --- .../ckeditor5-engine/src/model/element.js | 2 +- packages/ckeditor5-engine/src/model/writer.js | 12 +++++++ .../ckeditor5-engine/tests/model/writer.js | 35 +++++++++++++++++++ .../ckeditor5-table/src/tableclipboard.js | 2 +- .../ckeditor5-table/src/utils/structure.js | 2 +- 5 files changed, 50 insertions(+), 3 deletions(-) diff --git a/packages/ckeditor5-engine/src/model/element.js b/packages/ckeditor5-engine/src/model/element.js index 202771e8f81..7691850c491 100644 --- a/packages/ckeditor5-engine/src/model/element.js +++ b/packages/ckeditor5-engine/src/model/element.js @@ -232,7 +232,7 @@ export default class Element extends Node { /** * Creates a copy of this element and returns it. Created element has the same name and attributes as the original element. - * If clone is deep, the original element's children are also cloned. If not, then empty element is removed. + * If clone is deep, the original element's children are also cloned. If not, then empty element is returned. * * @protected * @param {Boolean} [deep=false] If set to `true` clones element and all its children recursively. When set to `false`, diff --git a/packages/ckeditor5-engine/src/model/writer.js b/packages/ckeditor5-engine/src/model/writer.js index 9b3cf5357fc..f61782606bb 100644 --- a/packages/ckeditor5-engine/src/model/writer.js +++ b/packages/ckeditor5-engine/src/model/writer.js @@ -116,6 +116,18 @@ export default class Writer { return new DocumentFragment(); } + /** + * Creates a copy of the element and returns it. Created element has the same name and attributes as the original element. + * If clone is deep, the original element's children are also cloned. If not, then empty element is returned. + * + * @param {module:engine/model/element~Element} element The element to clone. + * @param {Boolean} [deep=true] If set to `true` clones element and all its children recursively. When set to `false`, + * element will be cloned without any child. + */ + cloneElement( element, deep = true ) { + return element._clone( deep ); + } + /** * Inserts item on given position. * diff --git a/packages/ckeditor5-engine/tests/model/writer.js b/packages/ckeditor5-engine/tests/model/writer.js index 2f07e49f44c..cef91af2034 100644 --- a/packages/ckeditor5-engine/tests/model/writer.js +++ b/packages/ckeditor5-engine/tests/model/writer.js @@ -86,6 +86,35 @@ describe( 'Writer', () => { } ); } ); + describe( 'cloneElement()', () => { + it( 'should make deep clone of element', () => { + const element = createElement( 'foo', { 'abc': '123' } ); + + insertElement( createElement( 'bar', { 'xyz': '789' } ), element ); + + const clonedElement = cloneElement( element ); + + expect( clonedElement ).to.not.equal( element ); + expect( clonedElement.getChild( 0 ) ).to.not.equal( element.getChild( 0 ) ); + expect( clonedElement.toJSON() ).to.deep.equal( element.toJSON() ); + } ); + + it( 'should make shallow copy of element', () => { + const element = createElement( 'foo', { 'abc': '123' } ); + + insertElement( createElement( 'bar', { 'xyz': '789' } ), element ); + + const elementJson = element.toJSON(); + delete elementJson.children; + + const clonedElement = cloneElement( element, false ); + + expect( clonedElement ).to.not.equal( element ); + expect( clonedElement.childCount ).to.equal( 0 ); + expect( clonedElement.toJSON() ).to.deep.equal( elementJson ); + } ); + } ); + describe( 'insert()', () => { it( 'should insert node at given position', () => { const parent = createDocumentFragment(); @@ -2902,6 +2931,12 @@ describe( 'Writer', () => { } ); } + function cloneElement( element, deep = true ) { + return model.change( writer => { + return writer.cloneElement( element, deep ); + } ); + } + function insert( item, itemOrPosition, offset ) { model.enqueueChange( batch, writer => { writer.insert( item, itemOrPosition, offset ); diff --git a/packages/ckeditor5-table/src/tableclipboard.js b/packages/ckeditor5-table/src/tableclipboard.js index e7d6f8d9054..a594f613cb7 100644 --- a/packages/ckeditor5-table/src/tableclipboard.js +++ b/packages/ckeditor5-table/src/tableclipboard.js @@ -295,7 +295,7 @@ function replaceSelectedCellsWithPasted( pastedTable, pastedDimensions, selected // Clone cell to insert (to duplicate its attributes and children). // Cloning is required to support repeating pasted table content when inserting to a bigger selection. - const cellToInsert = pastedCell._clone( true ); + const cellToInsert = writer.cloneElement( pastedCell ); // Trim the cell if it's row/col-spans would exceed selection area. trimTableCellIfNeeded( cellToInsert, row, column, lastRowOfSelection, lastColumnOfSelection, writer ); diff --git a/packages/ckeditor5-table/src/utils/structure.js b/packages/ckeditor5-table/src/utils/structure.js index 4b818f2581b..7fbe517a58c 100644 --- a/packages/ckeditor5-table/src/utils/structure.js +++ b/packages/ckeditor5-table/src/utils/structure.js @@ -75,7 +75,7 @@ export function cropTableToDimensions( sourceTable, cropDimensions, writer ) { } // Otherwise clone the cell with all children and trim if it exceeds cropped area. else { - const tableCellCopy = tableCell._clone( true ); + const tableCellCopy = writer.cloneElement( tableCell ); writer.append( tableCellCopy, row ); From 21b45693761d6e275fa520b213ac2b1fd00b55d2 Mon Sep 17 00:00:00 2001 From: Kuba Niegowski Date: Wed, 3 Jun 2020 21:11:44 +0200 Subject: [PATCH 2/3] Updated test to bring 100CC --- packages/ckeditor5-engine/tests/model/writer.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/ckeditor5-engine/tests/model/writer.js b/packages/ckeditor5-engine/tests/model/writer.js index cef91af2034..093d010aeb1 100644 --- a/packages/ckeditor5-engine/tests/model/writer.js +++ b/packages/ckeditor5-engine/tests/model/writer.js @@ -87,7 +87,7 @@ describe( 'Writer', () => { } ); describe( 'cloneElement()', () => { - it( 'should make deep clone of element', () => { + it( 'should make deep copy of element', () => { const element = createElement( 'foo', { 'abc': '123' } ); insertElement( createElement( 'bar', { 'xyz': '789' } ), element ); @@ -2931,7 +2931,7 @@ describe( 'Writer', () => { } ); } - function cloneElement( element, deep = true ) { + function cloneElement( element, deep ) { return model.change( writer => { return writer.cloneElement( element, deep ); } ); From fda259351bbd601e83c6f7f00a3588651a32a7f2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20Go=C5=82aszewski?= Date: Thu, 4 Jun 2020 08:19:22 +0200 Subject: [PATCH 3/3] Use writer.cloneElement() in getSelectedContent(). --- packages/ckeditor5-engine/src/model/utils/getselectedcontent.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ckeditor5-engine/src/model/utils/getselectedcontent.js b/packages/ckeditor5-engine/src/model/utils/getselectedcontent.js index 95501c8088e..2419341ebbb 100644 --- a/packages/ckeditor5-engine/src/model/utils/getselectedcontent.js +++ b/packages/ckeditor5-engine/src/model/utils/getselectedcontent.js @@ -73,7 +73,7 @@ export default function getSelectedContent( model, selection ) { if ( item.is( 'textProxy' ) ) { writer.appendText( item.data, item.getAttributes(), frag ); } else { - writer.append( item._clone( true ), frag ); + writer.append( writer.cloneElement( item, true ), frag ); } }