瀏覽代碼

Improved checking for attributes combinations.

Oskar Wróbel 8 年之前
父節點
當前提交
dbad524854

+ 15 - 8
packages/ckeditor5-engine/src/controller/deletecontent.js

@@ -74,7 +74,7 @@ export default function deleteContent( selection, batch, options = {} ) {
 		//
 		// e.g. bold is disallowed for <H1>
 		// <h1>Fo{o</h1><p>b}a<b>r</b><p> -> <h1>Fo{}a<b>r</b><h1> -> <h1>Fo{}ar<h1>.
-		removeDisallowedAttributes( batch, Array.from( startPos.parent.getChildren() ), startPos );
+		removeDisallowedAttributes( Array.from( startPos.parent.getChildren() ), startPos, batch );
 	}
 
 	selection.setCollapsedAt( startPos );
@@ -228,21 +228,28 @@ function getNodeSchemaName( node ) {
 
 // Adds deltas with `removeAttributes` operation for disallowed by schema attributes on given nodes and its children.
 //
-// @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
-// @param {module:engine/model/node~Node|Array<module:engine/model/node~Node>} nodes List of nodes or a single node to filter.
+// @param {module:engine/model/node~Node|Array<module:engine/model/node~Node>} nodes
 // @param {module:engine/model/schema~SchemaPath} schemaPath
-function removeDisallowedAttributes( batch, nodes, schemaPath ) {
+// @param {module:engine/model/batch~Batch} batch
+function removeDisallowedAttributes( nodes, schemaPath, batch ) {
 	const schema = batch.document.schema;
 
 	for ( const node of nodes ) {
-		for ( const attribute of node.getAttributeKeys() ) {
-			if ( !schema.check( { name: getNodeSchemaName( node ), attributes: attribute, inside: schemaPath } ) ) {
-				batch.removeAttribute( node, attribute );
+		const name = getNodeSchemaName( node );
+
+		// When node with attributes is not allowed in current position.
+		if ( !schema.check( { name, inside: schemaPath, attributes: Array.from( node.getAttributeKeys() ) } ) ) {
+			// Let's remove attributes one by one.
+			// This should be improved to check all combination of attributes.
+			for ( const attribute of node.getAttributeKeys() ) {
+				if ( !schema.check( { name, attributes: attribute, inside: schemaPath } ) ) {
+					batch.removeAttribute( node, attribute );
+				}
 			}
 		}
 
 		if ( node.is( 'element' ) ) {
-			removeDisallowedAttributes( batch, Array.from( node.getChildren() ), Position.createAt( node ) );
+			removeDisallowedAttributes( Array.from( node.getChildren() ), Position.createAt( node ), batch );
 		}
 	}
 }

+ 26 - 29
packages/ckeditor5-engine/src/controller/insertcontent.js

@@ -229,7 +229,7 @@ class Insertion {
 		// If the node is a text and bare text is allowed in current position it means that the node
 		// contains disallowed attributes and we have to remove them.
 		else if ( this.schema.check( { name: '$text', inside: this.position } ) ) {
-			removeDisallowedAttributes( this.schema, [ node ], this.position );
+			removeDisallowedAttributes( [ node ], this.position, this.schema );
 			this._handleNode( node, context );
 		}
 		// If text is not allowed, try autoparagraphing.
@@ -290,7 +290,7 @@ class Insertion {
 
 			// We need to check and strip disallowed attributes in all nested nodes because after merge
 			// some attributes could end up in a path where are disallowed.
-			removeDisallowedAttributesOnBatch( this.batch, Array.from( this.position.parent.getChildren() ), position );
+			removeDisallowedAttributes( Array.from( position.parent.getChildren() ), position, this.schema, this.batch );
 
 			this.position = Position.createFromPosition( position );
 			position.detach();
@@ -317,7 +317,7 @@ class Insertion {
 
 			// We need to check and strip disallowed attributes in all nested nodes because after merge
 			// some attributes could end up in a place where are disallowed.
-			removeDisallowedAttributesOnBatch( this.batch, Array.from( this.position.parent.getChildren() ), position );
+			removeDisallowedAttributes( Array.from( position.parent.getChildren() ), position, this.schema, this.batch );
 
 			this.position = Position.createFromPosition( position );
 			position.detach();
@@ -343,7 +343,7 @@ class Insertion {
 			// When node is a text and is disallowed by schema it means that contains disallowed attributes
 			// and we need to remove them.
 			if ( node.is( 'text' ) && !this._checkIsAllowed( node, [ paragraph ] ) ) {
-				removeDisallowedAttributes( this.schema, [ node ], [ paragraph ] );
+				removeDisallowedAttributes( [ node ], [ paragraph ], this.schema );
 			}
 
 			if ( this._checkIsAllowed( node, [ paragraph ] ) ) {
@@ -447,38 +447,35 @@ function getNodeSchemaName( node ) {
 	return node.is( 'text' ) ? '$text' : node.name;
 }
 
-// Removes disallowed by schema attributes from given nodes.
+// Removes disallowed by schema attributes from given nodes. When batch parameter is provided then
+// attributes will be removed by adding deltas with `removeAttributes` operation to this batch
+// otherwise attributes will be removed directly from provided nodes.
 //
-// @param {module:engine/model/schema~Schema} schema
-// @param {Array<module:engine/model/node~Node>} nodes List of nodes to filter.
-// @param {module:engine/model/schema~SchemaPath} schemaPath
-function removeDisallowedAttributes( schema, nodes, schemaPath ) {
-	for ( const node of nodes ) {
-		for ( const attribute of node.getAttributeKeys() ) {
-			if ( !schema.check( { name: getNodeSchemaName( node ), attributes: attribute, inside: schemaPath } ) ) {
-				node.removeAttribute( attribute );
-			}
-		}
-	}
-}
-
-// Adds deltas with `removeAttributes` operation for disallowed by schema attributes on given nodes and its children.
-//
-// @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
-// @param {module:engine/model/node~Node|Array<module:engine/model/node~Node>} nodes List of nodes or a single node to filter.
+// @param {Array<module:engine/model/node~Node>} nodes
 // @param {module:engine/model/schema~SchemaPath} schemaPath
-function removeDisallowedAttributesOnBatch( batch, nodes, schemaPath ) {
-	const schema = batch.document.schema;
-
+// @param {module:engine/model/schema~Schema} schema
+// @param {module:engine/model/batch~Batch} [batch]
+function removeDisallowedAttributes( nodes, schemaPath, schema, batch ) {
 	for ( const node of nodes ) {
-		for ( const attribute of node.getAttributeKeys() ) {
-			if ( !schema.check( { name: getNodeSchemaName( node ), attributes: attribute, inside: schemaPath } ) ) {
-				batch.removeAttribute( node, attribute );
+		const name = getNodeSchemaName( node );
+
+		// When node with attributes is not allowed in current position.
+		if ( !schema.check( { name, inside: schemaPath, attributes: Array.from( node.getAttributeKeys() ) } ) ) {
+			// Let's remove attributes one by one.
+			// This should be improved to check all combination of attributes.
+			for ( const attribute of node.getAttributeKeys() ) {
+				if ( !schema.check( { name, inside: schemaPath, attributes: attribute } ) ) {
+					if ( batch ) {
+						batch.removeAttribute( node, attribute );
+					} else {
+						node.removeAttribute( attribute );
+					}
+				}
 			}
 		}
 
 		if ( node.is( 'element' ) ) {
-			removeDisallowedAttributesOnBatch( batch, Array.from( node.getChildren() ), Position.createAt( node ) );
+			removeDisallowedAttributes( Array.from( node.getChildren() ), Position.createAt( node ), schema, batch );
 		}
 	}
 }

+ 6 - 4
packages/ckeditor5-engine/tests/controller/insertcontent.js

@@ -604,6 +604,7 @@ describe( 'DataController', () => {
 				schema.allow( { name: 'table', inside: '$clipboardHolder' } );
 				schema.allow( { name: 'td', inside: '$clipboardHolder' } );
 				schema.allow( { name: 'td', inside: 'table' } );
+				schema.allow( { name: 'child', inside: 'td' } );
 				schema.allow( { name: '$block', inside: 'td' } );
 				schema.allow( { name: '$text', inside: 'td' } );
 
@@ -616,6 +617,7 @@ describe( 'DataController', () => {
 				schema.allow( { name: '$text', attributes: 'b', inside: 'paragraph' } );
 				schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'paragraph child' } );
 				schema.allow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'heading1 child' } );
+				schema.allow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'td child' } );
 			} );
 
 			it( 'filters out disallowed elements and leaves out the text', () => {
@@ -648,25 +650,25 @@ describe( 'DataController', () => {
 				expect( getData( doc ) ).to.equal( '<paragraph>fx<$text b="1">x</$text>xyyy[]oo</paragraph>' );
 			} );
 
-			it( 'filters out disallowed attributes when merging #1', () => {
+			it( 'filters out disallowed attributes after merge #1', () => {
 				setData( doc, '<paragraph>[]foo</paragraph>' );
 				insertHelper( '<paragraph>x<$text a="1" b="1">x</$text>x</paragraph>' );
 				expect( getData( doc ) ).to.equal( '<paragraph>x<$text b="1">x</$text>x[]foo</paragraph>' );
 			} );
 
-			it( 'filters out disallowed attributes when merging #2', () => {
+			it( 'filters out disallowed attributes after merge #2', () => {
 				setData( doc, '<paragraph>f[]oo</paragraph>' );
 				insertHelper( '<paragraph>x<$text a="1" b="1">x</$text>x</paragraph>' );
 				expect( getData( doc ) ).to.equal( '<paragraph>fx<$text b="1">x</$text>x[]oo</paragraph>' );
 			} );
 
-			it( 'filters out disallowed attributes when merging #3', () => {
+			it( 'filters out disallowed attributes after merge #3', () => {
 				setData( doc, '<paragraph>foo[]</paragraph>' );
 				insertHelper( '<paragraph>x<$text a="1" b="1">x</$text>x</paragraph>' );
 				expect( getData( doc ) ).to.equal( '<paragraph>foox<$text b="1">x</$text>x[]</paragraph>' );
 			} );
 
-			it( 'filters out disallowed attributes from nested child when merging', () => {
+			it( 'filters out disallowed attributes from nested elements after merge', () => {
 				setData( doc, '<paragraph>f[]oo</paragraph>' );
 				insertHelper( '<heading1>x<child>b<$text a="1" b="1">a</$text>r</child>x</heading1>' );
 				expect( getData( doc ) ).to.equal( '<paragraph>fx<child>b<$text b="1">a</$text>r</child>x[]oo</paragraph>' );