Explorar el Código

Merge pull request #1656 from ckeditor/t/ckeditor5/401

Feature: Introduced whitespace trimming to `Model#hasContent()`. `DataController#get()` method can now trim empty data (so it returns empty string instead of `<p>&nbsp;</p>`). Closes [ckeditor/ckeditor5#401](https://github.com/ckeditor/ckeditor5/issues/401).

BREAKING CHANGE: `DataController#get()` method now returns an empty string when the editor content is empty (instead of returning e.g. `<p>&nbsp;</p>`).
Piotrek Koszuliński hace 6 años
padre
commit
b45c2d8fd8

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

@@ -115,17 +115,23 @@ export default class DataController {
 	 * Returns the model's data converted by downcast dispatchers attached to {@link #downcastDispatcher} and
 	 * formatted by the {@link #processor data processor}.
 	 *
-	 * @param {String} [rootName='main'] Root name.
+	 * @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, 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( rootName = 'main' ) {
+	get( options ) {
+		const { rootName = 'main', trim = 'empty' } = options || {};
+
 		if ( !this._checkIfRootsExists( [ rootName ] ) ) {
 			/**
 			 * Cannot get data from a non-existing root. This error is thrown when {@link #get DataController#get() method}
 			 * 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.
 			 *
@@ -134,8 +140,13 @@ export default class DataController {
 			throw new CKEditorError( 'datacontroller-get-non-existent-root: Attempting to get data from a non-existing root.' );
 		}
 
-		// Get model range.
-		return this.stringify( this.model.document.getRoot( rootName ) );
+		const root = this.model.document.getRoot( rootName );
+
+		if ( trim === 'empty' && !this.model.hasContent( root, { ignoreWhitespaces: true } ) ) {
+			return '';
+		}
+
+		return this.stringify( root );
 	}
 
 	/**

+ 34 - 11
packages/ckeditor5-engine/src/model/model.js

@@ -445,26 +445,49 @@ 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 content.
+	 * {@link module:engine/model/element~Element element} has any meaningful content.
 	 *
-	 * Content is any text node or element which is registered in the {@link module:engine/model/schema~Schema schema}.
+	 * 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.ignoreWhitespaces] Whether text node with whitespaces only should be considered empty.
 	 * @returns {Boolean}
 	 */
-	hasContent( rangeOrElement ) {
-		if ( rangeOrElement instanceof ModelElement ) {
-			rangeOrElement = ModelRange._createIn( rangeOrElement );
-		}
+	hasContent( rangeOrElement, options ) {
+		const range = rangeOrElement instanceof ModelElement ? ModelRange._createIn( rangeOrElement ) : rangeOrElement;
 
-		if ( rangeOrElement.isCollapsed ) {
+		if ( range.isCollapsed ) {
 			return false;
 		}
 
-		for ( const item of rangeOrElement.getItems() ) {
-			// Remember, `TreeWalker` returns always `textProxy` nodes.
-			if ( item.is( 'textProxy' ) || this.schema.isObject( item ) ) {
+		// Check if there are any markers which affects data in this given range.
+		for ( const intersectingMarker of this.markers.getMarkersIntersectingRange( range ) ) {
+			if ( intersectingMarker.affectsData ) {
+				return true;
+			}
+		}
+
+		const { ignoreWhitespaces = false } = options || {};
+
+		for ( const item of range.getItems() ) {
+			if ( item.is( 'textProxy' ) ) {
+				if ( !ignoreWhitespaces ) {
+					return true;
+				} else if ( item.data.search( /\S/ ) !== -1 ) {
+					return true;
+				}
+			} else if ( this.schema.isObject( item ) ) {
 				return true;
 			}
 		}

+ 20 - 5
packages/ckeditor5-engine/tests/controller/datacontroller.js

@@ -346,15 +346,26 @@ describe( 'DataController', () => {
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
 			expect( data.get() ).to.equal( '<p>foo</p>' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo</p>' );
 		} );
 
-		it( 'should get empty paragraph', () => {
+		it( 'should trim empty paragraph by default', () => {
 			schema.register( 'paragraph', { inheritAllFrom: '$block' } );
 			setData( model, '<paragraph></paragraph>' );
 
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
-			expect( data.get() ).to.equal( '<p>&nbsp;</p>' );
+			expect( data.get() ).to.equal( '' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( '' );
+		} );
+
+		it( 'should get empty paragraph (with trim=none)', () => {
+			schema.register( 'paragraph', { inheritAllFrom: '$block' } );
+			setData( model, '<paragraph></paragraph>' );
+
+			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
+
+			expect( data.get( { trim: 'none' } ) ).to.equal( '<p>&nbsp;</p>' );
 		} );
 
 		it( 'should get two paragraphs', () => {
@@ -364,6 +375,7 @@ describe( 'DataController', () => {
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
 			expect( data.get() ).to.equal( '<p>foo</p><p>bar</p>' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo</p><p>bar</p>' );
 		} );
 
 		it( 'should get text directly in root', () => {
@@ -371,6 +383,7 @@ describe( 'DataController', () => {
 			setData( model, 'foo' );
 
 			expect( data.get() ).to.equal( 'foo' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( 'foo' );
 		} );
 
 		it( 'should get paragraphs without bold', () => {
@@ -380,6 +393,7 @@ describe( 'DataController', () => {
 			downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
 
 			expect( data.get() ).to.equal( '<p>foobar</p>' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foobar</p>' );
 		} );
 
 		it( 'should get paragraphs with bold', () => {
@@ -390,6 +404,7 @@ describe( 'DataController', () => {
 			downcastHelpers.attributeToElement( { model: 'bold', view: 'strong' } );
 
 			expect( data.get() ).to.equal( '<p>foo<strong>bar</strong></p>' );
+			expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo<strong>bar</strong></p>' );
 		} );
 
 		it( 'should get root name as a parameter', () => {
@@ -403,13 +418,13 @@ describe( 'DataController', () => {
 			downcastHelpers.attributeToElement( { model: 'bold', view: 'strong' } );
 
 			expect( data.get() ).to.equal( '<p>foo</p>' );
-			expect( data.get( 'main' ) ).to.equal( '<p>foo</p>' );
-			expect( data.get( 'title' ) ).to.equal( 'Bar' );
+			expect( data.get( { rootName: 'main' } ) ).to.equal( '<p>foo</p>' );
+			expect( data.get( { rootName: 'title' } ) ).to.equal( 'Bar' );
 		} );
 
 		it( 'should throw an error when non-existent root is used', () => {
 			expect( () => {
-				data.get( 'nonexistent' );
+				data.get( { rootName: 'nonexistent' } );
 			} ).to.throw(
 				CKEditorError,
 				'datacontroller-get-non-existent-root: Attempting to get data from a non-existing root.'

+ 142 - 1
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 (ignoreWhitespaces)', () => {
+			const pFoo = root.getChild( 1 );
+
+			expect( model.hasContent( pFoo, { ignoreWhitespaces: 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 (ignoreWhitespaces)', () => {
+			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, { ignoreWhitespaces: true } ) ).to.be.false;
+		} );
+
 		it( 'should return true if given element has element that is an object', () => {
 			const divImg = root.getChild( 2 );
 
@@ -571,6 +605,113 @@ describe( 'Model', () => {
 
 			expect( model.hasContent( range ) ).to.be.false;
 		} );
+
+		it( 'should return false for empty list items', () => {
+			const range = new ModelRange( ModelPosition._createAt( root, 3 ), ModelPosition._createAt( root, 6 ) );
+
+			expect( model.hasContent( range ) ).to.be.false;
+		} );
+
+		it( 'should return false for empty element with marker (usingOperation=false, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).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)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: false } );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.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 => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
+
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
+			} );
+
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
+		} );
+
+		it( 'should return true for empty text with marker (usingOperation=false, affectsData=false)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
+
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.true;
+		} );
+
+		it( 'should return false for empty element with marker (usingOperation=false, affectsData=true)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			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, { ignoreWhitespaces: true } ) ).to.be.false;
+		} );
+
+		it( 'should return false for empty element with marker (usingOperation=true, affectsData=true)', () => {
+			const pEmpty = root.getChild( 0 ).getChild( 0 );
+
+			model.enqueueChange( 'transparent', writer => {
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: true } );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.false;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
+		} );
+
+		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 => {
+				// Insert empty text.
+				const text = writer.createText( '    ', { bold: true } );
+				writer.append( text, pEmpty );
+
+				// Insert marker.
+				const range = ModelRange._createIn( pEmpty );
+				writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
+			} );
+
+			expect( model.hasContent( pEmpty ) ).to.be.true;
+			expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.true;
+		} );
 	} );
 
 	describe( 'createPositionFromPath()', () => {