diff --git a/src/view/writer.js b/src/view/writer.js index 2ac2450b8..b624d92bb 100644 --- a/src/view/writer.js +++ b/src/view/writer.js @@ -941,12 +941,13 @@ export default class Writer { if ( isText || isEmpty || isUI || ( isAttribute && shouldABeOutsideB( attribute, child ) ) ) { // Clone attribute. const newAttribute = attribute._clone(); - this._addToClonedElementsGroup( newAttribute ); // Wrap current node with new attribute. child._remove(); newAttribute._appendChild( child ); + parent._insertChild( i, newAttribute ); + this._addToClonedElementsGroup( newAttribute ); wrapPositions.push( new Position( parent, i ) ); } @@ -1006,10 +1007,10 @@ export default class Writer { // Replace wrapper element with its children child._remove(); - this._removeFromClonedElementsGroup( child ); - parent._insertChild( i, unwrapped ); + this._removeFromClonedElementsGroup( child ); + // Save start and end position of moved items. unwrapPositions.push( new Position( parent, i ), @@ -1415,10 +1416,10 @@ export default class Writer { // Break element. const clonedNode = positionParent._clone(); - this._addToClonedElementsGroup( clonedNode ); // Insert cloned node to position's parent node. positionParent.parent._insertChild( offsetAfter, clonedNode ); + this._addToClonedElementsGroup( clonedNode ); // Get nodes to move. const count = positionParent.childCount - positionOffset; @@ -1447,6 +1448,19 @@ export default class Writer { * @param {module:engine/view/attributeelement~AttributeElement} element Attribute element to save. */ _addToClonedElementsGroup( element ) { + // Add only if the element is in document tree. + if ( !element.root.is( 'rootElement' ) ) { + return; + } + + // Traverse the element's children recursively to find other attribute elements that also might got inserted. + // The loop is at the beginning so we can make fast returns later in the code. + if ( element.is( 'element' ) ) { + for ( const child of element.getChildren() ) { + this._addToClonedElementsGroup( child ); + } + } + const id = element.id; if ( !id ) { @@ -1477,6 +1491,14 @@ export default class Writer { * @param {module:engine/view/attributeelement~AttributeElement} element Attribute element to remove. */ _removeFromClonedElementsGroup( element ) { + // Traverse the element's children recursively to find other attribute elements that also got removed. + // The loop is at the beginning so we can make fast returns later in the code. + if ( element.is( 'element' ) ) { + for ( const child of element.getChildren() ) { + this._removeFromClonedElementsGroup( child ); + } + } + const id = element.id; if ( !id ) { @@ -1485,6 +1507,10 @@ export default class Writer { const group = this._cloneGroups.get( id ); + if ( !group ) { + return; + } + group.delete( element ); // Not removing group from element on purpose! // If other parts of code have reference to this element, they will be able to get references to other elements from the group. diff --git a/tests/view/writer/writer.js b/tests/view/writer/writer.js index 58a29b1e5..853de8811 100644 --- a/tests/view/writer/writer.js +++ b/tests/view/writer/writer.js @@ -13,7 +13,7 @@ import createViewRoot from '../_utils/createroot'; describe( 'Writer', () => { let writer, attributes, root, doc; - before( () => { + beforeEach( () => { attributes = { foo: 'bar', baz: 'quz' }; doc = new Document(); root = createViewRoot( doc ); @@ -43,6 +43,9 @@ describe( 'Writer', () => { describe( 'setSelectionFocus()', () => { it( 'should use selection._setFocus method internally', () => { + const position = ViewPosition.createAt( root ); + writer.setSelection( position ); + const spy = sinon.spy( writer.document.selection, '_setFocus' ); writer.setSelectionFocus( root, 0 ); @@ -255,10 +258,9 @@ describe( 'Writer', () => { describe( 'manages AttributeElement#_clonesGroup', () => { it( 'should return all clones of a broken attribute element with id', () => { - const container = writer.createContainerElement( 'div' ); const text = writer.createText( 'abccccde' ); - writer.insert( ViewPosition.createAt( container, 0 ), text ); + writer.insert( ViewPosition.createAt( root, 0 ), text ); const span = writer.createAttributeElement( 'span', null, { id: 'foo' } ); span._priority = 20; @@ -271,8 +273,8 @@ describe( 'Writer', () => { //
foo
+ + const span = writer.createAttributeElement( 'span', null, { id: 'span' } ); + + //foo
+ writer.wrap( ViewRange.createFromParentsAndOffsets( foo, 0, foo, 3 ), span ); + + // Find the span. + const createdSpan = p.getChild( 0 ); + + expect( createdSpan.getElementsWithSameId().size ).to.equal( 0 ); + } ); + + it( 'should add attribute elements to clone groups deeply', () => { + const p = writer.createContainerElement( 'p' ); + const foo = writer.createText( 'foo' ); + writer.insert( ViewPosition.createAt( p, 0 ), foo ); + //foo
+ + const span = writer.createAttributeElement( 'span', null, { id: 'span' } ); + + //foo
+ writer.wrap( ViewRange.createFromParentsAndOffsets( foo, 0, foo, 3 ), span ); + + //foo
+ writer.insert( ViewPosition.createAt( root, 0 ), p ); + + // Find the span. + const createdSpan = p.getChild( 0 ); + + expect( Array.from( createdSpan.getElementsWithSameId() ) ).to.deep.equal( [ createdSpan ] ); + } ); + + it( 'should remove attribute elements from clone groups deeply', () => { + const p1 = writer.createContainerElement( 'p' ); + const p2 = writer.createContainerElement( 'p' ); + const foo = writer.createText( 'foo' ); + const bar = writer.createText( 'bar' ); + + writer.insert( ViewPosition.createAt( root, 0 ), p1 ); + writer.insert( ViewPosition.createAt( root, 1 ), p2 ); + writer.insert( ViewPosition.createAt( p1, 0 ), foo ); + writer.insert( ViewPosition.createAt( p2, 0 ), bar ); + //foo
bar
foo
bar
foo
bar
bar