8
0
Просмотр исходного кода

Fixed removeDisallowedAttributes() usage in insertContent() and deleteContent().

Piotrek Koszuliński 8 лет назад
Родитель
Сommit
3979c41195

+ 18 - 2
packages/ckeditor5-engine/src/model/schema.js

@@ -163,8 +163,24 @@ export default class Schema {
 		return element;
 		return element;
 	}
 	}
 
 
-	removeDisallowedAttributes() {
-		// TODO
+	/**
+	 * Removes attributes disallowed the schema.
+	 *
+	 * @param {Iterable.<module:engine/model/node~Node>} nodes Nodes that will be filtered.
+	 * @param {module:engine/model/writer~Writer} writer
+	 */
+	removeDisallowedAttributes( nodes, writer ) {
+		for ( const node of nodes ) {
+			for ( const attribute of node.getAttributeKeys() ) {
+				if ( !this.checkAttribute( node, attribute ) ) {
+					writer.removeAttribute( attribute, node );
+				}
+			}
+
+			if ( node.is( 'element' ) ) {
+				this.removeDisallowedAttributes( node.getChildren(), writer );
+			}
+		}
 	}
 	}
 
 
 	_clearCache() {
 	_clearCache() {

+ 2 - 1
packages/ckeditor5-engine/src/model/utils/deletecontent.js

@@ -73,12 +73,13 @@ export default function deleteContent( model, selection, options = {} ) {
 		if ( !options.leaveUnmerged ) {
 		if ( !options.leaveUnmerged ) {
 			mergeBranches( writer, startPos, endPos );
 			mergeBranches( writer, startPos, endPos );
 
 
+			// TMP this will be replaced with a postifxer.
 			// We need to check and strip disallowed attributes in all nested nodes because after merge
 			// 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.
 			// some attributes could end up in a path where are disallowed.
 			//
 			//
 			// e.g. bold is disallowed for <H1>
 			// 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>.
 			// <h1>Fo{o</h1><p>b}a<b>r</b><p> -> <h1>Fo{}a<b>r</b><h1> -> <h1>Fo{}ar<h1>.
-			schema.removeDisallowedAttributes( startPos.parent.getChildren(), startPos, writer );
+			schema.removeDisallowedAttributes( startPos.parent.getChildren(), writer );
 		}
 		}
 
 
 		selection.setCollapsedAt( startPos );
 		selection.setCollapsedAt( startPos );

+ 18 - 33
packages/ckeditor5-engine/src/model/utils/insertcontent.js

@@ -116,6 +116,8 @@ class Insertion {
 		 * @member {module:engine/model/schema~Schema} #schema
 		 * @member {module:engine/model/schema~Schema} #schema
 		 */
 		 */
 		this.schema = model.schema;
 		this.schema = model.schema;
+
+		this._filterAttributesOf = [];
 	}
 	}
 
 
 	/**
 	/**
@@ -136,6 +138,10 @@ class Insertion {
 				isLast: ( i === ( nodes.length - 1 ) ) && parentContext.isLast
 				isLast: ( i === ( nodes.length - 1 ) ) && parentContext.isLast
 			} );
 			} );
 		}
 		}
+
+		// TMP this will become a postfixer.
+		this.schema.removeDisallowedAttributes( this._filterAttributesOf, this.writer );
+		this._filterAttributesOf = [];
 	}
 	}
 
 
 	/**
 	/**
@@ -222,13 +228,7 @@ class Insertion {
 		if ( node.is( 'element' ) ) {
 		if ( node.is( 'element' ) ) {
 			this.handleNodes( node.getChildren(), context );
 			this.handleNodes( node.getChildren(), context );
 		}
 		}
-		// 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.checkChild( this.position, '$text' ) ) {
-			this.schema.removeDisallowedAttributes( [ node ], this.position, this.writer );
-			this._handleNode( node, context );
-		}
-		// If text is not allowed, try autoparagraphing.
+		// If text is not allowed, try autoparagraphing it.
 		else {
 		else {
 			this._tryAutoparagraphing( node, context );
 			this._tryAutoparagraphing( node, context );
 		}
 		}
@@ -263,6 +263,8 @@ class Insertion {
 		} else {
 		} else {
 			this.nodeToSelect = null;
 			this.nodeToSelect = null;
 		}
 		}
+
+		this._filterAttributesOf.push( node );
 	}
 	}
 
 
 	/**
 	/**
@@ -284,11 +286,6 @@ class Insertion {
 
 
 			this.writer.merge( mergePosLeft );
 			this.writer.merge( mergePosLeft );
 
 
-			// 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;
-			this.schema.removeDisallowedAttributes( parent.getChildren(), Position.createAt( parent ), this.writer );
-
 			this.position = Position.createFromPosition( position );
 			this.position = Position.createFromPosition( position );
 			position.detach();
 			position.detach();
 		}
 		}
@@ -312,22 +309,18 @@ class Insertion {
 
 
 			this.writer.merge( mergePosRight );
 			this.writer.merge( mergePosRight );
 
 
-			// 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.
-			this.schema.removeDisallowedAttributes( position.parent.getChildren(), position, this.writer );
-
 			this.position = Position.createFromPosition( position );
 			this.position = Position.createFromPosition( position );
 			position.detach();
 			position.detach();
 		}
 		}
 
 
+		if ( mergeLeft || mergeRight ) {
+			// After merge elements that were marked by _insert() to be filtered might be gone so
+			// we need to mark the new container.
+			this._filterAttributesOf.push( this.position.parent );
+		}
+
 		mergePosLeft.detach();
 		mergePosLeft.detach();
 		mergePosRight.detach();
 		mergePosRight.detach();
-
-		// 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 ) {
-			this.schema.removeDisallowedAttributes( node.getChildren(), Position.createAt( node ), this.writer );
-		}
 	}
 	}
 
 
 	/**
 	/**
@@ -342,17 +335,9 @@ class Insertion {
 		// Do not autoparagraph if the paragraph won't be allowed there,
 		// Do not autoparagraph if the paragraph won't be allowed there,
 		// cause that would lead to an infinite loop. The paragraph would be rejected in
 		// cause that would lead to an infinite loop. The paragraph would be rejected in
 		// the next _handleNode() call and we'd be here again.
 		// the next _handleNode() call and we'd be here again.
-		if ( this._getAllowedIn( paragraph, this.position.parent ) ) {
-			// 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.schema.checkChild( paragraph, node ) ) {
-				this.schema.removeDisallowedAttributes( [ node ], [ paragraph ], this.writer );
-			}
-
-			if ( this.schema.checkChild( paragraph, node ) ) {
-				paragraph.appendChildren( node );
-				this._handleNode( paragraph, context );
-			}
+		if ( this._getAllowedIn( paragraph, this.position.parent ) && this.schema.checkChild( paragraph, node ) ) {
+			paragraph.appendChildren( node );
+			this._handleNode( paragraph, context );
 		}
 		}
 	}
 	}
 
 

+ 1 - 1
packages/ckeditor5-engine/tests/conversion/advanced-converters.js

@@ -42,7 +42,7 @@ describe( 'advanced-converters', () => {
 
 
 		viewRoot = editing.createRoot( 'div' );
 		viewRoot = editing.createRoot( 'div' );
 
 
-		viewDispatcher = new ViewConversionDispatcher( model, { schema: { check: () => true } } );
+		viewDispatcher = new ViewConversionDispatcher( model, { schema: { checkChild: () => true } } );
 		viewDispatcher.on( 'text', convertText() );
 		viewDispatcher.on( 'text', convertText() );
 		viewDispatcher.on( 'documentFragment', convertToModelFragment() );
 		viewDispatcher.on( 'documentFragment', convertToModelFragment() );
 
 

+ 121 - 4
packages/ckeditor5-engine/tests/model/schema.js

@@ -12,8 +12,11 @@ import Model from '../../src/model/model';
 import Element from '../../src/model/element';
 import Element from '../../src/model/element';
 import Position from '../../src/model/position';
 import Position from '../../src/model/position';
 import Text from '../../src/model/text';
 import Text from '../../src/model/text';
+import DocumentFragment from '../../src/model/documentfragment';
 
 
-import { setData } from '../../src/dev-utils/model';
+import { setData, getData, stringify } from '../../src/dev-utils/model';
+
+import AttributeDelta from '../../src/model/delta/attributedelta';
 
 
 describe( 'Schema', () => {
 describe( 'Schema', () => {
 	let schema, root1, r1p1, r1p2, r1bQ, r1bQp, root2;
 	let schema, root1, r1p1, r1p2, r1bQ, r1bQp, root2;
@@ -481,6 +484,119 @@ describe( 'Schema', () => {
 		} );
 		} );
 	} );
 	} );
 
 
+	describe( 'removeDisallowedAttributes()', () => {
+		let model, doc, root;
+
+		beforeEach( () => {
+			model = new Model();
+			doc = model.document;
+			root = doc.createRoot();
+			schema = model.schema;
+
+			schema.register( 'paragraph', {
+				inheritAllFrom: '$block'
+			} );
+			schema.register( 'div', {
+				inheritAllFrom: '$block'
+			} );
+			schema.register( 'image', {
+				isObject: true
+			} );
+			schema.extend( '$block', {
+				allowIn: 'div'
+			} );
+		} );
+
+		it( 'should filter out disallowed attributes from given nodes', () => {
+			schema.extend( '$text', { allowAttributes: 'a' } );
+			schema.extend( 'image', { allowAttributes: 'b' } );
+
+			const text = new Text( 'foo', { a: 1, b: 1 } );
+			const image = new Element( 'image', { a: 1, b: 1 } );
+
+			root.appendChildren( [ text, image ] );
+
+			model.change( writer => {
+				schema.removeDisallowedAttributes( root.getChildren(), writer );
+
+				expect( Array.from( text.getAttributeKeys() ) ).to.deep.equal( [ 'a' ] );
+				expect( Array.from( image.getAttributeKeys() ) ).to.deep.equal( [ 'b' ] );
+
+				expect( writer.batch.deltas ).to.length( 2 );
+				expect( writer.batch.deltas[ 0 ] ).to.instanceof( AttributeDelta );
+				expect( writer.batch.deltas[ 1 ] ).to.instanceof( AttributeDelta );
+
+				expect( getData( model, { withoutSelection: true } ) )
+					.to.equal( '<$text a="1">foo</$text><image b="1"></image>' );
+			} );
+		} );
+
+		it( 'should filter out disallowed attributes from all descendants of given nodes', () => {
+			schema.on( 'checkAttribute', ( evt, args ) => {
+				const ctx = args[ 0 ];
+				const ctxItem = ctx[ ctx.length - 1 ];
+				const ctxParent = ctx[ ctx.length - 2 ];
+				const ctxParent2 = ctx[ ctx.length - 3 ];
+				const attributeName = args[ 1 ];
+
+				// 'a' in div>$text
+				if ( ctxItem.name == '$text' && ctxParent.name == 'div' && attributeName == 'a' ) {
+					evt.stop();
+					evt.return = true;
+				}
+
+				// 'b' in div>paragraph>$text
+				if ( ctxItem.name == '$text' && ctxParent.name == 'paragraph' && ctxParent2.name == 'div' && attributeName == 'b' ) {
+					evt.stop();
+					evt.return = true;
+				}
+
+				// 'a' in div>image
+				if ( ctxItem.name == 'image' && ctxParent.name == 'div' && attributeName == 'a' ) {
+					evt.stop();
+					evt.return = true;
+				}
+
+				// 'b' in div>paragraph>image
+				if ( ctxItem.name == 'image' && ctxParent.name == 'paragraph' && ctxParent2.name == 'div' && attributeName == 'b' ) {
+					evt.stop();
+					evt.return = true;
+				}
+			}, { priority: 'high' } );
+
+			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 div = new Element( 'div', [], [ paragraph, bar, imageInDiv ] );
+
+			root.appendChildren( [ div ] );
+
+			model.change( writer => {
+				schema.removeDisallowedAttributes( root.getChildren(), writer );
+
+				expect( writer.batch.deltas ).to.length( 4 );
+				expect( writer.batch.deltas[ 0 ] ).to.instanceof( AttributeDelta );
+				expect( writer.batch.deltas[ 1 ] ).to.instanceof( AttributeDelta );
+				expect( writer.batch.deltas[ 2 ] ).to.instanceof( AttributeDelta );
+				expect( writer.batch.deltas[ 3 ] ).to.instanceof( AttributeDelta );
+
+				expect( getData( model, { 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( 'rules compilation', () => {
 	describe( 'rules compilation', () => {
 		describe( 'allowIn cases', () => {
 		describe( 'allowIn cases', () => {
 			it( 'passes $root>paragraph', () => {
 			it( 'passes $root>paragraph', () => {
@@ -1160,11 +1276,11 @@ describe( 'Schema', () => {
 				// Disallow bold in heading1.
 				// Disallow bold in heading1.
 				schema.on( 'checkAttribute', ( evt, args ) => {
 				schema.on( 'checkAttribute', ( evt, args ) => {
 					const context = args[ 0 ];
 					const context = args[ 0 ];
-					const contextLast = context[ context.length - 1 ];
-					const contextSecondToLast = context[ context.length - 2 ];
+					const ctxItem = context[ context.length - 1 ];
+					const ctxParent = context[ context.length - 2 ];
 					const attributeName = args[ 1 ];
 					const attributeName = args[ 1 ];
 
 
-					if ( contextLast.name == '$text' && contextSecondToLast.name == 'heading1' && attributeName == 'bold' ) {
+					if ( ctxItem.name == '$text' && ctxParent.name == 'heading1' && attributeName == 'bold' ) {
 						evt.stop();
 						evt.stop();
 						evt.return = false;
 						evt.return = false;
 					}
 					}
@@ -1443,4 +1559,5 @@ describe( 'Schema', () => {
 	// * it doesn't make sense for VCD to get schema as a param (it can get it from the model)
 	// * it doesn't make sense for VCD to get schema as a param (it can get it from the model)
 	// * V->M conversion tests might got outdated and would need to be reviewed if someone has a spare week ;)
 	// * V->M conversion tests might got outdated and would need to be reviewed if someone has a spare week ;)
 	// * Do we need both $inline and $text?
 	// * Do we need both $inline and $text?
+	// * Consider reversing context array for writing simpler callbacks
 } );
 } );

+ 38 - 4
packages/ckeditor5-engine/tests/model/utils/deletecontent.js

@@ -466,10 +466,44 @@ describe( 'DataController utils', () => {
 				beforeEach( () => {
 				beforeEach( () => {
 					const schema = model.schema;
 					const schema = model.schema;
 
 
-					// TODO this requires a callback
-					// schema.xallow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'paragraph' } );
-					// schema.xallow( { name: '$text', attributes: [ 'b', 'c' ], inside: 'pchild' } );
-					// schema.xdisallow( { name: '$text', attributes: [ 'c' ], inside: 'pchild pchild' } );
+					schema.on( 'checkAttribute', ( evt, args ) => {
+						const ctx = args[ 0 ];
+						const ctxItem = ctx[ ctx.length - 1 ];
+						const ctxParent = ctx[ ctx.length - 2 ];
+						const ctxParent2 = ctx[ ctx.length - 3 ];
+						const attributeName = args[ 1 ];
+
+						// allow 'a' and 'b' in paragraph>$text
+						if (
+							ctxItem.name == '$text' &&
+							ctxParent.name == 'paragraph' &&
+							[ 'a', 'b' ].includes( attributeName )
+						) {
+							evt.stop();
+							evt.return = true;
+						}
+
+						// allow 'b' and 'c' in pchild>$text
+						if (
+							ctxItem.name == '$text' &&
+							ctxParent.name == 'pchild' &&
+							[ 'b', 'c' ].includes( attributeName )
+						) {
+							evt.stop();
+							evt.return = true;
+						}
+
+						// disallow 'c' in pchild>pchild>$text
+						if (
+							ctxItem.name == '$text' &&
+							ctxParent.name == 'pchild' &&
+							ctxParent2.name == 'pchild' &&
+							attributeName == 'c'
+						) {
+							evt.stop();
+							evt.return = false;
+						}
+					}, { priority: 'high' } );
 
 
 					schema.extend( 'pchild', { allowIn: 'pchild' } );
 					schema.extend( 'pchild', { allowIn: 'pchild' } );
 				} );
 				} );

+ 59 - 8
packages/ckeditor5-engine/tests/model/utils/insertcontent.js

@@ -272,8 +272,17 @@ describe( 'DataController utils', () => {
 			} );
 			} );
 
 
 			it( 'not insert autoparagraph when paragraph is disallowed at the current position', () => {
 			it( 'not insert autoparagraph when paragraph is disallowed at the current position', () => {
-				// TODO this requires a callback
-				// model.schema.xdisallow( { name: 'paragraph', inside: '$root' } );
+				// Disallow paragraph in $root.
+				model.schema.on( 'checkChild', ( evt, args ) => {
+					const context = args[ 0 ];
+					const child = args[ 1 ];
+					const childRule = model.schema.getRule( child );
+
+					if ( childRule.name == 'paragraph' && context[ context.length - 1 ].name == '$root' ) {
+						evt.stop();
+						evt.return = false;
+					}
+				} );
 
 
 				const content = new DocumentFragment( [
 				const content = new DocumentFragment( [
 					new Element( 'heading1', [], [ new Text( 'bar' ) ] ),
 					new Element( 'heading1', [], [ new Text( 'bar' ) ] ),
@@ -609,12 +618,54 @@ describe( 'DataController utils', () => {
 				schema.extend( 'element', { allowIn: 'paragraph' } );
 				schema.extend( 'element', { allowIn: 'paragraph' } );
 				schema.extend( 'element', { allowIn: 'heading1' } );
 				schema.extend( 'element', { allowIn: 'heading1' } );
 
 
-				// TODO this requires a callback
-				// schema.xallow( { name: '$text', attributes: 'b', inside: 'paragraph' } );
-				// schema.xallow( { name: '$text', attributes: [ 'b' ], inside: 'paragraph element' } );
-				// schema.xallow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'heading1 element' } );
-				// schema.xallow( { name: '$text', attributes: [ 'a', 'b' ], inside: 'td element' } );
-				// schema.xallow( { name: '$text', attributes: [ 'b' ], inside: 'element table td' } );
+				schema.on( 'checkAttribute', ( evt, args ) => {
+					const ctx = args[ 0 ];
+					const ctxItem = ctx[ ctx.length - 1 ];
+					const ctxParent = ctx[ ctx.length - 2 ];
+					const ctxParent2 = ctx[ ctx.length - 3 ];
+					const ctxParent3 = ctx[ ctx.length - 4 ];
+					const attributeName = args[ 1 ];
+
+					// 'b' in paragraph>$text
+					if (
+						ctxItem.name == '$text' &&
+						ctxParent.name == 'paragraph' &&
+						attributeName == 'b'
+					) {
+						evt.stop();
+						evt.return = true;
+					}
+
+					// 'b' in paragraph>element>$text
+					if (
+						ctxItem.name == '$text' &&
+						ctxParent.name == 'element' && ctxParent2.name == 'paragraph' &&
+						attributeName == 'b'
+					) {
+						evt.stop();
+						evt.return = true;
+					}
+
+					// 'a' and 'b' in heading1>element>$text
+					if (
+						ctxItem.name == '$text' &&
+						ctxParent.name == 'element' && ctxParent2 == 'heading1' &&
+						[ 'a', 'b' ].includes( attributeName )
+					) {
+						evt.stop();
+						evt.return = true;
+					}
+
+					// 'b' in element>table>td>$text
+					if (
+						ctxItem.name == '$text' &&
+						ctxParent.name == 'td' && ctxParent2.name == 'table' && ctxParent3.name == 'element' &&
+						attributeName == 'b'
+					) {
+						evt.stop();
+						evt.return = true;
+					}
+				}, { priority: 'high' } );
 			} );
 			} );
 
 
 			it( 'filters out disallowed elements and leaves out the text', () => {
 			it( 'filters out disallowed elements and leaves out the text', () => {