Преглед на файлове

Implemented Schema#addChildCheck.

Piotrek Koszuliński преди 7 години
родител
ревизия
bf7af09cc9

+ 75 - 13
packages/ckeditor5-engine/src/model/schema.js

@@ -112,22 +112,21 @@ import Range from './range';
  * It means that you can add listeners to implement your specific rules which are not limited by the declarative
  * {@link module:engine/model/schema~SchemaItemDefinition API}.
  *
- * The block quote feature defines such a listener to disallow nested `<blockQuote>` structures:
+ * Those listeners can be added either by listening directly to the {@link ~Schema#event:checkChild} event or
+ * by using the handy {@link ~Schema#addChildCheck} method.
  *
- *		schema.on( 'checkChild', ( evt, args ) => {
- *			// The checkChild()'s params.
+ * For instance, the block quote feature defines such a listener to disallow nested `<blockQuote>` structures:
+ *
+ *		schema.addChildCheck( context, childDefinition ) => {
  *			// Note that context is automatically normalized to SchemaContext instance and
- *			// child to its definition (SchemaCompiledItemDefinition) by a highest-priority listener.
- *			const context = args[ 0 ];
- *			const childDefinition = args[ 1 ];
+ *			// child to its definition (SchemaCompiledItemDefinition).
  *
  *			// If checkChild() is called with a context that ends with blockQuote and blockQuote as a child
- *			// to check, make the method return false and stop the event so no other listener will override your decision.
- *			if ( childDefinition && childDefinition.name == 'blockQuote' && context.endsWith( 'blockQuote' ) ) {
- *				evt.stop();
- *				evt.return = false;
+ *			// to check, make the checkChild() method return false.
+ *			if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'blockQuote' ) {
+ *				return false;
  *			}
- *		}, { priority: 'high' } );
+ *		} );
  *
  * ## Defining attributes
  *
@@ -412,6 +411,63 @@ export default class Schema {
 	}
 
 	/**
+	 * Allows registering a callback to the {@link #checkChild} method calls.
+	 *
+	 * Callbacks allow you to implement rules which are not otherwise possible to achieve
+	 * by using the declarative API of {@link module:engine/model/schema~SchemaItemDefinition}.
+	 * For example, by using this method you can disallow elements in specific contexts.
+	 *
+	 * This method is a shorthand for using the {@link #event:checkChild} event. For even better control,
+	 * you can use that event instead.
+	 *
+	 * Example:
+	 *
+	 *		// Disallow heading1 directly inside a blockQuote.
+	 *		schema.addChildCheck( ( ctx, childDefinition ) => {
+	 *			if ( ctx.endsWith( 'blockQuote' ) && childDefinition.name == 'heading1' ) {
+	 *				return false;
+	 *			}
+	 *		} );
+	 *
+	 * Which translates to:
+	 *
+	 *		schema.on( 'checkChild', ( evt, args ) => {
+	 *			const context = args[ 0 ];
+	 *			const childDefinition = args[ 1 ];
+	 *
+	 *			if ( context.endsWith( 'blockQuote' ) && childDefinition && childDefinition.name == 'heading1' ) {
+	 *				// Prevent next listeners from being called.
+	 *				evt.stop();
+	 *				// Set the checkChild()'s return value.
+	 *				evt.return = false;
+	 *			}
+	 *		}, { priority: 'high' } );
+	 *
+	 * @param {Function} callback The callback to be called. It is called with two parameters:
+	 * {@link module:engine/model/schema~SchemaContext} (context) instance and
+	 * {@link module:engine/model/schema~SchemaCompiledItemDefinition} (child-to-check definition).
+	 * The callback may return `true/false` to override `checkChild()`'s return value. If it does not return
+	 * a boolean value, the default algorithm (or other callbacks) will define `checkChild()`'s return value.
+	 */
+	addChildCheck( callback ) {
+		this.on( 'checkChild', ( evt, [ ctx, childDef ] ) => {
+			// checkChild() was called with a non-registered child.
+			// In 99% cases such check should return false, so not to overcomplicate all callbacks
+			// don't even execute them.
+			if ( !childDef ) {
+				return;
+			}
+
+			const retValue = callback( ctx, childDef );
+
+			if ( typeof retValue == 'boolean' ) {
+				evt.stop();
+				evt.return = retValue;
+			}
+		}, { priority: 'high' } );
+	}
+
+	/**
 	 * Returns the lowest {@link module:engine/model/schema~Schema#isLimit limit element} containing the entire
 	 * selection or the root otherwise.
 	 *
@@ -598,7 +654,11 @@ mix( Schema, ObservableMixin );
  * additional behavior – e.g. implementing rules which cannot be defined using the declarative
  * {@link module:engine/model/schema~SchemaItemDefinition} interface.
  *
- * The {@link #checkChild} method fires an event because it's
+ * **Note:** The {@link #addChildCheck} method is a more handy way to register callbacks. Internally,
+ * it registers a listener to this event but comes with a simpler API and it is the recommended choice
+ * in most of the cases.
+ *
+ * The {@link #checkChild} method fires an event because it is
  * {@link module:utils/observablemixin~ObservableMixin#decorate decorated} with it. Thanks to that you can
  * use this event in a various way, but the most important use case is overriding standard behaviour of the
  * `checkChild()` method. Let's see a typical listener template:
@@ -614,13 +674,15 @@ mix( Schema, ObservableMixin );
  * {@link module:engine/model/schema~SchemaCompiledItemDefinition} instance, so you don't have to worry about
  * the various ways how `context` and `child` may be passed to `checkChild()`.
  *
+ * **Note:** `childDefinition` may be `undefined` if `checkChild()` was called with a non-registered element.
+ *
  * So, in order to implement a rule "disallow `heading1` in `blockQuote`" you can add such a listener:
  *
  *		schema.on( 'checkChild', ( evt, args ) => {
  *			const context = args[ 0 ];
  *			const childDefinition = args[ 1 ];
  *
- *			if ( context.endsWith( 'blockQuote' ) && childDefinition.name == 'heading1' ) {
+ *			if ( context.endsWith( 'blockQuote' ) && childDefinition && childDefinition.name == 'heading1' ) {
  *				// Prevent next listeners from being called.
  *				evt.stop();
  *				// Set the checkChild()'s return value.

+ 4 - 8
packages/ckeditor5-engine/tests/conversion/buildviewconverter.js

@@ -495,15 +495,11 @@ describe( 'View converter builder', () => {
 		buildViewConverter().for( dispatcher ).fromElement( 'p' ).toElement( 'paragraph' );
 
 		// Disallow $root>div.
-		schema.on( 'checkChild', ( evt, args ) => {
-			const ctx = args[ 0 ];
-			const childRule = args[ 1 ];
-
-			if ( childRule.name == 'div' && ctx.endsWith( '$root' ) ) {
-				evt.stop();
-				evt.return = false;
+		schema.addChildCheck( ( ctx, childDef ) => {
+			if ( childDef.name == 'div' && ctx.endsWith( '$root' ) ) {
+				return false;
 			}
-		}, { priority: 'high' } );
+		} );
 
 		dispatcher.on( 'element', convertToModelFragment(), { priority: 'lowest' } );
 

+ 4 - 8
packages/ckeditor5-engine/tests/conversion/view-to-model-converters.js

@@ -63,15 +63,11 @@ describe( 'view-to-model-converters', () => {
 		} );
 
 		it( 'should not convert text if it is wrong with schema', () => {
-			schema.on( 'checkChild', ( evt, args ) => {
-				const ctx = args[ 0 ];
-				const childRule = args[ 1 ];
-
-				if ( childRule.name == '$text' && ctx.endsWith( '$root' ) ) {
-					evt.stop();
-					evt.return = false;
+			schema.addChildCheck( ( ctx, childDef ) => {
+				if ( childDef.name == '$text' && ctx.endsWith( '$root' ) ) {
+					return false;
 				}
-			}, { priority: 'high' } );
+			} );
 
 			const viewText = new ViewText( 'foobar' );
 			dispatcher.on( 'text', convertText() );

+ 3 - 6
packages/ckeditor5-engine/tests/manual/tickets/1088/1.js

@@ -68,12 +68,9 @@ ClassicEditor
 			}
 		} );
 
-		schema.on( 'checkChild', ( evt, args ) => {
-			const childRule = args[ 1 ];
-
-			if ( args[ 0 ].endsWith( '$root' ) && childRule.name == 'heading3' ) {
-				evt.stop();
-				evt.return = false;
+		schema.addChildCheck( ( ctx, childDef ) => {
+			if ( ctx.endsWith( '$root' ) && childDef.name == 'heading3' ) {
+				return false;
 			}
 		} );
 	} )

+ 94 - 20
packages/ckeditor5-engine/tests/model/schema.js

@@ -161,7 +161,7 @@ describe( 'Schema', () => {
 			} );
 		} );
 
-		it( 'ensures no unregistered items in allowIn', () => {
+		it( 'ensures no non-registered items in allowIn', () => {
 			schema.register( 'foo', {
 				allowIn: '$root'
 			} );
@@ -256,7 +256,7 @@ describe( 'Schema', () => {
 			expect( schema.getDefinition( ctx.last ).isMe ).to.be.true;
 		} );
 
-		it( 'returns undefined when trying to get an unregistered item', () => {
+		it( 'returns undefined when trying to get an non-registered item', () => {
 			expect( schema.getDefinition( '404' ) ).to.be.undefined;
 		} );
 	} );
@@ -498,6 +498,84 @@ describe( 'Schema', () => {
 		} );
 	} );
 
+	describe( 'addChildCheck()', () => {
+		beforeEach( () => {
+			schema.register( '$root' );
+			schema.register( 'paragraph', {
+				allowIn: '$root'
+			} );
+		} );
+
+		it( 'adds a high-priority listener', () => {
+			const order = [];
+
+			schema.on( 'checkChild', () => {
+				order.push( 'checkChild:high-before' );
+			}, { priority: 'high' } );
+
+			schema.addChildCheck( () => {
+				order.push( 'addChildCheck' );
+			} );
+
+			schema.on( 'checkChild', () => {
+				order.push( 'checkChild:high-after' );
+			}, { priority: 'high' } );
+
+			schema.checkChild( root1, r1p1 );
+
+			expect( order.join() ).to.equal( 'checkChild:high-before,addChildCheck,checkChild:high-after' );
+		} );
+
+		it( 'stops the event and overrides the return value when callback returned true', () => {
+			schema.register( '$text' );
+
+			expect( schema.checkChild( root1, '$text' ) ).to.be.false;
+
+			schema.addChildCheck( () => {
+				return true;
+			} );
+
+			schema.on( 'checkChild', () => {
+				throw new Error( 'the event should be stopped' );
+			}, { priority: 'high' } );
+
+			expect( schema.checkChild( root1, '$text' ) ).to.be.true;
+		} );
+
+		it( 'stops the event and overrides the return value when callback returned false', () => {
+			expect( schema.checkChild( root1, r1p1 ) ).to.be.true;
+
+			schema.addChildCheck( () => {
+				return false;
+			} );
+
+			schema.on( 'checkChild', () => {
+				throw new Error( 'the event should be stopped' );
+			}, { priority: 'high' } );
+
+			expect( schema.checkChild( root1, r1p1 ) ).to.be.false;
+		} );
+
+		it( 'receives context and child definition as params', () => {
+			schema.addChildCheck( ( ctx, childDef ) => {
+				expect( ctx ).to.be.instanceOf( SchemaContext );
+				expect( childDef ).to.equal( schema.getDefinition( 'paragraph' ) );
+			} );
+
+			expect( schema.checkChild( root1, r1p1 ) ).to.be.true;
+		} );
+
+		it( 'is not called when checking a non-registered element', () => {
+			expect( schema.getDefinition( 'foo' ) ).to.be.undefined;
+
+			schema.addChildCheck( () => {
+				throw new Error( 'callback should not be called' );
+			} );
+
+			expect( schema.checkChild( root1, 'foo' ) ).to.be.false;
+		} );
+	} );
+
 	describe( 'getLimitElement()', () => {
 		let model, doc, root;
 
@@ -1001,7 +1079,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( div, div ) ).to.be.true;
 			} );
 
-			it( 'rejects $root>paragraph – unregistered paragraph', () => {
+			it( 'rejects $root>paragraph – non-registered paragraph', () => {
 				schema.register( '$root' );
 
 				expect( schema.checkChild( root1, r1p1 ) ).to.be.false;
@@ -1430,7 +1508,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( root1, 'foo404' ) ).to.be.false;
 			} );
 
-			it( 'does not break when trying to check registered child in a context which contains unregistered elements', () => {
+			it( 'does not break when trying to check registered child in a context which contains non-registered elements', () => {
 				const foo404 = new Element( 'foo404' );
 
 				root1.appendChildren( foo404 );
@@ -1443,7 +1521,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( foo404, '$text' ) ).to.be.false;
 			} );
 
-			it( 'does not break when used allowedIn pointing to an unregistered element', () => {
+			it( 'does not break when used allowedIn pointing to an non-registered element', () => {
 				schema.register( '$root' );
 				schema.register( '$text', {
 					allowIn: 'foo404'
@@ -1452,7 +1530,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( root1, '$text' ) ).to.be.false;
 			} );
 
-			it( 'does not break when used allowWhere pointing to an unregistered element', () => {
+			it( 'does not break when used allowWhere pointing to an non-registered element', () => {
 				schema.register( '$root' );
 				schema.register( '$text', {
 					allowWhere: 'foo404'
@@ -1461,7 +1539,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( root1, '$text' ) ).to.be.false;
 			} );
 
-			it( 'does not break when used allowContentOf pointing to an unregistered element', () => {
+			it( 'does not break when used allowContentOf pointing to an non-registered element', () => {
 				schema.register( '$root', {
 					allowContentOf: 'foo404'
 				} );
@@ -1481,7 +1559,7 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( root1, 'paragraph' ) ).to.be.false;
 			} );
 
-			it( 'does not break when inheriting all from an unregistered element', () => {
+			it( 'does not break when inheriting all from an non-registered element', () => {
 				schema.register( 'paragraph', {
 					inheritAllFrom: '$block'
 				} );
@@ -1591,11 +1669,11 @@ describe( 'Schema', () => {
 		} );
 
 		describe( 'missing attribute definitions', () => {
-			it( 'does not crash when checking an attribute of a unregistered element', () => {
+			it( 'does not crash when checking an attribute of a non-registered element', () => {
 				expect( schema.checkAttribute( r1p1, 'align' ) ).to.be.false;
 			} );
 
-			it( 'does not crash when inheriting attributes of a unregistered element', () => {
+			it( 'does not crash when inheriting attributes of a non-registered element', () => {
 				schema.register( 'paragraph', {
 					allowAttributesOf: '$block'
 				} );
@@ -1603,7 +1681,7 @@ describe( 'Schema', () => {
 				expect( schema.checkAttribute( r1p1, 'whatever' ) ).to.be.false;
 			} );
 
-			it( 'does not crash when inheriting all from a unregistered element', () => {
+			it( 'does not crash when inheriting all from a non-registered element', () => {
 				schema.register( 'paragraph', {
 					allowAttributesOf: '$block'
 				} );
@@ -1613,7 +1691,7 @@ describe( 'Schema', () => {
 		} );
 
 		describe( 'missing types definitions', () => {
-			it( 'does not crash when inheriting types of an unregistered element', () => {
+			it( 'does not crash when inheriting types of an non-registered element', () => {
 				schema.register( 'paragraph', {
 					inheritTypesFrom: '$block'
 				} );
@@ -1650,15 +1728,11 @@ describe( 'Schema', () => {
 				} );
 
 				// Disallow blockQuote in blockQuote.
-				schema.on( 'checkChild', ( evt, args ) => {
-					const ctx = args[ 0 ];
-					const childRule = args[ 1 ];
-
-					if ( childRule.name == 'blockQuote' && ctx.endsWith( 'blockQuote' ) ) {
-						evt.stop();
-						evt.return = false;
+				schema.addChildCheck( ( ctx, childDef ) => {
+					if ( childDef.name == 'blockQuote' && ctx.endsWith( 'blockQuote' ) ) {
+						return false;
 					}
-				}, { priority: 'high' } );
+				} );
 			},
 			() => {
 				schema.register( 'image', {

+ 3 - 7
packages/ckeditor5-engine/tests/model/utils/insertcontent.js

@@ -273,13 +273,9 @@ describe( 'DataController utils', () => {
 
 			it( 'not insert autoparagraph when paragraph is disallowed at the current position', () => {
 				// Disallow paragraph in $root.
-				model.schema.on( 'checkChild', ( evt, args ) => {
-					const ctx = args[ 0 ];
-					const childRule = args[ 1 ];
-
-					if ( childRule.name == 'paragraph' && ctx.endsWith( '$root' ) ) {
-						evt.stop();
-						evt.return = false;
+				model.schema.addChildCheck( ( ctx, childDef ) => {
+					if ( childDef.name == 'paragraph' && ctx.endsWith( '$root' ) ) {
+						return false;
 					}
 				} );