Merge branch 'master' into t/ckeditor5/1096

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",

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 );
 	}
 
 	/**

src/model/model.js

@@ -446,26 +446,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;
 			}
 		}

src/model/operation/transform.js

@@ -306,7 +306,7 @@ export function transformSets( operationsA, operationsB, options ) {
 		originalOperationsBCount: operationsB.length
 	};
 
-	const contextFactory = new ContextFactory( options.document, options.useRelations );
+	const contextFactory = new ContextFactory( options.document, options.useRelations, options.forceWeakRemove );
 	contextFactory.setOriginalOperations( operationsA );
 	contextFactory.setOriginalOperations( operationsB );
 
@@ -386,13 +386,17 @@ class ContextFactory {
 	// @param {module:engine/model/document~Document} document Document which the operations change.
 	// @param {Boolean} useRelations Whether during transformation relations should be used (used during undo for
 	// better conflict resolution).
-	constructor( document, useRelations ) {
+	// @param {Boolean} [forceWeakRemove=false] If set to `false`, remove operation will be always stronger than move operation,
+	// so the removed nodes won't end up back in the document root. When set to `true`, context data will be used.
+	constructor( document, useRelations, forceWeakRemove = false ) {
 		// `model.History` instance which information about undone operations will be taken from.
 		this._history = document.history;
 
 		// Whether additional context should be used.
 		this._useRelations = useRelations;
 
+		this._forceWeakRemove = !!forceWeakRemove;
+
 		// For each operation that is created during transformation process, we keep a reference to the original operation
 		// which it comes from. The original operation works as a kind of "identifier". Every contextual information
 		// gathered during transformation that we want to save for given operation, is actually saved for the original operation.
@@ -583,7 +587,8 @@ class ContextFactory {
 			aWasUndone: this._wasUndone( opA ),
 			bWasUndone: this._wasUndone( opB ),
 			abRelation: this._useRelations ? this._getRelation( opA, opB ) : null,
-			baRelation: this._useRelations ? this._getRelation( opB, opA ) : null
+			baRelation: this._useRelations ? this._getRelation( opB, opA ) : null,
+			forceWeakRemove: this._forceWeakRemove
 		};
 	}
 
@@ -1313,7 +1318,7 @@ setTransformation( MergeOperation, MoveOperation, ( a, b, context ) => {
 	//
 	const removedRange = Range._createFromPositionAndShift( b.sourcePosition, b.howMany );
 
-	if ( b.type == 'remove' && !context.bWasUndone ) {
+	if ( b.type == 'remove' && !context.bWasUndone && !context.forceWeakRemove ) {
 		if ( a.deletionPosition.hasSameParentAs( b.sourcePosition ) && removedRange.containsPosition( a.sourcePosition ) ) {
 			return [ new NoOperation( 0 ) ];
 		}
@@ -1596,9 +1601,9 @@ setTransformation( MoveOperation, MoveOperation, ( a, b, context ) => {
 	//
 	// If only one of operations is a remove operation, we force remove operation to be the "stronger" one
 	// to provide more expected results.
-	if ( a.type == 'remove' && b.type != 'remove' && !context.aWasUndone ) {
+	if ( a.type == 'remove' && b.type != 'remove' && !context.aWasUndone && !context.forceWeakRemove ) {
 		aIsStrong = true;
-	} else if ( a.type != 'remove' && b.type == 'remove' && !context.bWasUndone ) {
+	} else if ( a.type != 'remove' && b.type == 'remove' && !context.bWasUndone && !context.forceWeakRemove ) {
 		aIsStrong = false;
 	}
 
@@ -1768,7 +1773,7 @@ setTransformation( MoveOperation, SplitOperation, ( a, b, context ) => {
 	if ( b.graveyardPosition ) {
 		const movesGraveyardElement = moveRange.start.isEqual( b.graveyardPosition ) || moveRange.containsPosition( b.graveyardPosition );
 
-		if ( a.howMany > 1 && movesGraveyardElement ) {
+		if ( a.howMany > 1 && movesGraveyardElement && !context.aWasUndone ) {
 			ranges.push( Range._createFromPositionAndShift( b.insertionPosition, 1 ) );
 		}
 	}
@@ -1780,7 +1785,7 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 	const movedRange = Range._createFromPositionAndShift( a.sourcePosition, a.howMany );
 
 	if ( b.deletionPosition.hasSameParentAs( a.sourcePosition ) && movedRange.containsPosition( b.sourcePosition ) ) {
-		if ( a.type == 'remove' ) {
+		if ( a.type == 'remove' && !context.forceWeakRemove ) {
 			// Case 1:
 			//
 			// The element to remove got merged.
@@ -1794,21 +1799,22 @@ setTransformation( MoveOperation, MergeOperation, ( a, b, context ) => {
 				const results = [];
 
 				let gyMoveSource = b.graveyardPosition.clone();
-				let splitNodesMoveSource = b.targetPosition.clone();
+				let splitNodesMoveSource = b.targetPosition._getTransformedByMergeOperation( b );
 
 				if ( a.howMany > 1 ) {
 					results.push( new MoveOperation( a.sourcePosition, a.howMany - 1, a.targetPosition, 0 ) );
-					gyMoveSource = gyMoveSource._getTransformedByInsertion( a.targetPosition, a.howMany - 1 );
+
+					gyMoveSource = gyMoveSource._getTransformedByMove( a.sourcePosition, a.targetPosition, a.howMany - 1 );
 					splitNodesMoveSource = splitNodesMoveSource._getTransformedByMove( a.sourcePosition, a.targetPosition, a.howMany - 1 );
 				}
 
 				const gyMoveTarget = b.deletionPosition._getCombined( a.sourcePosition, a.targetPosition );
 				const gyMove = new MoveOperation( gyMoveSource, 1, gyMoveTarget, 0 );
 
-				const targetPositionPath = gyMove.getMovedRangeStart().path.slice();
-				targetPositionPath.push( 0 );
+				const splitNodesMoveTargetPath = gyMove.getMovedRangeStart().path.slice();
+				splitNodesMoveTargetPath.push( 0 );
 
-				const splitNodesMoveTarget = new Position( gyMove.targetPosition.root, targetPositionPath );
+				const splitNodesMoveTarget = new Position( gyMove.targetPosition.root, splitNodesMoveTargetPath );
 				splitNodesMoveSource = splitNodesMoveSource._getTransformedByMove( gyMoveSource, gyMoveTarget, 1 );
 				const splitNodesMove = new MoveOperation( splitNodesMoveSource, b.howMany, splitNodesMoveTarget, 0 );
 
@@ -2052,7 +2058,9 @@ setTransformation( SplitOperation, MoveOperation, ( a, b, context ) => {
 		// is already moved to the correct position, we need to only move the nodes after the split position.
 		// This will be done by `MoveOperation` instead of `SplitOperation`.
 		//
-		if ( rangeToMove.start.isEqual( a.graveyardPosition ) || rangeToMove.containsPosition( a.graveyardPosition ) ) {
+		const gyElementMoved = rangeToMove.start.isEqual( a.graveyardPosition ) || rangeToMove.containsPosition( a.graveyardPosition );
+
+		if ( !context.bWasUndone && gyElementMoved ) {
 			const sourcePosition = a.splitPosition._getTransformedByMoveOperation( b );
 
 			const newParentPosition = a.graveyardPosition._getTransformedByMoveOperation( b );

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,13 +375,15 @@ 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', () => {
 			schema.extend( '$text', { allowIn: '$root' } );
 			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.'