Bläddra i källkod

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.
Szymon Cofalik 8 år sedan
förälder
incheckning
a35d899140

+ 8 - 42
packages/ckeditor5-engine/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 );
-		}
-	}
-}

+ 5 - 38
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( [ 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 );
-		}
-	}
-}

+ 38 - 1
packages/ckeditor5-engine/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.

+ 164 - 1
packages/ckeditor5-engine/tests/model/schema/schema.js

@@ -6,12 +6,15 @@
 import { default as Schema, SchemaItem } from '../../../src/model/schema';
 import Document from '../../../src/model/document';
 import Element from '../../../src/model/element';
+import Text from '../../../src/model/text';
+import DocumentFragment from '../../../src/model/documentfragment';
 import Position from '../../../src/model/position';
 import Range from '../../../src/model/range';
 import Selection from '../../../src/model/selection';
+import AttributeDelta from '../../../src/model/delta/attributedelta';
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
-import { setData, stringify } from '../../../src/dev-utils/model';
+import { setData, getData, stringify } from '../../../src/dev-utils/model';
 
 testUtils.createSinonSandbox();
 
@@ -752,4 +755,164 @@ describe( 'Schema', () => {
 			expect( schema.getLimitElement( doc.selection ) ).to.equal( root );
 		} );
 	} );
+
+	describe( 'removeDisallowedAttributes()', () => {
+		let doc, root;
+
+		beforeEach( () => {
+			doc = new Document();
+			root = doc.createRoot();
+			schema = doc.schema;
+
+			schema.registerItem( 'paragraph', '$block' );
+			schema.registerItem( 'div', '$block' );
+			schema.registerItem( 'image' );
+			schema.objects.add( 'image' );
+			schema.allow( { name: '$block', inside: 'div' } );
+		} );
+
+		describe( 'filtering attributes from nodes', () => {
+			let text, image;
+
+			beforeEach( () => {
+				schema.allow( { name: '$text', attributes: [ 'a' ], inside: '$root' } );
+				schema.allow( { name: 'image', attributes: [ 'b' ], inside: '$root' } );
+
+				text = new Text( 'foo', { a: 1, b: 1 } );
+				image = new Element( 'image', { a: 1, b: 1 } );
+			} );
+
+			it( 'should filter out disallowed attributes from given nodes', () => {
+				schema.removeDisallowedAttributes( [ text, image ], '$root' );
+
+				expect( Array.from( text.getAttributeKeys() ) ).to.deep.equal( [ 'a' ] );
+				expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'b' ] );
+			} );
+
+			it( 'should filter out disallowed attributes from given nodes (batch)', () => {
+				const root = doc.getRoot();
+				const batch = doc.batch();
+
+				root.appendChildren( [ text, image ] );
+
+				schema.removeDisallowedAttributes( [ text, image ], '$root', batch );
+
+				expect( Array.from( text.getAttributeKeys() ) ).to.deep.equal( [ 'a' ] );
+				expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'b' ] );
+
+				expect( batch.deltas ).to.length( 2 );
+				expect( batch.deltas[ 0 ] ).to.instanceof( AttributeDelta );
+				expect( batch.deltas[ 1 ] ).to.instanceof( AttributeDelta );
+			} );
+		} );
+
+		describe( 'filtering attributes from child nodes', () => {
+			let div;
+
+			beforeEach( () => {
+				schema.allow( { name: '$text', attributes: [ 'a' ], inside: 'div' } );
+				schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'div paragraph' } );
+				schema.allow( { name: 'image', attributes: [ 'a' ], inside: 'div' } );
+				schema.allow( { name: 'image', attributes: [ 'b' ], inside: 'div paragraph' } );
+
+				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 ] );
+
+				div = new Element( 'div', [], [ paragraph, bar, imageInDiv ] );
+			} );
+
+			it( 'should filter out disallowed attributes from child nodes', () => {
+				schema.removeDisallowedAttributes( [ div ], '$root' );
+
+				expect( stringify( div ) )
+					.to.equal(
+						'<div>' +
+							'<paragraph>' +
+								'<$text b="1">foo</$text>' +
+								'<image b="1"></image>' +
+							'</paragraph>' +
+							'<$text a="1">bar</$text>' +
+							'<image a="1"></image>' +
+						'</div>'
+					);
+			} );
+
+			it( 'should filter out disallowed attributes from child nodes (batch)', () => {
+				const root = doc.getRoot();
+				const batch = doc.batch();
+
+				root.appendChildren( [ div ] );
+
+				schema.removeDisallowedAttributes( [ div ], '$root', batch );
+
+				expect( batch.deltas ).to.length( 4 );
+				expect( batch.deltas[ 0 ] ).to.instanceof( AttributeDelta );
+				expect( batch.deltas[ 1 ] ).to.instanceof( AttributeDelta );
+				expect( batch.deltas[ 2 ] ).to.instanceof( AttributeDelta );
+				expect( batch.deltas[ 3 ] ).to.instanceof( AttributeDelta );
+
+				expect( getData( doc, { withoutSelection: true } ) )
+					.to.equal(
+						'<div>' +
+							'<paragraph>' +
+								'<$text b="1">foo</$text>' +
+								'<image b="1"></image>' +
+							'</paragraph>' +
+							'<$text a="1">bar</$text>' +
+							'<image a="1"></image>' +
+						'</div>'
+					);
+			} );
+		} );
+
+		describe( 'allowed parameters', () => {
+			let frag;
+
+			beforeEach( () => {
+				schema.allow( { name: '$text', attributes: [ 'a' ], inside: '$root' } );
+				schema.allow( { name: '$text', attributes: [ 'b' ], inside: 'paragraph' } );
+
+				frag = new DocumentFragment( [
+					new Text( 'foo', { a: 1 } ),
+					new Element( 'paragraph', [], [ new Text( 'bar', { a: 1, b: 1 } ) ] ),
+					new Text( 'biz', { b: 1 } )
+				] );
+			} );
+
+			it( 'should accept iterable as nodes', () => {
+				schema.removeDisallowedAttributes( frag.getChildren(), '$root' );
+
+				expect( stringify( frag ) )
+					.to.equal( '<$text a="1">foo</$text><paragraph><$text b="1">bar</$text></paragraph>biz' );
+			} );
+
+			it( 'should accept Position as inside', () => {
+				schema.removeDisallowedAttributes( frag.getChildren(), Position.createAt( root ) );
+
+				expect( stringify( frag ) )
+					.to.equal( '<$text a="1">foo</$text><paragraph><$text b="1">bar</$text></paragraph>biz' );
+			} );
+
+			it( 'should accept Node as inside', () => {
+				schema.removeDisallowedAttributes( frag.getChildren(), [ root ] );
+
+				expect( stringify( frag ) )
+					.to.equal( '<$text a="1">foo</$text><paragraph><$text b="1">bar</$text></paragraph>biz' );
+			} );
+		} );
+
+		it( 'should not filter out allowed combination of attributes', () => {
+			schema.allow( { name: 'image', attributes: [ 'a', 'b' ] } );
+			schema.requireAttributes( 'image', [ 'a', 'b' ] );
+
+			const image = new Element( 'image', { a: 1, b: 1 } );
+
+			schema.removeDisallowedAttributes( [ image ], '$root' );
+
+			expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'a', 'b' ] );
+		} );
+	} );
 } );