Merge pull request #1124 from ckeditor/t/1120b

Feature: Introduced `Schema#removeDisallowedAttributes` method to filter out disallowed by schema attributes from given nodes. Closes #1120.

Author: scofalik

src/controller/deletecontent.js

@@ -41,9 +41,11 @@ export default function deleteContent( selection, batch, options = {} ) {
 		return;
 	}
 
+	const schema = batch.document.schema;
+
 	// 1. Replace the entire content with paragraph.
 	// See: https://github.com/ckeditor/ckeditor5-engine/issues/1012#issuecomment-315017594.
-	if ( !options.doNotResetEntireContent && shouldEntireContentBeReplacedWithParagraph( batch.document.schema, selection ) ) {
+	if ( !options.doNotResetEntireContent && shouldEntireContentBeReplacedWithParagraph( schema, selection ) ) {
 		replaceEntireContentWithParagraph( batch, selection );
 
 		return;
@@ -74,14 +76,14 @@ 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( startPos.parent.getChildren(), startPos, batch );
+		schema.removeDisallowedAttributes( startPos.parent.getChildren(), startPos, batch );
 	}
 
 	selection.setCollapsedAt( startPos );
 
 	// 4. Autoparagraphing.
 	// Check if a text is allowed in the new container. If not, try to create a new paragraph (if it's allowed here).
-	if ( shouldAutoparagraph( batch.document, startPos ) ) {
+	if ( shouldAutoparagraph( schema, startPos ) ) {
 		insertParagraph( batch, startPos, selection );
 	}
 
@@ -158,9 +160,9 @@ function mergeBranches( batch, startPos, endPos ) {
 	mergeBranches( batch, startPos, endPos );
 }
 
-function shouldAutoparagraph( doc, position ) {
-	const isTextAllowed = doc.schema.check( { name: '$text', inside: position } );
-	const isParagraphAllowed = doc.schema.check( { name: 'paragraph', inside: position } );
+function shouldAutoparagraph( schema, position ) {
+	const isTextAllowed = schema.check( { name: '$text', inside: position } );
+	const isParagraphAllowed = schema.check( { name: 'paragraph', inside: position } );
 
 	return !isTextAllowed && isParagraphAllowed;
 }
@@ -217,39 +219,3 @@ function shouldEntireContentBeReplacedWithParagraph( schema, selection ) {
 
 	return schema.check( { name: 'paragraph', inside: limitElement.name } );
 }
-
-// Gets a name under which we should check this node in the schema.
-//
-// @param {module:engine/model/node~Node} node The node.
-// @returns {String} node name.
-function getNodeSchemaName( node ) {
-	return node.is( 'text' ) ? '$text' : node.name;
-}
-
-// Creates AttributeDeltas that removes attributes that are disallowed by schema on given node and its children.
-//
-// @param {Array<module:engine/model/node~Node>} nodes Nodes that will be filtered.
-// @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked.
-// @param {module:engine/model/batch~Batch} batch Batch to which the deltas will be added.
-function removeDisallowedAttributes( nodes, inside, batch ) {
-	const schema = batch.document.schema;
-
-	for ( const node of nodes ) {
-		const name = getNodeSchemaName( node );
-
-		// When node with attributes is not allowed in current position.
-		if ( !schema.check( { name, inside, 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, attributes: attribute } ) ) {
-					batch.removeAttribute( node, attribute );
-				}
-			}
-		}
-
-		if ( node.is( 'element' ) ) {
-			removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), batch );
-		}
-	}
-}

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( [ node ], this.position, this.schema );
+			this.schema.removeDisallowedAttributes( [ node ], this.position );
 			this._handleNode( node, context );
 		}
 		// If text is not allowed, try autoparagraphing.
@@ -291,7 +291,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.
 			const parent = position.nodeBefore;
-			removeDisallowedAttributes( parent.getChildren(), Position.createAt( parent ), this.schema, this.batch );
+			this.schema.removeDisallowedAttributes( parent.getChildren(), Position.createAt( parent ), this.batch );
 
 			this.position = Position.createFromPosition( position );
 			position.detach();
@@ -318,7 +318,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.
-			removeDisallowedAttributes( position.parent.getChildren(), position, this.schema, this.batch );
+			this.schema.removeDisallowedAttributes( position.parent.getChildren(), position, this.batch );
 
 			this.position = Position.createFromPosition( position );
 			position.detach();
@@ -330,7 +330,7 @@ class Insertion {
 		// When there was no merge we need to check and strip disallowed attributes in all nested nodes of
 		// just inserted node because some attributes could end up in a place where are disallowed.
 		if ( !mergeLeft && !mergeRight ) {
-			removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), this.schema, this.batch );
+			this.schema.removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), this.batch );
 		}
 	}
 
@@ -350,7 +350,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( [ node ], [ paragraph ], this.schema );
+				this.schema.removeDisallowedAttributes( [ node ], [ paragraph ] );
 			}
 
 			if ( this._checkIsAllowed( node, [ paragraph ] ) ) {
@@ -453,36 +453,3 @@ class Insertion {
 function getNodeSchemaName( node ) {
 	return node.is( 'text' ) ? '$text' : node.name;
 }
-
-// Removes disallowed by schema attributes from given nodes. When batch parameter is provided then
-// attributes will be removed by creating AttributeDeltas otherwise attributes will be removed
-// directly from provided nodes.
-//
-// @param {Array<module:engine/model/node~Node>} nodes Nodes that will be filtered.
-// @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked.
-// @param {module:engine/model/schema~Schema} schema Schema instance uses for element validation.
-// @param {module:engine/model/batch~Batch} [batch] Batch to which the deltas will be added.
-function removeDisallowedAttributes( nodes, inside, schema, batch ) {
-	for ( const node of nodes ) {
-		const name = getNodeSchemaName( node );
-
-		// When node with attributes is not allowed in current position.
-		if ( !schema.check( { name, inside, 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, attributes: attribute } ) ) {
-					if ( batch ) {
-						batch.removeAttribute( node, attribute );
-					} else {
-						node.removeAttribute( attribute );
-					}
-				}
-			}
-		}
-
-		if ( node.is( 'element' ) ) {
-			removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), schema, batch );
-		}
-	}
-}

src/model/schema.js

@@ -9,11 +9,11 @@
 
 import Position from './position';
 import Element from './element';
+import Range from './range';
 import clone from '@ckeditor/ckeditor5-utils/src/lib/lodash/clone';
 import isArray from '@ckeditor/ckeditor5-utils/src/lib/lodash/isArray';
 import isString from '@ckeditor/ckeditor5-utils/src/lib/lodash/isString';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
-import Range from './range';
 
 /**
  * Schema is a definition of the structure of the document. It allows to define which tree model items (element, text, etc.)
@@ -415,6 +415,43 @@ export default class Schema {
 		return element;
 	}
 
+	/**
+	 * Removes disallowed by {@link module:engine/model/schema~Schema schema} attributes from given nodes.
+	 * When {@link module:engine/model/batch~Batch batch} parameter is provided then attributes will be removed
+	 * using that batch, by creating {@link module:engine/model/delta/attributedelta~AttributeDelta attribute deltas}.
+	 * Otherwise, attributes will be removed directly from provided nodes using {@link module:engine/model/node~Node node} API.
+	 *
+	 * @param {Iterable.<module:engine/model/node~Node>} nodes Nodes that will be filtered.
+	 * @param {module:engine/model/schema~SchemaPath} inside Path inside which schema will be checked.
+	 * @param {module:engine/model/batch~Batch} [batch] Batch to which the deltas will be added.
+	 */
+	removeDisallowedAttributes( nodes, inside, batch ) {
+		for ( const node of nodes ) {
+			const name = node.is( 'text' ) ? '$text' : node.name;
+			const attributes = Array.from( node.getAttributeKeys() );
+			const queryPath = Schema._normalizeQueryPath( inside );
+
+			// When node with attributes is not allowed in current position.
+			if ( !this.check( { name, attributes, inside: queryPath } ) ) {
+				// Let's remove attributes one by one.
+				// TODO: this should be improved to check all combination of attributes.
+				for ( const attribute of node.getAttributeKeys() ) {
+					if ( !this.check( { name, attributes: attribute, inside: queryPath } ) ) {
+						if ( batch ) {
+							batch.removeAttribute( node, attribute );
+						} else {
+							node.removeAttribute( attribute );
+						}
+					}
+				}
+			}
+
+			if ( node.is( 'element' ) ) {
+				this.removeDisallowedAttributes( node.getChildren(), queryPath.concat( node.name ), batch );
+			}
+		}
+	}
+
 	/**
 	 * Returns {@link module:engine/model/schema~SchemaItem schema item} that was registered in the schema under given name.
 	 * If item has not been found, throws error.