浏览代码

Refactoring.

Krzysztof Krztoń 6 年之前
父节点
当前提交
a5873909dc

+ 9 - 17
packages/ckeditor5-engine/src/controller/datacontroller.js

@@ -123,10 +123,7 @@ export default class DataController {
 	 * @returns {String} Output data.
 	 */
 	get( options ) {
-		options = options || {};
-
-		const rootName = options.rootName || 'main';
-		const trim = options.trim || 'empty';
+		const { rootName = 'main', trim = 'empty' } = options || {};
 
 		if ( !this._checkIfRootsExists( [ rootName ] ) ) {
 			/**
@@ -143,8 +140,10 @@ export default class DataController {
 			throw new CKEditorError( 'datacontroller-get-non-existent-root: Attempting to get data from a non-existing root.' );
 		}
 
+		const root = this.model.document.getRoot( rootName );
+
 		// Get model range.
-		return this.stringify( this.model.document.getRoot( rootName ), trim === 'empty' );
+		return trim === 'empty' && !this.model.hasContent( root, { trimWhitespaces: true } ) ? '' : this.stringify( root );
 	}
 
 	/**
@@ -154,21 +153,14 @@ export default class DataController {
 	 *
 	 * @param {module:engine/model/element~Element|module:engine/model/documentfragment~DocumentFragment} modelElementOrFragment
 	 * Element whose content will be stringified.
-	 * @param {Boolean} [skipEmpty=false] Whether content considered empty should be skipped. The method
-	 * will return an empty string in such cases.
 	 * @returns {String} Output data.
 	 */
-	stringify( modelElementOrFragment, skipEmpty = false ) {
-		if ( skipEmpty && this.model.isEmpty( modelElementOrFragment ) ) {
-			// If skipEmpty and model is considered empty return empty string.
-			return '';
-		} else {
-			// Model -> view.
-			const viewDocumentFragment = this.toView( modelElementOrFragment );
+	stringify( modelElementOrFragment ) {
+		// Model -> view.
+		const viewDocumentFragment = this.toView( modelElementOrFragment );
 
-			// View -> data.
-			return this.processor.toData( viewDocumentFragment );
-		}
+		// View -> data.
+		return this.processor.toData( viewDocumentFragment );
 	}
 
 	/**

+ 16 - 42
packages/ckeditor5-engine/src/model/model.js

@@ -455,7 +455,7 @@ export default class Model {
 	 * @param {module:engine/model/range~Range|module:engine/model/element~Element} rangeOrElement Range or element to check.
 	 * @returns {Boolean}
 	 */
-	hasContent( rangeOrElement ) {
+	hasContent( rangeOrElement, options ) {
 		if ( rangeOrElement instanceof ModelElement ) {
 			rangeOrElement = ModelRange._createIn( rangeOrElement );
 		}
@@ -464,57 +464,31 @@ export default class Model {
 			return false;
 		}
 
-		for ( const item of rangeOrElement.getItems() ) {
-			// Remember, `TreeWalker` returns always `textProxy` nodes.
-			if ( item.is( 'textProxy' ) || this.schema.isObject( item ) ) {
-				return true;
-			}
-		}
-
-		return false;
-	}
-
-	/**
-	 * Checks whether the given {@link module:engine/model/range~Range range} or
-	 * {@link module:engine/model/element~Element element} is considered empty.
-	 *
-	 * The range or element is considered non-empty if it contains any:
-	 * 		* EmptyElement
-	 * 		* Text node containing at least one non-whitepsace character
-	 * 		* Non-plain `ContainerElement` (for example widget)
-	 * 		* Non-plain `AttributeElement` (for example comment)
-	 *
-	 * 	This method should be used to check if the element/range/editor contains any printable/meaningful content.
-	 * 	It is also the correct method to check if editor is empty.
-	 *
-	 * @param {module:engine/model/range~Range|module:engine/model/element~Element} rangeOrElement Range or element to check.
-	 * @returns {Boolean}
-	 */
-	isEmpty( rangeOrElement ) {
-		if ( rangeOrElement instanceof ModelElement ) {
-			rangeOrElement = ModelRange._createIn( rangeOrElement );
-		}
-
-		if ( rangeOrElement.isCollapsed ) {
-			return true;
-		}
-
 		// Check if there are any markers which affects data in this given range.
 		for ( const intersectingMarker of this.markers.getMarkersIntersectingRange( rangeOrElement ) ) {
 			if ( intersectingMarker.affectsData ) {
-				return false;
+				return true;
 			}
 		}
 
+		const { trimWhitespaces = false } = options || {};
+
 		for ( const item of rangeOrElement.getItems() ) {
-			if ( item.is( 'textProxy' ) && item.data.match( /\S+/gi ) !== null ) {
-				return false;
-			} else if ( this.schema.isObject( item ) || item.is( 'emptyElement' ) ) {
-				return false;
+			// Remember, `TreeWalker` returns always `textProxy` nodes.
+			if ( item.is( 'textProxy' ) ) {
+				if ( !trimWhitespaces ) {
+					return true;
+				} else if ( item.data.match( /\S+/gi ) !== null ) {
+					return true;
+				}
+			}
+
+			if ( this.schema.isObject( item ) ) {
+				return true;
 			}
 		}
 
-		return true;
+		return false;
 	}
 
 	/**

+ 1 - 0
packages/ckeditor5-engine/tests/controller/datacontroller.js

@@ -355,6 +355,7 @@ describe( 'DataController', () => {
 
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
+			expect( data.get() ).to.equal( '' );
 			expect( data.get( { trim: 'empty' } ) ).to.equal( '' );
 		} );
 

+ 102 - 287
packages/ckeditor5-engine/tests/model/model.js

@@ -500,6 +500,9 @@ describe( 'Model', () => {
 				isObject: true
 			} );
 			schema.extend( 'image', { allowIn: 'div' } );
+			schema.register( 'listItem', {
+				inheritAllFrom: '$block'
+			} );
 
 			setData(
 				model,
@@ -510,7 +513,10 @@ describe( 'Model', () => {
 				'<paragraph>foo</paragraph>' +
 				'<div>' +
 				'<image></image>' +
-				'</div>'
+				'</div>' +
+				'<listItem></listItem>' +
+				'<listItem></listItem>' +
+				'<listItem></listItem>'
 			);
 
 			root = model.document.getRoot();
@@ -522,6 +528,34 @@ describe( 'Model', () => {
 			expect( model.hasContent( pFoo ) ).to.be.true;
 		} );
 
+		it( 'should return true if given element has text node (trimWhitespaces)', () => {
+			const pFoo = root.getChild( 1 );
+
+			expect( model.hasContent( pFoo, { trimWhitespaces: true } ) ).to.be.true;
+		} );
+
+		it( 'should return true if given element has text node containing spaces only', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
+				writer.insertText( '    ', pEmpty, 'end' );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.true;
+		} );
+
+		it( 'should false true if given element has text node containing spaces only (trimWhitespaces)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
+				writer.insertText( '    ', pEmpty, 'end' );
+			} );
+
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+		} );
+
 		it( 'should return true if given element has element that is an object', () => {
 			const divImg = root.getChild( 2 );
 
@@ -571,332 +605,113 @@ describe( 'Model', () => {
 
 			expect( model.hasContent( range ) ).to.be.false;
 		} );
-	} );
-
-	describe( 'isEmpty()', () => {
-		beforeEach( () => {
-			// Register paragraphs and divs.
-			schema.register( 'paragraph', { inheritAllFrom: '$block' } );
-			schema.register( 'div', { inheritAllFrom: '$block' } );
-			schema.extend( '$block', { allowIn: 'div' } );
-
-			// Allow bold attribute on text nodes.
-			schema.extend( '$text', { allowAttributes: 'bold' } );
-
-			// Allow sub attribute on text nodes.
-			schema.extend( '$text', { allowAttributes: 'subscript' } );
-
-			// Register blockquotes.
-			schema.register( 'blockQuote', {
-				allowWhere: '$block',
-				allowContentOf: '$root'
-			} );
-
-			// Register headings.
-			schema.register( 'heading1', {
-				inheritAllFrom: '$block'
-			} );
-
-			// Register images.
-			schema.register( 'image', {
-				isObject: true,
-				isBlock: true,
-				allowWhere: '$block',
-				allowAttributes: [ 'alt', 'src', 'srcset' ]
-			} );
-			schema.extend( 'image', { allowIn: 'div' } );
-
-			// Register lists.
-			schema.register( 'listItem', {
-				inheritAllFrom: '$block',
-				allowAttributes: [ 'listType', 'listIndent' ]
-			} );
-
-			// Register media.
-			schema.register( 'media', {
-				isObject: true,
-				isBlock: true,
-				allowWhere: '$block',
-				allowAttributes: [ 'url' ]
-			} );
-
-			// Register tables.
-			schema.register( 'table', {
-				allowWhere: '$block',
-				allowAttributes: [ 'headingRows', 'headingColumns' ],
-				isLimit: true,
-				isObject: true,
-				isBlock: true
-			} );
-
-			schema.register( 'tableRow', {
-				allowIn: 'table',
-				isLimit: true
-			} );
-
-			schema.register( 'tableCell', {
-				allowIn: 'tableRow',
-				allowAttributes: [ 'colspan', 'rowspan' ],
-				isLimit: true
-			} );
-
-			// Allow all $block content inside table cell.
-			schema.extend( '$block', { allowIn: 'tableCell' } );
-		} );
-
-		it( 'should return true for collapsed range', () => {
-			setData( model, '<paragraph>Foo[] Bar</paragraph>' );
-
-			const root = model.document.getRoot();
-			const selection = model.document.selection;
-			const range = selection.getFirstRange();
-
-			expect( stringify( root, selection ) ).to.equal( '<paragraph>Foo[] Bar</paragraph>' );
-
-			expect( model.isEmpty( range ) ).to.true;
-		} );
-
-		it( 'should return true for range on empty text', () => {
-			setData( model, '<paragraph>Foo[ ]Bar</paragraph>' );
-
-			const root = model.document.getRoot();
-			const selection = model.document.selection;
-			const range = selection.getFirstRange();
-
-			expect( stringify( root, selection ) ).to.equal( '<paragraph>Foo[ ]Bar</paragraph>' );
-
-			expect( model.isEmpty( range ) ).to.true;
-		} );
-
-		it( 'should return false for range on non-empty text', () => {
-			setData( model, '<paragraph>F[oo ]Bar</paragraph>' );
-
-			const root = model.document.getRoot();
-			const selection = model.document.selection;
-			const range = selection.getFirstRange();
-
-			expect( stringify( root, selection ) ).to.equal( '<paragraph>F[oo ]Bar</paragraph>' );
-
-			expect( model.isEmpty( range ) ).to.false;
-		} );
 
-		it( 'should return true for empty paragraph', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>' );
-		} );
-
-		it( 'should return true for multiple empty paragraphs', () => {
-			expectIsEmpty( true, '<paragraph></paragraph><paragraph></paragraph><paragraph></paragraph>' );
-		} );
-
-		it( 'should return true for paragraph with spaces only', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
-					writer.insertText( '    ', root.getChild( 0 ), 'end' );
-				} );
+		it( 'should return false for empty list items', () => {
+			const range = new ModelRange( ModelPosition._createAt( root, 3 ), ModelPosition._createAt( root, 6 ) );
 
-				return '<paragraph>    </paragraph>';
-			} );
+			expect( model.hasContent( range ) ).to.be.false;
 		} );
 
-		it( 'should return true for paragraph with whitespaces only', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
-					writer.insertText( ' \r\n\t\f\v ', root.getChild( 0 ), 'end' );
-				} );
+		it( 'should return false for empty element with marker (usingOperation=false, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-				return '<paragraph> \r\n\t\f\v </paragraph>';
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
 			} );
-		} );
-
-		it( 'should return true for text with attribute containing spaces only', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
-					const text = writer.createText( '   ', { bold: true } );
-					writer.append( text, root.getChild( 0 ) );
-				} );
 
-				return '<paragraph><$text bold="true">   </$text></paragraph>';
-			} );
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
 		} );
 
-		it( 'should return true for text with attribute containing whitespaces only', () => {
-			expectIsEmpty( true, '<paragraph></paragraph><paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
-					const text1 = writer.createText( '          ', { bold: true } );
-					const text2 = writer.createText( ' \r\n\t\f\v ', { bold: true, subscript: true } );
-
-					writer.append( text1, root.getChild( 0 ) );
-					writer.append( text2, root.getChild( 1 ) );
-				} );
+		it( 'should return false for empty element with marker (usingOperation=true, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-				return '<paragraph><$text bold="true">          </$text></paragraph>' +
-					'<paragraph><$text bold="true" subscript="true"> \r\n\t\f\v </$text></paragraph>';
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: false } );
 			} );
-		} );
-
-		it( 'should return false for paragraph with text', () => {
-			expectIsEmpty( false, '<paragraph>Foo Bar</paragraph>' );
-		} );
-
-		it( 'should return false for empty paragraph and paragraph with text', () => {
-			expectIsEmpty( false, '<paragraph></paragraph><paragraph>Foo Bar</paragraph>' );
-		} );
-
-		it( 'should return true for nested empty blocks', () => {
-			expectIsEmpty( true, '<div><paragraph></paragraph></div>' );
-		} );
-
-		it( 'should return true for multiple nested empty blocks', () => {
-			expectIsEmpty( true, '<div><paragraph></paragraph><blockQuote></blockQuote></div>' );
-		} );
 
-		it( 'should return true for empty heading', () => {
-			expectIsEmpty( true, '<heading1></heading1>' );
-		} );
-
-		it( 'should return true for empty list (single list item)', () => {
-			expectIsEmpty( true, '<listItem></listItem>' );
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
 		} );
 
-		it( 'should return true for empty list (multiple list item)', () => {
-			expectIsEmpty( true, '<listItem></listItem><listItem></listItem><listItem></listItem>' );
-		} );
+		it( 'should return false (trimWhitespaces) for empty text with marker (usingOperation=false, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-		it( 'should return true for empty list with whitespaces only', () => {
-			expectIsEmpty( true, '<listItem></listItem><listItem></listItem>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
-					writer.insertText( ' \r\n\t\f\v ', root.getChild( 0 ), 'end' );
-					writer.insertText( '  ', root.getChild( 1 ), 'end' );
-				} );
+			model.enqueueChange( 'transparent', writer => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
 
-				return '<listItem> \r\n\t\f\v </listItem><listItem>  </listItem>';
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
 			} );
-		} );
-
-		it( 'should return false for list with text', () => {
-			expectIsEmpty( false, '<listItem>foo</listItem><listItem>bar</listItem>' );
-		} );
-
-		it( 'should return false for empty table', () => {
-			expectIsEmpty( false, '<table><tableRow><tableCell></tableCell></tableRow></table>' );
-		} );
-
-		it( 'should return false for single image', () => {
-			expectIsEmpty( false, '<image></image>' );
-		} );
-
-		it( 'should return false for empty element and single image', () => {
-			expectIsEmpty( false, '<listItem></listItem><image></image>' );
-		} );
 
-		it( 'should return false for media element', () => {
-			expectIsEmpty( false, '<media url="https://www.youtube.com/watch?v=H08tGjXNHO4"></media>' );
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
 		} );
 
-		it( 'should return true for empty element with marker (usingOperation=false, affectsData=false)', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
-				} );
-
-				return '<paragraph><comment1:start></comment1:start></paragraph>';
-			} );
-		} );
+		it( 'should return true for empty text with marker (usingOperation=false, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-		it( 'should return true for empty element with marker (usingOperation=true, affectsData=false)', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: false } );
-				} );
+			model.enqueueChange( 'transparent', writer => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
 
-				return '<paragraph><comment1:start></comment1:start></paragraph>';
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
 			} );
-		} );
 
-		it( 'should return true for empty text with marker (usingOperation=false, affectsData=false)', () => {
-			expectIsEmpty( true, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert empty text.
-					const text = writer.createText( '    ', { bold: true } );
-					writer.append( text, root.getChild( 0 ) );
-
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
-				} );
-
-				return '<paragraph><comment1:start></comment1:start><$text bold="true">    </$text>' +
-					'<comment1:end></comment1:end></paragraph>';
-			} );
+			expect( model.hasContent( pEmpty ) ).to.be.true;
 		} );
 
 		it( 'should return false for empty element with marker (usingOperation=false, affectsData=true)', () => {
-			expectIsEmpty( false, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
-				} );
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-				return '<paragraph><comment1:start></comment1:start></paragraph>';
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
 			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
 		} );
 
 		it( 'should return false for empty element with marker (usingOperation=true, affectsData=true)', () => {
-			expectIsEmpty( false, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: true } );
-				} );
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-				return '<paragraph><comment1:start></comment1:start></paragraph>';
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: true } );
 			} );
-		} );
-
-		it( 'should return false for empty text with marker (usingOperation=false, affectsData=true)', () => {
-			expectIsEmpty( false, '<paragraph></paragraph>', root => {
-				model.enqueueChange( 'transparent', writer => {
-					// Insert empty text.
-					const text = writer.createText( '    ', { bold: true } );
-					writer.append( text, root.getChild( 0 ) );
-
-					// Insert marker.
-					const range = ModelRange._createIn( root.getChild( 0 ) );
-					writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
-				} );
 
-				return '<paragraph><comment1:start></comment1:start><$text bold="true">    </$text>' +
-					'<comment1:end></comment1:end></paragraph>';
-			} );
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
 		} );
 
-		function expectIsEmpty( isEmpty, modelData, modifyModel ) {
-			setData( model, modelData );
-
-			const root = model.document.getRoot();
+		it( 'should return true (trimWhitespaces) for empty text with marker (usingOperation=false, affectsData=true)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
-			let expectedModel = modelData;
-			if ( modifyModel ) {
-				expectedModel = modifyModel( root );
-			}
+			model.enqueueChange( 'transparent', writer => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
 
-			expect( stringify( root, null, model.markers ) ).to.equal( expectedModel );
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
+			} );
 
-			// Test by passing element.
-			expect( model.isEmpty( root ) ).to.equal( !!isEmpty );
-			// Test by passing range.
-			expect( model.isEmpty( ModelRange._createIn( root ) ) ).to.equal( !!isEmpty );
-		}
+			expect( model.hasContent( pEmpty ) ).to.be.true;
+			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.true;
+		} );
 	} );
 
 	describe( 'createPositionFromPath()', () => {