diff --git a/package.json b/package.json index bf49578ff..8a5182796 100644 --- a/package.json +++ b/package.json @@ -44,8 +44,8 @@ "lint-staged": "^7.0.0" }, "engines": { - "node": ">=6.9.0", - "npm": ">=3.0.0" + "node": ">=8.0.0", + "npm": ">=5.7.1" }, "author": "CKSource (http://cksource.com/)", "license": "GPL-2.0-or-later", diff --git a/src/controller/datacontroller.js b/src/controller/datacontroller.js index 14efa575a..67d502f0c 100644 --- a/src/controller/datacontroller.js +++ b/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 `
` 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 ); } /** diff --git a/src/model/model.js b/src/model/model.js index c844ffd2c..3c970ab29 100644 --- a/src/model/model.js +++ b/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 `
foo
' ); + expect( data.get( { trim: 'empty' } ) ).to.equal( 'foo
' ); } ); - it( 'should get empty paragraph', () => { + it( 'should trim empty paragraph by default', () => { schema.register( 'paragraph', { inheritAllFrom: '$block' } ); setData( model, '' ); + 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, '
' ); } ); it( 'should get two paragraphs', () => { @@ -364,6 +375,7 @@ describe( 'DataController', () => { downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } ); expect( data.get() ).to.equal( '
foo
bar
' ); + expect( data.get( { trim: 'empty' } ) ).to.equal( 'foo
bar
' ); } ); 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( 'foobar
' ); + expect( data.get( { trim: 'empty' } ) ).to.equal( 'foobar
' ); } ); it( 'should get paragraphs with bold', () => { @@ -390,6 +404,7 @@ describe( 'DataController', () => { downcastHelpers.attributeToElement( { model: 'bold', view: 'strong' } ); expect( data.get() ).to.equal( 'foobar
' ); + expect( data.get( { trim: 'empty' } ) ).to.equal( 'foobar
' ); } ); 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( 'foo
' ); - expect( data.get( 'main' ) ).to.equal( 'foo
' ); - expect( data.get( 'title' ) ).to.equal( 'Bar' ); + expect( data.get( { rootName: 'main' } ) ).to.equal( 'foo
' ); + 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.' diff --git a/tests/model/model.js b/tests/model/model.js index e1b8bea74..a0ef069bc 100644 --- a/tests/model/model.js +++ b/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', () => { '1
2
' ); } ); } ); + + // ckeditor/ckeditor5-utils#269 + // The expected times has a significant margin above the usual execution time (which is around 40-50% + // of the expected time) because it depends on the browser and environment in which tests are run. + // However, for larger data sets the difference between using `diff()` and `fastDiff()` (see above issue for context) + // is more than 10x in execution time so it is clearly visible in these tests when something goes wrong. + describe( 'rendering performance', () => { + before( function() { + // Ignore on Edge browser where performance is quite poor. + if ( env.isEdge ) { + this.skip(); + } + } ); + + it( 'should not take more than 350ms to render around 300 element nodes (same html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 65 ), generateViewData1( 55 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + it( 'should not take more than 350ms to render around 300 element nodes (different html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 55 ), generateViewData2( 65 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + it( 'should not take more than 350ms to render around 500 element nodes (same html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 105 ), generateViewData1( 95 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + it( 'should not take more than 350ms to render around 500 element nodes (different html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 95 ), generateViewData2( 105 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + it( 'should not take more than 350ms to render around 1000 element nodes (same html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 195 ), generateViewData1( 205 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + it( 'should not take more than 350ms to render around 1000 element nodes (different html)', () => { + const renderingTime = measureRenderingTime( viewRoot, generateViewData1( 205 ), generateViewData2( 195 ) ); + expect( renderingTime ).to.be.within( 0, 350 ); + } ); + + function measureRenderingTime( viewRoot, initialData, newData ) { + // Set initial data. + const initialView = parse( initialData ); + viewRoot._appendChild( initialView ); + renderer.markToSync( 'children', viewRoot ); + renderer.render(); + + // Set new data. + const newView = parse( newData ); + viewRoot._removeChildren( 0, viewRoot.childCount ); + viewRoot._appendChild( newView ); + renderer.markToSync( 'children', viewRoot ); + + // Measure render time. + const start = Date.now(); + + renderer.render(); + + return Date.now() - start; + } + + function generateViewData1( repeat = 1 ) { + const viewData = '' + + '