diff --git a/src/model/schema.js b/src/model/schema.js index 311a43d41..5c9d9cb52 100644 --- a/src/model/schema.js +++ b/src/model/schema.js @@ -733,14 +733,23 @@ export default class Schema { */ removeDisallowedAttributes( nodes, writer ) { for ( const node of nodes ) { - for ( const attribute of node.getAttributeKeys() ) { - if ( !this.checkAttribute( node, attribute ) ) { - writer.removeAttribute( attribute, node ); - } + // When node is a `Text` it has no children, so just filter it out. + if ( node.is( 'text' ) ) { + removeDisallowedAttributeFromNode( this, node, writer ); } - - if ( node.is( 'element' ) ) { - this.removeDisallowedAttributes( node.getChildren(), writer ); + // In a case of `Element` iterates through positions between nodes inside this element + // and filter out node before the current position, or position parent when position + // is at start of an element. Using positions prevent from omitting merged nodes + // see https://github.com/ckeditor/ckeditor5-engine/issues/1789. + else { + const rangeInNode = Range._createIn( node ); + const positionsInRange = rangeInNode.getPositions(); + + for ( const position of positionsInRange ) { + const item = position.nodeBefore || position.parent; + + removeDisallowedAttributeFromNode( this, item, writer ); + } } } } @@ -1592,3 +1601,11 @@ function* convertToMinimalFlatRanges( ranges ) { yield* range.getMinimalFlatRanges(); } } + +function removeDisallowedAttributeFromNode( schema, node, writer ) { + for ( const attribute of node.getAttributeKeys() ) { + if ( !schema.checkAttribute( node, attribute ) ) { + writer.removeAttribute( attribute, node ); + } + } +} diff --git a/tests/model/schema.js b/tests/model/schema.js index ce52e35ef..6c7622778 100644 --- a/tests/model/schema.js +++ b/tests/model/schema.js @@ -1769,6 +1769,21 @@ describe( 'Schema', () => { } ); } ); + it( 'should filter out disallowed attributes from empty element', () => { + schema.extend( 'div', { allowAttributes: 'a' } ); + + const div = new Element( 'div', { a: 1, b: 1 } ); + + root._appendChild( [ div ] ); + + model.change( writer => { + schema.removeDisallowedAttributes( [ div ], writer ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( '
' ); + } ); + } ); + it( 'should filter out disallowed attributes from all descendants of given nodes', () => { schema.addAttributeCheck( ( ctx, attributeName ) => { // Allow 'a' on div>$text. @@ -1790,13 +1805,18 @@ describe( 'Schema', () => { if ( ctx.endsWith( 'div paragraph image' ) && attributeName == 'b' ) { return true; } + + // Allow 'a' on div>paragraph. + if ( ctx.endsWith( 'div paragraph' ) && attributeName == 'a' ) { + return true; + } } ); const foo = new Text( 'foo', { a: 1, b: 1 } ); const bar = new Text( 'bar', { a: 1, b: 1 } ); const imageInDiv = new Element( 'image', { a: 1, b: 1 } ); const imageInParagraph = new Element( 'image', { a: 1, b: 1 } ); - const paragraph = new Element( 'paragraph', [], [ foo, imageInParagraph ] ); + const paragraph = new Element( 'paragraph', { a: 1, b: 1 }, [ foo, imageInParagraph ] ); const div = new Element( 'div', [], [ paragraph, bar, imageInDiv ] ); root._appendChild( [ div ] ); @@ -1804,25 +1824,67 @@ describe( 'Schema', () => { model.change( writer => { schema.removeDisallowedAttributes( root.getChildren(), writer ); - expect( writer.batch.operations ).to.length( 4 ); - expect( writer.batch.operations[ 0 ] ).to.instanceof( AttributeOperation ); - expect( writer.batch.operations[ 1 ] ).to.instanceof( AttributeOperation ); - expect( writer.batch.operations[ 2 ] ).to.instanceof( AttributeOperation ); - expect( writer.batch.operations[ 3 ] ).to.instanceof( AttributeOperation ); - expect( getData( model, { withoutSelection: true } ) ) .to.equal( '
' + - '' + - '<$text b="1">foo' + - '' + - '' + - '<$text a="1">bar' + - '' + + '' + + '<$text b="1">foo' + + '' + + '' + + '<$text a="1">bar' + + '' + '
' ); } ); } ); + + it( 'should filter out disallowed attributes from parent node and all descendants nodes', () => { + schema.extend( 'div', { allowAttributes: 'a' } ); + schema.extend( '$text', { allowAttributes: 'b' } ); + + const foo = new Text( 'foo', { a: 1, b: 1 } ); + const div = new Element( 'div', { a: 1, b: 1 }, [ foo ] ); + + root._appendChild( [ div ] ); + + model.change( writer => { + schema.removeDisallowedAttributes( root.getChildren(), writer ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( '
<$text b="1">foo
' ); + } ); + } ); + + it( 'should filter out all attributes from nodes that are merged while clearing', () => { + const a = new Text( 'a', { a: 1, b: 1 } ); + const b = new Text( 'b', { b: 1 } ); + const c = new Text( 'c', { a: 1, b: 1 } ); + const div = new Element( 'div', [], [ a, b, c ] ); + + root._appendChild( [ div ] ); + + model.change( writer => { + schema.removeDisallowedAttributes( [ div ], writer ); + + expect( getData( model, { withoutSelection: true } ) ).to.equal( '
abc
' ); + } ); + } ); + + it( 'should do not filter out sibling nodes', () => { + const foo = new Text( 'foo', { a: 1 } ); + const bar = new Text( 'bar', { a: 1, b: 1 } ); + const biz = new Text( 'biz', { a: 1 } ); + const div = new Element( 'div', [], [ foo, bar, biz ] ); + + root._appendChild( [ div ] ); + + model.change( writer => { + schema.removeDisallowedAttributes( [ bar ], writer ); + + expect( getData( model, { withoutSelection: true } ) ) + .to.equal( '
<$text a="1">foobar<$text a="1">biz
' ); + } ); + } ); } ); describe( 'definitions compilation', () => {