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

A couple of minor improvements and refactoring.

Piotrek Koszuliński преди 6 години
родител
ревизия
19dac9db4d

+ 8 - 5
packages/ckeditor5-engine/src/controller/datacontroller.js

@@ -118,8 +118,8 @@ export default class DataController {
 	 * @param {Object} [options]
 	 * @param {String} [options.rootName='main'] Root name.
 	 * @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `empty` by default,
-	 * which means whenever editor content is considered empty, the empty string will be returned. To turn off trimming completely
-	 * use `none`. In such cases exact content will be returned (for example `<p>&nbsp;</p>` for empty editor).
+	 * which means whenever editor content is considered empty, an empty string will be returned. To turn off trimming completely
+	 * use `'none'`. In such cases exact content will be returned (for example `<p>&nbsp;</p>` for an empty editor).
 	 * @returns {String} Output data.
 	 */
 	get( options ) {
@@ -131,7 +131,7 @@ export default class DataController {
 			 * is called with non-existent root name. For example, if there is an editor instance with only `main` root,
 			 * calling {@link #get} like:
 			 *
-			 * 		data.get( 'root2' );
+			 *		data.get( 'root2' );
 			 *
 			 * will throw this error.
 			 *
@@ -142,8 +142,11 @@ export default class DataController {
 
 		const root = this.model.document.getRoot( rootName );
 
-		// Get model range.
-		return trim === 'empty' && !this.model.hasContent( root, { trimWhitespaces: true } ) ? '' : this.stringify( root );
+		if ( trim === 'empty' && !this.model.hasContent( root, { ignoreWhitespaces: true } ) ) {
+			return '';
+		}
+
+		return this.stringify( root );
 	}
 
 	/**

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

@@ -447,44 +447,47 @@ export default class Model {
 	 * Checks whether the given {@link module:engine/model/range~Range range} or
 	 * {@link module:engine/model/element~Element element} has any meaningful content.
 	 *
-	 * Meaningful content is any text node, element which is registered in the {@link module:engine/model/schema~Schema schema}
-	 * or any {@link module:engine/model/markercollection~Marker marker} which
+	 * Meaningful content is:
+	 *
+	 * * any text node (`options.ignoreWhitespaces` allows controlling whether this text node must also contain
+	 * any non-whitespace characters),
+	 * * or any {@link module:engine/model/schema~Schema#isObject object element},
+	 * * or any {@link module:engine/model/markercollection~Marker marker} which
 	 * {@link module:engine/model/markercollection~Marker#_affectsData affects data}.
 	 *
+	 * This means that a range containing an empty `<paragraph></paragraph>` is not considered to have a meaningful content.
+	 * However, a range containing an `<image></image>` (which would normally be marked in the schema as an object element)
+	 * is considered non-empty.
+	 *
 	 * @param {module:engine/model/range~Range|module:engine/model/element~Element} rangeOrElement Range or element to check.
 	 * @param {Object} [options]
-	 * @param {Boolean} [options.trimWhitespaces] Whether text node with whitespaces only should be considered to be empty element.
+	 * @param {Boolean} [options.ignoreWhitespaces] Whether text node with whitespaces only should be considered empty.
 	 * @returns {Boolean}
 	 */
 	hasContent( rangeOrElement, options ) {
-		if ( rangeOrElement instanceof ModelElement ) {
-			rangeOrElement = ModelRange._createIn( rangeOrElement );
-		}
+		const range = rangeOrElement instanceof ModelElement ? ModelRange._createIn( rangeOrElement ) : rangeOrElement;
 
-		if ( rangeOrElement.isCollapsed ) {
+		if ( range.isCollapsed ) {
 			return false;
 		}
 
 		// Check if there are any markers which affects data in this given range.
-		for ( const intersectingMarker of this.markers.getMarkersIntersectingRange( rangeOrElement ) ) {
+		for ( const intersectingMarker of this.markers.getMarkersIntersectingRange( range ) ) {
 			if ( intersectingMarker.affectsData ) {
 				return true;
 			}
 		}
 
-		const { trimWhitespaces = false } = options || {};
+		const { ignoreWhitespaces = false } = options || {};
 
-		for ( const item of rangeOrElement.getItems() ) {
-			// Remember, `TreeWalker` returns always `textProxy` nodes.
+		for ( const item of range.getItems() ) {
 			if ( item.is( 'textProxy' ) ) {
-				if ( !trimWhitespaces ) {
+				if ( !ignoreWhitespaces ) {
 					return true;
-				} else if ( item.data.match( /\S+/gi ) !== null ) {
+				} else if ( item.data.search( /\S/ ) !== -1 ) {
 					return true;
 				}
-			}
-
-			if ( this.schema.isObject( item ) ) {
+			} else if ( this.schema.isObject( item ) ) {
 				return true;
 			}
 		}

+ 12 - 12
packages/ckeditor5-engine/tests/model/model.js

@@ -528,10 +528,10 @@ describe( 'Model', () => {
 			expect( model.hasContent( pFoo ) ).to.be.true;
 		} );
 
-		it( 'should return true if given element has text node (trimWhitespaces)', () => {
+		it( 'should return true if given element has text node (ignoreWhitespaces)', () => {
 			const pFoo = root.getChild( 1 );
 
-			expect( model.hasContent( pFoo, { trimWhitespaces: true } ) ).to.be.true;
+			expect( model.hasContent( pFoo, { ignoreWhitespaces: true } ) ).to.be.true;
 		} );
 
 		it( 'should return true if given element has text node containing spaces only', () => {
@@ -545,7 +545,7 @@ describe( 'Model', () => {
 			expect( model.hasContent( pEmpty ) ).to.be.true;
 		} );
 
-		it( 'should false true if given element has text node containing spaces only (trimWhitespaces)', () => {
+		it( 'should false true if given element has text node containing spaces only (ignoreWhitespaces)', () => {
 			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
 			model.enqueueChange( 'transparent', writer => {
@@ -553,7 +553,7 @@ describe( 'Model', () => {
 				writer.insertText( '    ', pEmpty, 'end' );
 			} );
 
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
 		it( 'should return true if given element has element that is an object', () => {
@@ -622,7 +622,7 @@ describe( 'Model', () => {
 			} );
 
 			expect( model.hasContent( pEmpty ) ).to.be.false;
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
 		it( 'should return false for empty element with marker (usingOperation=true, affectsData=false)', () => {
@@ -635,10 +635,10 @@ describe( 'Model', () => {
 			} );
 
 			expect( model.hasContent( pEmpty ) ).to.be.false;
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
-		it( 'should return false (trimWhitespaces) for empty text with marker (usingOperation=false, affectsData=false)', () => {
+		it( 'should return false (ignoreWhitespaces) for empty text with marker (usingOperation=false, affectsData=false)', () => {
 			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
 			model.enqueueChange( 'transparent', writer => {
@@ -651,7 +651,7 @@ describe( 'Model', () => {
 				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
 			} );
 
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
 		it( 'should return true for empty text with marker (usingOperation=false, affectsData=false)', () => {
@@ -680,7 +680,7 @@ describe( 'Model', () => {
 			} );
 
 			expect( model.hasContent( pEmpty ) ).to.be.false;
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
 		it( 'should return false for empty element with marker (usingOperation=true, affectsData=true)', () => {
@@ -693,10 +693,10 @@ describe( 'Model', () => {
 			} );
 
 			expect( model.hasContent( pEmpty ) ).to.be.false;
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
 		} );
 
-		it( 'should return true (trimWhitespaces) for empty text with marker (usingOperation=false, affectsData=true)', () => {
+		it( 'should return true (ignoreWhitespaces) for empty text with marker (usingOperation=false, affectsData=true)', () => {
 			const pEmpty = root.getChild( 0 ).getChild( 0 );
 
 			model.enqueueChange( 'transparent', writer => {
@@ -710,7 +710,7 @@ describe( 'Model', () => {
 			} );
 
 			expect( model.hasContent( pEmpty ) ).to.be.true;
-			expect( model.hasContent( pEmpty, { trimWhitespaces: true } ) ).to.be.true;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.true;
 		} );
 	} );