浏览代码

Added some missing tests and made small code simplications.

Piotrek Koszuliński 8 年之前
父节点
当前提交
20e0c94b74

+ 1 - 1
packages/ckeditor5-engine/src/model/document.js

@@ -266,7 +266,7 @@ export default class Document {
 			const type = ( data.walker == backwardWalker ? 'elementEnd' : 'elementStart' );
 			const value = data.value;
 
-			if ( value.type == type && schema.isObject( value.item.name ) ) {
+			if ( value.type == type && schema.isObject( value.item ) ) {
 				return Range.createOn( value.item );
 			}
 

+ 1 - 1
packages/ckeditor5-engine/src/model/documentselection.js

@@ -621,7 +621,7 @@ export default class DocumentSelection extends Selection {
 			// ...look for a first character node in that range and take attributes from it.
 			for ( const value of range ) {
 				// If the item is an object, we don't want to get attributes from its children.
-				if ( value.item.is( 'element' ) && schema.isObject( value.item.name ) ) {
+				if ( value.item.is( 'element' ) && schema.isObject( value.item ) ) {
 					break;
 				}
 

+ 3 - 7
packages/ckeditor5-engine/src/model/model.js

@@ -80,13 +80,9 @@ export default class Model {
 		this.schema.register( '$text', {
 			allowIn: '$block'
 		} );
-
-		// TODO review
-		// Create an "all allowed" context in the schema for processing the pasted content.
-		// Read: https://github.com/ckeditor/ckeditor5-engine/issues/638#issuecomment-255086588
-
 		this.schema.register( '$clipboardHolder', {
-			allowContentOf: '$root'
+			allowContentOf: '$root',
+			isLimit: true
 		} );
 		this.schema.extend( '$text', { allowIn: '$clipboardHolder' } );
 	}
@@ -317,7 +313,7 @@ export default class Model {
 
 		for ( const item of rangeOrElement.getItems() ) {
 			// Remember, `TreeWalker` returns always `textProxy` nodes.
-			if ( item.is( 'textProxy' ) || this.schema.isObject( item.name ) ) {
+			if ( item.is( 'textProxy' ) || this.schema.isObject( item ) ) {
 				return true;
 			}
 		}

+ 42 - 21
packages/ckeditor5-engine/src/model/schema.js

@@ -3,6 +3,10 @@
  * For licensing, see LICENSE.md.
  */
 
+/**
+ * @module engine/model/schema
+ */
+
 import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 import ObservableMixin from '@ckeditor/ckeditor5-utils/src/observablemixin';
 import mix from '@ckeditor/ckeditor5-utils/src/mix';
@@ -10,17 +14,13 @@ import mix from '@ckeditor/ckeditor5-utils/src/mix';
 import Range from './range';
 
 /**
- * @module engine/model/schema
- */
-
-/**
  * @mixes module:utils/emittermixin~ObservableMixin
  */
 export default class Schema {
 	constructor() {
 		this._sourceRules = {};
 
-		// TODO events
+		// TODO docs
 		this.decorate( 'checkChild' );
 		this.decorate( 'checkAttribute' );
 
@@ -36,7 +36,9 @@ export default class Schema {
 	register( itemName, rules ) {
 		if ( this._sourceRules[ itemName ] ) {
 			// TODO docs
-			throw new CKEditorError( 'schema-cannot-register-item-twice: A single item cannot be registered twice in the schema.' );
+			throw new CKEditorError( 'schema-cannot-register-item-twice: A single item cannot be registered twice in the schema.', {
+				itemName
+			} );
 		}
 
 		this._sourceRules[ itemName ] = [
@@ -47,11 +49,11 @@ export default class Schema {
 	}
 
 	extend( itemName, rules ) {
-		// TODO it should not throw if we want to allow e.g. adding attrs before element is registered
-		// (which may be done by another feature).
 		if ( !this._sourceRules[ itemName ] ) {
 			// TODO docs
-			throw new CKEditorError( 'schema-cannot-extend-missing-item: Cannot extend an item which was not registered yet.' );
+			throw new CKEditorError( 'schema-cannot-extend-missing-item: Cannot extend an item which was not registered yet.', {
+				itemName
+			} );
 		}
 
 		this._sourceRules[ itemName ].push( Object.assign( {}, rules ) );
@@ -86,30 +88,42 @@ export default class Schema {
 		return this.getRules()[ itemName ];
 	}
 
-	isRegistered( itemName ) {
-		return !!this.getRule( itemName );
+	/**
+	 * @param {module:engine/model/item~Item|SchemaContextItem|String} item
+	 */
+	isRegistered( item ) {
+		return !!this.getRule( item );
 	}
 
-	isBlock( itemName ) {
-		const rule = this.getRule( itemName );
+	/**
+	 * @param {module:engine/model/item~Item|SchemaContextItem|String} item
+	 */
+	isBlock( item ) {
+		const rule = this.getRule( item );
 
 		return !!( rule && rule.isBlock );
 	}
 
-	isLimit( itemName ) {
-		const rule = this.getRule( itemName );
+	/**
+	 * @param {module:engine/model/item~Item|SchemaContextItem|String} item
+	 */
+	isLimit( item ) {
+		const rule = this.getRule( item );
 
 		return !!( rule && rule.isLimit );
 	}
 
-	isObject( itemName ) {
-		const rule = this.getRule( itemName );
+	/**
+	 * @param {module:engine/model/item~Item|SchemaContextItem|String} item
+	 */
+	isObject( item ) {
+		const rule = this.getRule( item );
 
 		return !!( rule && rule.isObject );
 	}
 
 	/**
-	 * @param {module:engine/model/element~Element|module:engine/model/position~Position|Array.<String>} context
+	 * @param {SchemaContextDefinition} context
 	 * @param {module:engine/model/node~Node|String}
 	 */
 	checkChild( context, child ) {
@@ -123,7 +137,7 @@ export default class Schema {
 	}
 
 	/**
-	 * @param {module:engine/model/node~Node} context
+	 * @param {SchemaContextDefinition} context
 	 * @param {String}
 	 */
 	checkAttribute( context, attributeName ) {
@@ -154,7 +168,7 @@ export default class Schema {
 				return node.getCommonAncestor( range.getCommonAncestor() );
 			}, null );
 
-		while ( !this.isLimit( element.name ) ) {
+		while ( !this.isLimit( element ) ) {
 			if ( element.parent ) {
 				element = element.parent;
 			} else {
@@ -329,7 +343,7 @@ export class SchemaContext {
 	}
 
 	/**
-	 * Returns an iterator that iterates over all context items
+	 * Returns an iterator that iterates over all context items.
 	 *
 	 * @returns {Iterator.<TODO>}
 	 */
@@ -350,6 +364,13 @@ export class SchemaContext {
 	}
 }
 
+/**
+ * TODO
+ *
+ * @typedef {module:engine/model/node~Node|module:engine/model/position~Position|
+ * Array.<String|module:engine/model/node~Node>} SchemaContextDefinition
+ */
+
 function compileBaseItemRule( sourceItemRules, itemName ) {
 	const itemRule = {
 		name: itemName,

+ 1 - 1
packages/ckeditor5-engine/src/model/selection.js

@@ -785,7 +785,7 @@ function isUnvisitedBlockContainer( element, visited ) {
 
 	visited.add( element );
 
-	return element.document.model.schema.isBlock( element.name ) && element.parent;
+	return element.document.model.schema.isBlock( element ) && element.parent;
 }
 
 // Finds the lowest element in position's ancestors which is a block.

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

@@ -174,7 +174,7 @@ function checkCanBeMerged( leftPos, rightPos, schema ) {
 	const rangeToCheck = new Range( leftPos, rightPos );
 
 	for ( const value of rangeToCheck.getWalker() ) {
-		if ( schema.isObject( value.item.name ) || schema.isLimit( value.item.name ) ) {
+		if ( schema.isObject( value.item ) || schema.isLimit( value.item ) ) {
 			return false;
 		}
 	}

+ 3 - 12
packages/ckeditor5-engine/src/model/utils/insertcontent.js

@@ -170,7 +170,7 @@ class Insertion {
 		// Let's handle object in a special way.
 		// * They should never be merged with other elements.
 		// * If they are not allowed in any of the selection ancestors, they could be either autoparagraphed or totally removed.
-		if ( this._checkIsObject( node ) ) {
+		if ( this.schema.isObject( node ) ) {
 			this._handleObject( node, context );
 
 			return;
@@ -258,7 +258,7 @@ class Insertion {
 		livePos.detach();
 
 		// The last inserted object should be selected because we can't put a collapsed selection after it.
-		if ( this._checkIsObject( node ) && !this.schema.checkChild( this.position, '$text' ) ) {
+		if ( this.schema.isObject( node ) && !this.schema.checkChild( this.position, '$text' ) ) {
 			this.nodeToSelect = node;
 		} else {
 			this.nodeToSelect = null;
@@ -355,7 +355,7 @@ class Insertion {
 
 		while ( allowedIn != this.position.parent ) {
 			// If a parent which we'd need to leave is a limit element, break.
-			if ( this.schema.isLimit( this.position.parent.name ) ) {
+			if ( this.schema.isLimit( this.position.parent ) ) {
 				return false;
 			}
 
@@ -402,13 +402,4 @@ class Insertion {
 
 		return null;
 	}
-
-	/**
-	 * Checks whether according to the schema this is an object type element.
-	 *
-	 * @param {module:engine/model/node~Node} node The node to check.
-	 */
-	_checkIsObject( node ) {
-		return this.schema.isObject( node.is( 'text' ) ? '$text' : node.name );
-	}
 }

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

@@ -82,7 +82,7 @@ function tryExtendingTo( data, value ) {
 	// Entering an element.
 	if ( value.type == ( data.isForward ? 'elementStart' : 'elementEnd' ) ) {
 		// If it's an object, we can select it now.
-		if ( data.schema.isObject( value.item.name ) ) {
+		if ( data.schema.isObject( value.item ) ) {
 			return Position.createAt( value.item, data.isForward ? 'after' : 'before' );
 		}
 
@@ -94,7 +94,7 @@ function tryExtendingTo( data, value ) {
 	// Leaving an element.
 	else {
 		// If leaving a limit element, stop.
-		if ( data.schema.isLimit( value.item.name ) ) {
+		if ( data.schema.isLimit( value.item ) ) {
 			// NOTE: Fast-forward the walker until the end.
 			data.walker.skip( () => true );
 

+ 28 - 3
packages/ckeditor5-engine/tests/model/model.js

@@ -27,7 +27,32 @@ describe( 'Model', () => {
 		changes = '';
 	} );
 
-	describe( 'change & enqueueChange', () => {
+	describe( 'constructor()', () => {
+		it( 'registers $root to the schema', () => {
+			expect( schema.isRegistered( '$root' ) ).to.be.true;
+			expect( schema.isLimit( '$root' ) ).to.be.true;
+		} );
+
+		it( 'registers $block to the schema', () => {
+			expect( schema.isRegistered( '$block' ) ).to.be.true;
+			expect( schema.isBlock( '$block' ) ).to.be.true;
+			expect( schema.checkChild( [ '$root' ], '$block' ) ).to.be.true;
+		} );
+
+		it( 'registers $text to the schema', () => {
+			expect( schema.isRegistered( '$text' ) ).to.be.true;
+			expect( schema.checkChild( [ '$block' ], '$text' ) ).to.be.true;
+		} );
+
+		it( 'registers $clipboardHolder to the schema', () => {
+			expect( schema.isRegistered( '$clipboardHolder' ) ).to.be.true;
+			expect( schema.isLimit( '$clipboardHolder' ) ).to.be.true;
+			expect( schema.checkChild( [ '$clipboardHolder' ], '$text' ) ).to.be.true;
+			expect( schema.checkChild( [ '$clipboardHolder' ], '$block' ) ).to.be.true;
+		} );
+	} );
+
+	describe( 'change() & enqueueChange()', () => {
 		it( 'should execute changes immediately', () => {
 			model.change( () => {
 				changes += 'A';
@@ -318,7 +343,7 @@ describe( 'Model', () => {
 		} );
 	} );
 
-	describe( 'applyOperation', () => {
+	describe( 'applyOperation()', () => {
 		it( 'should execute provided operation end return the result of operation', () => {
 			const returnValue = { foo: 'bar' };
 
@@ -334,7 +359,7 @@ describe( 'Model', () => {
 		} );
 	} );
 
-	describe( 'transformDeltas', () => {
+	describe( 'transformDeltas()', () => {
 		it( 'should use deltaTransform.transformDeltaSets', () => {
 			sinon.spy( deltaTransform, 'transformDeltaSets' );
 

+ 103 - 11
packages/ckeditor5-engine/tests/model/schema.js

@@ -211,7 +211,53 @@ describe( 'Schema', () => {
 	} );
 
 	describe( 'getRule()', () => {
-		// TODO
+		it( 'returns a rule based on an item name', () => {
+			schema.register( 'foo', {
+				isMe: true
+			} );
+
+			expect( schema.getRule( 'foo' ).isMe ).to.be.true;
+		} );
+
+		it( 'returns a rule based on an element name', () => {
+			schema.register( 'foo', {
+				isMe: true
+			} );
+
+			expect( schema.getRule( new Element( 'foo' ) ).isMe ).to.be.true;
+		} );
+
+		it( 'returns a rule based on a text node', () => {
+			schema.register( '$text', {
+				isMe: true
+			} );
+
+			expect( schema.getRule( new Text( 'foo' ) ).isMe ).to.be.true;
+		} );
+
+		it( 'returns a rule based on a text proxy', () => {
+			schema.register( '$text', {
+				isMe: true
+			} );
+
+			const text = new Text( 'foo' );
+			const textProxy = new TextProxy( text, 0, 1 );
+
+			expect( schema.getRule( textProxy ).isMe ).to.be.true;
+		} );
+
+		it( 'returns a rule based on a schema context item', () => {
+			schema.register( 'foo', {
+				isMe: true
+			} );
+			const ctx = new SchemaContext( [ '$root', 'foo' ] );
+
+			expect( schema.getRule( ctx.last ).isMe ).to.be.true;
+		} );
+
+		it( 'returns undefined when trying to get an unregistered item', () => {
+			expect( schema.getRule( '404' ) ).to.be.undefined;
+		} );
 	} );
 
 	describe( 'isRegistered()', () => {
@@ -224,6 +270,13 @@ describe( 'Schema', () => {
 		it( 'returns false if an item was not registered', () => {
 			expect( schema.isRegistered( 'foo' ) ).to.be.false;
 		} );
+
+		it( 'uses getRule()\'s item to rule normalization', () => {
+			const stub = sinon.stub( schema, 'getRule' ).returns( {} );
+
+			expect( schema.isRegistered( 'foo' ) ).to.be.true;
+			expect( stub.calledOnce ).to.be.true;
+		} );
 	} );
 
 	describe( 'isBlock()', () => {
@@ -244,6 +297,13 @@ describe( 'Schema', () => {
 		it( 'returns false if an item was not registered at all', () => {
 			expect( schema.isBlock( 'foo' ) ).to.be.false;
 		} );
+
+		it( 'uses getRule()\'s item to rule normalization', () => {
+			const stub = sinon.stub( schema, 'getRule' ).returns( { isBlock: true } );
+
+			expect( schema.isBlock( 'foo' ) ).to.be.true;
+			expect( stub.calledOnce ).to.be.true;
+		} );
 	} );
 
 	describe( 'isLimit()', () => {
@@ -264,6 +324,13 @@ describe( 'Schema', () => {
 		it( 'returns false if an item was not registered at all', () => {
 			expect( schema.isLimit( 'foo' ) ).to.be.false;
 		} );
+
+		it( 'uses getRule()\'s item to rule normalization', () => {
+			const stub = sinon.stub( schema, 'getRule' ).returns( { isLimit: true } );
+
+			expect( schema.isLimit( 'foo' ) ).to.be.true;
+			expect( stub.calledOnce ).to.be.true;
+		} );
 	} );
 
 	describe( 'isObject()', () => {
@@ -284,6 +351,13 @@ describe( 'Schema', () => {
 		it( 'returns false if an item was not registered at all', () => {
 			expect( schema.isObject( 'foo' ) ).to.be.false;
 		} );
+
+		it( 'uses getRule()\'s item to rule normalization', () => {
+			const stub = sinon.stub( schema, 'getRule' ).returns( { isObject: true } );
+
+			expect( schema.isObject( 'foo' ) ).to.be.true;
+			expect( stub.calledOnce ).to.be.true;
+		} );
 	} );
 
 	describe( 'checkChild()', () => {
@@ -352,7 +426,6 @@ describe( 'Schema', () => {
 		} );
 
 		// TODO checks fires event
-		// TODO checks with a custom context array
 	} );
 
 	describe( 'checkAttribute()', () => {
@@ -375,8 +448,35 @@ describe( 'Schema', () => {
 			expect( schema.checkAttribute( new Text( 'foo' ), 'align' ) ).to.be.false;
 		} );
 
+		it( 'accepts a position as a context', () => {
+			const posInRoot = Position.createAt( root1 );
+			const posInParagraph = Position.createAt( r1p1 );
+
+			expect( schema.checkAttribute( posInRoot, 'align' ) ).to.be.false;
+			expect( schema.checkAttribute( posInParagraph, 'align' ) ).to.be.true;
+		} );
+
+		it( 'accepts an array of node names as a context', () => {
+			const contextInRoot = [ '$root' ];
+			const contextInParagraph = [ '$root', 'paragraph' ];
+			const contextInText = [ '$root', 'paragraph', '$text' ];
+
+			expect( schema.checkAttribute( contextInRoot, 'align' ) ).to.be.false;
+			expect( schema.checkAttribute( contextInParagraph, 'align' ) ).to.be.true;
+			expect( schema.checkAttribute( contextInText, 'bold' ) ).to.be.true;
+		} );
+
+		it( 'accepts an array of nodes as a context', () => {
+			const contextInRoot = [ root1 ];
+			const contextInParagraph = [ root1, r1p1 ];
+			const contextInText = [ root1, r1p1, r1p1.getChild( 0 ) ];
+
+			expect( schema.checkAttribute( contextInRoot, 'align' ) ).to.be.false;
+			expect( schema.checkAttribute( contextInParagraph, 'align' ) ).to.be.true;
+			expect( schema.checkAttribute( contextInText, 'bold' ) ).to.be.true;
+		} );
+
 		// TODO checks fires event
-		// TODO checks with a custom context array
 	} );
 
 	describe( 'getLimitElement()', () => {
@@ -1096,7 +1196,6 @@ describe( 'Schema', () => {
 				expect( schema.checkChild( r1p1, r1p1.getChild( 0 ) ) ).to.be.true;
 			} );
 
-			// TODO we need to make it possible to disallow bQ in bQ.
 			it( 'passes $root>blockQuote>paragraph – blockQuote inherits content of $root', () => {
 				schema.register( '$root' );
 				schema.register( 'blockQuote', {
@@ -1870,13 +1969,6 @@ describe( 'Schema', () => {
 			expect( schema.isObject( 'caption' ) ).to.be.false;
 		} );
 	} );
-
-	// TODO:
-	// * getValidRanges
-	// * checkAttributeInSelectionn
-	// * add normalization to isObject(), isLimit(), isBlock(), isRegistered() and improve the existing code
-	//   * see insertContent's _checkIsObject()
-	// * test the default abstract entities (in model.js)
 } );
 
 describe( 'SchemaContext', () => {