Skip to content
This repository was archived by the owner on Jun 26, 2020. It is now read-only.

Commit 2b95dc3

Browse files
ReinmarReinmar
authored
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](ckeditor/ckeditor5#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>`).
2 parents f579c93 + 55a07ef commit 2b95dc3

File tree

4 files changed

+212
-22
lines changed

4 files changed

+212
-22
lines changed

‎src/controller/datacontroller.js‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,17 +115,23 @@ export default class DataController {
115115
* Returns the model's data converted by downcast dispatchers attached to {@link #downcastDispatcher} and
116116
* formatted by the {@link #processor data processor}.
117117
*
118-
* @param {String} [rootName='main'] Root name.
118+
* @param {Object} [options]
119+
* @param {String} [options.rootName='main'] Root name.
120+
* @param {String} [options.trim='empty'] Whether returned data should be trimmed. This option is set to `empty` by default,
121+
* which means whenever editor content is considered empty, an empty string will be returned. To turn off trimming completely
122+
* use `'none'`. In such cases exact content will be returned (for example `<p>&nbsp;</p>` for an empty editor).
119123
* @returns {String} Output data.
120124
*/
121-
get( rootName = 'main' ) {
125+
get( options ) {
126+
const { rootName = 'main', trim = 'empty' } = options || {};
127+
122128
if ( !this._checkIfRootsExists( [ rootName ] ) ) {
123129
/**
124130
* Cannot get data from a non-existing root. This error is thrown when {@link #get DataController#get() method}
125131
* is called with non-existent root name. For example, if there is an editor instance with only `main` root,
126132
* calling {@link #get} like:
127133
*
128-
* data.get( 'root2' );
134+
* data.get( 'root2' );
129135
*
130136
* will throw this error.
131137
*
@@ -134,8 +140,13 @@ export default class DataController {
134140
throw new CKEditorError( 'datacontroller-get-non-existent-root: Attempting to get data from a non-existing root.' );
135141
}
136142

137-
// Get model range.
138-
return this.stringify( this.model.document.getRoot( rootName ) );
143+
const root = this.model.document.getRoot( rootName );
144+
145+
if ( trim === 'empty' && !this.model.hasContent( root, { ignoreWhitespaces: true } ) ) {
146+
return '';
147+
}
148+
149+
return this.stringify( root );
139150
}
140151

141152
/**

‎src/model/model.js‎

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -445,26 +445,49 @@ export default class Model {
445445

446446
/**
447447
* Checks whether the given {@link module:engine/model/range~Range range} or
448-
* {@link module:engine/model/element~Element element}
449-
* has any content.
448+
* {@link module:engine/model/element~Element element} has any meaningful content.
450449
*
451-
* Content is any text node or element which is registered in the {@link module:engine/model/schema~Schema schema}.
450+
* Meaningful content is:
451+
*
452+
* * any text node (`options.ignoreWhitespaces` allows controlling whether this text node must also contain
453+
* any non-whitespace characters),
454+
* * or any {@link module:engine/model/schema~Schema#isObject object element},
455+
* * or any {@link module:engine/model/markercollection~Marker marker} which
456+
* {@link module:engine/model/markercollection~Marker#_affectsData affects data}.
457+
*
458+
* This means that a range containing an empty `<paragraph></paragraph>` is not considered to have a meaningful content.
459+
* However, a range containing an `<image></image>` (which would normally be marked in the schema as an object element)
460+
* is considered non-empty.
452461
*
453462
* @param {module:engine/model/range~Range|module:engine/model/element~Element} rangeOrElement Range or element to check.
463+
* @param {Object} [options]
464+
* @param {Boolean} [options.ignoreWhitespaces] Whether text node with whitespaces only should be considered empty.
454465
* @returns {Boolean}
455466
*/
456-
hasContent( rangeOrElement ) {
457-
if ( rangeOrElement instanceof ModelElement ) {
458-
rangeOrElement = ModelRange._createIn( rangeOrElement );
459-
}
467+
hasContent( rangeOrElement, options ) {
468+
const range = rangeOrElement instanceof ModelElement ? ModelRange._createIn( rangeOrElement ) : rangeOrElement;
460469

461-
if ( rangeOrElement.isCollapsed ) {
470+
if ( range.isCollapsed ) {
462471
return false;
463472
}
464473

465-
for ( const item of rangeOrElement.getItems() ) {
466-
// Remember, `TreeWalker` returns always `textProxy` nodes.
467-
if ( item.is( 'textProxy' ) || this.schema.isObject( item ) ) {
474+
// Check if there are any markers which affects data in this given range.
475+
for ( const intersectingMarker of this.markers.getMarkersIntersectingRange( range ) ) {
476+
if ( intersectingMarker.affectsData ) {
477+
return true;
478+
}
479+
}
480+
481+
const { ignoreWhitespaces = false } = options || {};
482+
483+
for ( const item of range.getItems() ) {
484+
if ( item.is( 'textProxy' ) ) {
485+
if ( !ignoreWhitespaces ) {
486+
return true;
487+
} else if ( item.data.search( /\S/ ) !== -1 ) {
488+
return true;
489+
}
490+
} else if ( this.schema.isObject( item ) ) {
468491
return true;
469492
}
470493
}

‎tests/controller/datacontroller.js‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -346,15 +346,26 @@ describe( 'DataController', () => {
346346
downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
347347

348348
expect( data.get() ).to.equal( '<p>foo</p>' );
349+
expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo</p>' );
349350
} );
350351

351-
it( 'should get empty paragraph', () => {
352+
it( 'should trim empty paragraph by default', () => {
352353
schema.register( 'paragraph', { inheritAllFrom: '$block' } );
353354
setData( model, '<paragraph></paragraph>' );
354355

355356
downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
356357

357-
expect( data.get() ).to.equal( '<p>&nbsp;</p>' );
358+
expect( data.get() ).to.equal( '' );
359+
expect( data.get( { trim: 'empty' } ) ).to.equal( '' );
360+
} );
361+
362+
it( 'should get empty paragraph (with trim=none)', () => {
363+
schema.register( 'paragraph', { inheritAllFrom: '$block' } );
364+
setData( model, '<paragraph></paragraph>' );
365+
366+
downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
367+
368+
expect( data.get( { trim: 'none' } ) ).to.equal( '<p>&nbsp;</p>' );
358369
} );
359370

360371
it( 'should get two paragraphs', () => {
@@ -364,13 +375,15 @@ describe( 'DataController', () => {
364375
downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
365376

366377
expect( data.get() ).to.equal( '<p>foo</p><p>bar</p>' );
378+
expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo</p><p>bar</p>' );
367379
} );
368380

369381
it( 'should get text directly in root', () => {
370382
schema.extend( '$text', { allowIn: '$root' } );
371383
setData( model, 'foo' );
372384

373385
expect( data.get() ).to.equal( 'foo' );
386+
expect( data.get( { trim: 'empty' } ) ).to.equal( 'foo' );
374387
} );
375388

376389
it( 'should get paragraphs without bold', () => {
@@ -380,6 +393,7 @@ describe( 'DataController', () => {
380393
downcastHelpers.elementToElement( { model: 'paragraph', view: 'p' } );
381394

382395
expect( data.get() ).to.equal( '<p>foobar</p>' );
396+
expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foobar</p>' );
383397
} );
384398

385399
it( 'should get paragraphs with bold', () => {
@@ -390,6 +404,7 @@ describe( 'DataController', () => {
390404
downcastHelpers.attributeToElement( { model: 'bold', view: 'strong' } );
391405

392406
expect( data.get() ).to.equal( '<p>foo<strong>bar</strong></p>' );
407+
expect( data.get( { trim: 'empty' } ) ).to.equal( '<p>foo<strong>bar</strong></p>' );
393408
} );
394409

395410
it( 'should get root name as a parameter', () => {
@@ -403,13 +418,13 @@ describe( 'DataController', () => {
403418
downcastHelpers.attributeToElement( { model: 'bold', view: 'strong' } );
404419

405420
expect( data.get() ).to.equal( '<p>foo</p>' );
406-
expect( data.get( 'main' ) ).to.equal( '<p>foo</p>' );
407-
expect( data.get( 'title' ) ).to.equal( 'Bar' );
421+
expect( data.get( { rootName: 'main' } ) ).to.equal( '<p>foo</p>' );
422+
expect( data.get( { rootName: 'title' } ) ).to.equal( 'Bar' );
408423
} );
409424

410425
it( 'should throw an error when non-existent root is used', () => {
411426
expect( () => {
412-
data.get( 'nonexistent' );
427+
data.get( { rootName: 'nonexistent' } );
413428
} ).to.throw(
414429
CKEditorError,
415430
'datacontroller-get-non-existent-root: Attempting to get data from a non-existing root.'

‎tests/model/model.js‎

Lines changed: 142 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,9 @@ describe( 'Model', () => {
500500
isObject: true
501501
} );
502502
schema.extend( 'image', { allowIn: 'div' } );
503+
schema.register( 'listItem', {
504+
inheritAllFrom: '$block'
505+
} );
503506

504507
setData(
505508
model,
@@ -510,7 +513,10 @@ describe( 'Model', () => {
510513
'<paragraph>foo</paragraph>' +
511514
'<div>' +
512515
'<image></image>' +
513-
'</div>'
516+
'</div>' +
517+
'<listItem></listItem>' +
518+
'<listItem></listItem>' +
519+
'<listItem></listItem>'
514520
);
515521

516522
root = model.document.getRoot();
@@ -522,6 +528,34 @@ describe( 'Model', () => {
522528
expect( model.hasContent( pFoo ) ).to.be.true;
523529
} );
524530

531+
it( 'should return true if given element has text node (ignoreWhitespaces)', () => {
532+
const pFoo = root.getChild( 1 );
533+
534+
expect( model.hasContent( pFoo, { ignoreWhitespaces: true } ) ).to.be.true;
535+
} );
536+
537+
it( 'should return true if given element has text node containing spaces only', () => {
538+
const pEmpty = root.getChild( 0 ).getChild( 0 );
539+
540+
model.enqueueChange( 'transparent', writer => {
541+
// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
542+
writer.insertText( ' ', pEmpty, 'end' );
543+
} );
544+
545+
expect( model.hasContent( pEmpty ) ).to.be.true;
546+
} );
547+
548+
it( 'should false true if given element has text node containing spaces only (ignoreWhitespaces)', () => {
549+
const pEmpty = root.getChild( 0 ).getChild( 0 );
550+
551+
model.enqueueChange( 'transparent', writer => {
552+
// Model `setData()` method trims whitespaces so use writer here to insert whitespace only text.
553+
writer.insertText( ' ', pEmpty, 'end' );
554+
} );
555+
556+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
557+
} );
558+
525559
it( 'should return true if given element has element that is an object', () => {
526560
const divImg = root.getChild( 2 );
527561

@@ -571,6 +605,113 @@ describe( 'Model', () => {
571605

572606
expect( model.hasContent( range ) ).to.be.false;
573607
} );
608+
609+
it( 'should return false for empty list items', () => {
610+
const range = new ModelRange( ModelPosition._createAt( root, 3 ), ModelPosition._createAt( root, 6 ) );
611+
612+
expect( model.hasContent( range ) ).to.be.false;
613+
} );
614+
615+
it( 'should return false for empty element with marker (usingOperation=false, affectsData=false)', () => {
616+
const pEmpty = root.getChild( 0 ).getChild( 0 );
617+
618+
model.enqueueChange( 'transparent', writer => {
619+
// Insert marker.
620+
const range = ModelRange._createIn( pEmpty );
621+
writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
622+
} );
623+
624+
expect( model.hasContent( pEmpty ) ).to.be.false;
625+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
626+
} );
627+
628+
it( 'should return false for empty element with marker (usingOperation=true, affectsData=false)', () => {
629+
const pEmpty = root.getChild( 0 ).getChild( 0 );
630+
631+
model.enqueueChange( 'transparent', writer => {
632+
// Insert marker.
633+
const range = ModelRange._createIn( pEmpty );
634+
writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: false } );
635+
} );
636+
637+
expect( model.hasContent( pEmpty ) ).to.be.false;
638+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
639+
} );
640+
641+
it( 'should return false (ignoreWhitespaces) for empty text with marker (usingOperation=false, affectsData=false)', () => {
642+
const pEmpty = root.getChild( 0 ).getChild( 0 );
643+
644+
model.enqueueChange( 'transparent', writer => {
645+
// Insert empty text.
646+
const text = writer.createText( ' ', { bold: true } );
647+
writer.append( text, pEmpty );
648+
649+
// Insert marker.
650+
const range = ModelRange._createIn( pEmpty );
651+
writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
652+
} );
653+
654+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
655+
} );
656+
657+
it( 'should return true for empty text with marker (usingOperation=false, affectsData=false)', () => {
658+
const pEmpty = root.getChild( 0 ).getChild( 0 );
659+
660+
model.enqueueChange( 'transparent', writer => {
661+
// Insert empty text.
662+
const text = writer.createText( ' ', { bold: true } );
663+
writer.append( text, pEmpty );
664+
665+
// Insert marker.
666+
const range = ModelRange._createIn( pEmpty );
667+
writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: false } );
668+
} );
669+
670+
expect( model.hasContent( pEmpty ) ).to.be.true;
671+
} );
672+
673+
it( 'should return false for empty element with marker (usingOperation=false, affectsData=true)', () => {
674+
const pEmpty = root.getChild( 0 ).getChild( 0 );
675+
676+
model.enqueueChange( 'transparent', writer => {
677+
// Insert marker.
678+
const range = ModelRange._createIn( pEmpty );
679+
writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
680+
} );
681+
682+
expect( model.hasContent( pEmpty ) ).to.be.false;
683+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
684+
} );
685+
686+
it( 'should return false for empty element with marker (usingOperation=true, affectsData=true)', () => {
687+
const pEmpty = root.getChild( 0 ).getChild( 0 );
688+
689+
model.enqueueChange( 'transparent', writer => {
690+
// Insert marker.
691+
const range = ModelRange._createIn( pEmpty );
692+
writer.addMarker( 'comment1', { range, usingOperation: true, affectsData: true } );
693+
} );
694+
695+
expect( model.hasContent( pEmpty ) ).to.be.false;
696+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.false;
697+
} );
698+
699+
it( 'should return true (ignoreWhitespaces) for empty text with marker (usingOperation=false, affectsData=true)', () => {
700+
const pEmpty = root.getChild( 0 ).getChild( 0 );
701+
702+
model.enqueueChange( 'transparent', writer => {
703+
// Insert empty text.
704+
const text = writer.createText( ' ', { bold: true } );
705+
writer.append( text, pEmpty );
706+
707+
// Insert marker.
708+
const range = ModelRange._createIn( pEmpty );
709+
writer.addMarker( 'comment1', { range, usingOperation: false, affectsData: true } );
710+
} );
711+
712+
expect( model.hasContent( pEmpty ) ).to.be.true;
713+
expect( model.hasContent( pEmpty, { ignoreWhitespaces: true } ) ).to.be.true;
714+
} );
574715
} );
575716

576717
describe( 'createPositionFromPath()', () => {

0 commit comments

Comments
 (0)