8
0
Quellcode durchsuchen

Merge pull request #1083 from ckeditor/t/ckeditor5-typing/101

Fix: Multiple spaces in an empty paragraph are now allowed. Closes ckeditor/ckeditor5-typing#101.
Piotrek Koszuliński vor 8 Jahren
Ursprung
Commit
4fdde44226

+ 61 - 91
packages/ckeditor5-engine/src/view/domconverter.js

@@ -65,7 +65,7 @@ export default class DomConverter {
 		 *
 		 * @member {Array.<String>} module:engine/view/domconverter~DomConverter#preElements
 		 */
-		this.preElements = [ 'pre' ];
+		this.preElements = [ 'pre', 'code' ];
 
 		/**
 		 * Tag names of DOM `Element`s which are considered block elements.
@@ -177,7 +177,7 @@ export default class DomConverter {
 	 * @param {Document} domDocument Document which will be used to create DOM nodes.
 	 * @param {Object} [options] Conversion options.
 	 * @param {Boolean} [options.bind=false] Determines whether new elements will be bound.
-	 * @param {Boolean} [options.withChildren=true] If true node's and document fragment's children  will be converted too.
+	 * @param {Boolean} [options.withChildren=true] If `true`, node's and document fragment's children will be converted too.
 	 * @returns {Node|DocumentFragment} Converted node or DocumentFragment.
 	 */
 	viewToDom( viewNode, domDocument, options = {} ) {
@@ -885,21 +885,24 @@ export default class DomConverter {
 	}
 
 	/**
-	 * Takes text data from given {@link module:engine/view/text~Text#data} and processes it so it is correctly displayed in DOM.
+	 * Takes text data from a given {@link module:engine/view/text~Text#data} and processes it so
+	 * it is correctly displayed in the DOM.
 	 *
 	 * Following changes are done:
 	 *
-	 * * multiple spaces are replaced to a chain of spaces and `&nbsp;`,
-	 * * space at the beginning of the text node is changed to `&nbsp;` if it is a first text node in it's container
-	 * element or if previous text node ends by space character,
-	 * * space at the end of the text node is changed to `&nbsp;` if it is a last text node in it's container.
+	 * * a space at the beginning is changed to `&nbsp;` if this is the first text node in its container
+	 * element or if a previous text node ends with a space character,
+	 * * space at the end of the text node is changed to `&nbsp;` if this is the last text node in its container,
+	 * * remaining spaces are replaced to a chain of spaces and `&nbsp;` (e.g. `'x   x'` becomes `'x &nbsp; x'`).
+	 *
+	 * Content of {@link #preElements} is not processed.
 	 *
 	 * @private
 	 * @param {module:engine/view/text~Text} node View text node to process.
 	 * @returns {String} Processed text data.
 	 */
 	_processDataFromViewText( node ) {
-		const data = node.data;
+		let data = node.data;
 
 		// If any of node ancestors has a name which is in `preElements` array, then currently processed
 		// view text node is (will be) in preformatted element. We should not change whitespaces then.
@@ -907,105 +910,44 @@ export default class DomConverter {
 			return data;
 		}
 
-		const prevNode = this._getTouchingViewTextNode( node, false );
-		const nextNode = this._getTouchingViewTextNode( node, true );
-
-		// Second part of text data, from the space after the last non-space character to the end.
-		// We separate `textEnd` and `textStart` because `textEnd` needs some special handling.
-		let textEnd = data.match( / *$/ )[ 0 ];
-		// First part of data, between first and last part of data.
-		let textStart = data.substr( 0, data.length - textEnd.length );
-
-		// If previous text node does not exist or it ends by space character, replace space character at the beginning of text.
-		// ` x`			-> `_x`
-		// `  x`		-> `_ x`
-		// `   x`		-> `_  x`
-		if ( !prevNode || prevNode.data.charAt( prevNode.data.length - 1 ) == ' ' ) {
-			textStart = textStart.replace( /^ /, '\u00A0' );
+		// 1. Replace the first space with a nbsp if the previous node ends with a space or there is no previous node
+		// (container element boundary).
+		if ( data.charAt( 0 ) == ' ' ) {
+			const prevNode = this._getTouchingViewTextNode( node, false );
+			const prevEndsWithSpace = prevNode && this._nodeEndsWithSpace( prevNode );
+
+			if ( prevEndsWithSpace || !prevNode ) {
+				data = '\u00A0' + data.substr( 1 );
+			}
 		}
 
-		// Multiple consecutive spaces. Change them to ` &nbsp;` pairs.
-		// `_x  x`		-> `_x _x`
-		// `_ x  x`		-> `_ x _x`
-		// `_  x  x`	-> `_ _x _x`
-		// `_  x   x`	-> `_ _x _ x`
-		// `_  x    x`	-> `_ _x _ _x`
-		// `_   x    x` -> `_ _ x _ _x`
-		textStart = textStart.replace( / {2}/g, ' \u00A0' );
-
-		// Process `textEnd` only if there is anything to process.
-		if ( textEnd.length > 0 ) {
-			// (1) We need special treatment for the last part of text node, it has to end on `&nbsp;`, not space:
-			// `x `		-> `x_`
-			// `x  `	-> `x _`
-			// `x   `	-> `x_ _`
-			// `x    `	-> `x _ _`
-			// (2) Different case when there is a node after:
-			// `x <b>b</b>`		-> `x <b>b</b>`
-			// `x  <b>b</b>`	-> `x _<b>b</b>`
-			// `x   <b>b</b>`	-> `x _ <b>b</b>`
-			// `x    <b>b</b>`	-> `x _ _<b>b</b>`
-			// (3) But different, when that node starts by &nbsp; (or space that will be converted to &nbsp;):
-			// `x <b>_b</b>`	-> `x <b>_b</b>`
-			// `x  <b>_b</b>`	-> `x_ <b>_b</b>`
-			// `x   <b>_b</b>`	-> `x _ <b>_b</b>`
-			// `x    <b>_b</b>`	-> `x_ _ <b>_b</b>`
-			// Let's assume that starting from space is normal behavior, because starting from &nbsp; is a less frequent case.
-			let textEndStartsFromNbsp = false;
+		// 2. Replace the last space with a nbsp if this is the last text node (container element boundary).
+		if ( data.charAt( data.length - 1 ) == ' ' ) {
+			const nextNode = this._getTouchingViewTextNode( node, true );
 
 			if ( !nextNode ) {
-				// (1)
-				if ( textEnd.length % 2 ) {
-					textEndStartsFromNbsp = true;
-				}
-			} else if ( nextNode.data.charAt( 0 ) == ' ' || nextNode.data.charAt( 0 ) == '\u00A0' ) {
-				// (3)
-				if ( textEnd.length % 2 === 0 ) {
-					textEndStartsFromNbsp = true;
-				}
+				data = data.substr( 0, data.length - 1 ) + '\u00A0';
 			}
-
-			if ( textEndStartsFromNbsp ) {
-				textEnd = '\u00A0' + textEnd.substr( 0, textEnd.length - 1 );
-			}
-
-			textEnd = textEnd.replace( / {2}/g, ' \u00A0' );
 		}
 
-		return textStart + textEnd;
+		return data.replace( / {2}/g, ' \u00A0' );
 	}
 
 	/**
-	 * Helper function. For given {@link module:engine/view/text~Text view text node}, it finds previous or next sibling that is contained
-	 * in the same block element. If there is no such sibling, `null` is returned.
+	 * Checks whether given node ends with a space character after changing appropriate space characters to `&nbsp;`s.
 	 *
 	 * @private
-	 * @param {module:engine/view/text~Text} node
-	 * @param {Boolean} getNext
-	 * @returns {module:engine/view/text~Text}
+	 * @param {module:engine/view/text~Text} node Node to check.
+	 * @returns {Boolean} `true` if given `node` ends with space, `false` otherwise.
 	 */
-	_getTouchingViewTextNode( node, getNext ) {
-		if ( !node.parent ) {
-			return null;
+	_nodeEndsWithSpace( node ) {
+		if ( node.getAncestors().some( parent => this.preElements.includes( parent.name ) ) ) {
+			return false;
 		}
 
-		const treeWalker = new ViewTreeWalker( {
-			startPosition: getNext ? ViewPosition.createAfter( node ) : ViewPosition.createBefore( node ),
-			direction: getNext ? 'forward' : 'backward'
-		} );
+		const data = this._processDataFromViewText( node );
 
-		for ( const value of treeWalker ) {
-			if ( value.item.is( 'containerElement' ) ) {
-				// ViewContainerElement is found on a way to next ViewText node, so given `node` was first/last
-				// text node in it's container element.
-				return null;
-			} else if ( value.item.is( 'text' ) ) {
-				// Found a text node in the same container element.
-				return value.item;
-			}
-		}
-
-		return null;
+		return data.charAt( data.length - 1 ) == ' ';
 	}
 
 	/**
@@ -1075,6 +1017,34 @@ export default class DomConverter {
 		return data;
 	}
 
+	/**
+	 * Helper function. For given {@link module:engine/view/text~Text view text node}, it finds previous or next sibling
+	 * that is contained in the same container element. If there is no such sibling, `null` is returned.
+	 *
+	 * @param {module:engine/view/text~Text} node Reference node.
+	 * @param {Boolean} getNext
+	 * @returns {module:engine/view/text~Text|null} Touching text node or `null` if there is no next or previous touching text node.
+	 */
+	_getTouchingViewTextNode( node, getNext ) {
+		const treeWalker = new ViewTreeWalker( {
+			startPosition: getNext ? ViewPosition.createAfter( node ) : ViewPosition.createBefore( node ),
+			direction: getNext ? 'forward' : 'backward'
+		} );
+
+		for ( const value of treeWalker ) {
+			if ( value.item.is( 'containerElement' ) ) {
+				// ViewContainerElement is found on a way to next ViewText node, so given `node` was first/last
+				// text node in its container element.
+				return null;
+			} else if ( value.item.is( 'text' ) ) {
+				// Found a text node in the same container element.
+				return value.item;
+			}
+		}
+
+		return null;
+	}
+
 	/**
 	 * Helper function. For given `Text` node, it finds previous or next sibling that is contained in the same block element.
 	 * If there is no such sibling, `null` is returned.

+ 80 - 12
packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js

@@ -269,7 +269,7 @@ describe( 'DomConverter', () => {
 			// At the end.
 			test( 'x ', 'x_' );
 			test( 'x  ', 'x _' );
-			test( 'x   ', 'x_ _' );
+			test( 'x   ', 'x __' );
 			test( 'x    ', 'x _ _' );
 
 			// In the middle.
@@ -282,11 +282,19 @@ describe( 'DomConverter', () => {
 			test( ' x ', '_x_' );
 			test( '  x  x  ', '_ x _x _' );
 			test( '   x x  ', '_ _x x _' );
-			test( '   x x   ', '_ _x x_ _' );
+			test( '   x x   ', '_ _x x __' );
 			test( '   x    x ', '_ _x _ _x_' );
+
+			// Only spaces.
 			test( ' ', '_' );
+			test( '  ', '__' );
+			test( '   ', '_ _' );
+			test( '    ', '_ __' );
+			test( '     ', '_ _ _' );
+			test( '      ', '_ _ __' );
 
 			// With hard &nbsp;
+			// It should be treated like a normal sign.
 			test( '_x', '_x' );
 			test( ' _x', '__x' );
 			test( '  _x', '_ _x' );
@@ -323,33 +331,93 @@ describe( 'DomConverter', () => {
 
 			test( [ 'x', ' y' ], 'x y' );
 			test( [ 'x ', ' y' ], 'x _y' );
-			test( [ 'x  ', ' y' ], 'x_ _y' );
+			test( [ 'x  ', ' y' ], 'x _ y' );
 			test( [ 'x   ', ' y' ], 'x _ _y' );
-			test( [ 'x    ', ' y' ], 'x_ _ _y' );
+			test( [ 'x    ', ' y' ], 'x _ _ y' );
 
 			test( [ 'x', '_y' ], 'x_y' );
 			test( [ 'x ', '_y' ], 'x _y' );
-			test( [ 'x  ', '_y' ], 'x_ _y' );
+			test( [ 'x  ', '_y' ], 'x __y' );
 			test( [ 'x   ', '_y' ], 'x _ _y' );
-			test( [ 'x    ', '_y' ], 'x_ _ _y' );
+			test( [ 'x    ', '_y' ], 'x _ __y' );
 
 			test( [ 'x', '  y' ], 'x _y' );
 			test( [ 'x ', '  y' ], 'x _ y' );
-			test( [ 'x  ', '  y' ], 'x_ _ y' );
+			test( [ 'x  ', '  y' ], 'x _ _y' );
 			test( [ 'x   ', '  y' ], 'x _ _ y' );
-			test( [ 'x    ', '  y' ], 'x_ _ _ y' );
+			test( [ 'x    ', '  y' ], 'x _ _ _y' );
 
 			test( [ 'x', '   y' ], 'x _ y' );
 			test( [ 'x ', '   y' ], 'x _ _y' );
-			test( [ 'x  ', '   y' ], 'x_ _ _y' );
+			test( [ 'x  ', '   y' ], 'x _ _ y' );
 			test( [ 'x   ', '   y' ], 'x _ _ _y' );
-			test( [ 'x    ', '   y' ], 'x_ _ _ _y' );
+			test( [ 'x    ', '   y' ], 'x _ _ _ y' );
+
+			// "Non-empty" + "empty" text nodes.
+			test( [ 'x',	' '		], 'x_' );
+			test( [ 'x',	'  '	], 'x _' );
+			test( [ 'x',	'   '	], 'x __' );
+			test( [ 'x ',	' '		], 'x _' );
+			test( [ 'x ',	'  '	], 'x __' );
+			test( [ 'x ',	'   '	], 'x _ _' );
+			test( [ 'x  ',	' '		], 'x __' );
+			test( [ 'x  ',	'  '	], 'x _ _' );
+			test( [ 'x  ',	'   '	], 'x _ __' );
+			test( [ 'x   ',	' '		], 'x _ _' );
+			test( [ 'x   ',	'  '	], 'x _ __' );
+			test( [ 'x   ',	'   '	], 'x _ _ _' );
+
+			test( [ ' ',	'x'		], '_x' );
+			test( [ '  ',	'x'		], '_ x' );
+			test( [ '   ',	'x'		], '_ _x' );
+			test( [ ' ',	' x'	], '_ x' );
+			test( [ '  ',	' x'	], '_ _x' );
+			test( [ '   ',	' x'	], '_ _ x' );
+			test( [ ' ',	'  x'	], '_ _x' );
+			test( [ '  ',	'  x'	], '_ _ x' );
+			test( [ '   ',	'  x'	], '_ _ _x' );
+			test( [ ' ',	'   x'	], '_ _ x' );
+			test( [ '  ',	'   x'	], '_ _ _x' );
+			test( [ '   ',	'   x'	], '_ _ _ x' );
+
+			test( [ 'x',	' ',		'x'		],	'x x' );
+			test( [ 'x',	' ',		' x'	],	'x _x' );
+			test( [ 'x',	'  ',		' x'	],	'x _ x' );
+			test( [ 'x',	'   ',		'  x'	],	'x _ _ x' );
+			test( [ 'x ',	' ',		' x'	],	'x _ x' );
+			test( [ 'x ',	'  ',		' x'	],	'x _ _x' );
+			test( [ 'x ',	'   ',		'  x'	],	'x _ _ _x' );
+			test( [ 'x  ',	' ',		' x'	],	'x _ _x' );
+			test( [ 'x  ',	'  ',		' x'	],	'x _ _ x' );
+			test( [ 'x  ',	'   ',		'  x'	],	'x _ _ _ x' );
+			test( [ 'x   ',	' ',		' x'	],	'x _ _ x' );
+			test( [ 'x   ',	'  ',		' x'	],	'x _ _ _x' );
+			test( [ 'x   ',	'   ',		'  x'	],	'x _ _ _ _x' );
+
+			// "Empty" + "empty" text nodes.
+			test( [ ' ', ' ' ], '__' );
+			test( [ '  ', ' ' ], '_ _' );
+			test( [ '   ', ' ' ], '_ __' );
+			test( [ ' ', '  ' ], '_ _' );
+			test( [ ' ', '   ' ], '_ __' );
+			test( [ '  ', '  ' ], '_ __' );
+			test( [ '  ', '   ' ], '_ _ _' );
+			test( [ '   ', '  ' ], '_ _ _' );
+			test( [ '   ', '   ' ], '_ _ __' );
 
 			it( 'not in preformatted blocks', () => {
-				const viewDiv = new ViewContainerElement( 'pre', null, new ViewText( '   foo   ' ) );
+				const viewDiv = new ViewContainerElement( 'pre', null, [ new ViewText( '   foo   ' ), new ViewText( ' bar ' ) ] );
+				const domDiv = converter.viewToDom( viewDiv, document );
+
+				expect( domDiv.innerHTML ).to.equal( '   foo    bar ' );
+			} );
+
+			it( 'text node before in a preformatted node', () => {
+				const viewCode = new ViewAttributeElement( 'code', null, new ViewText( 'foo   ' ) );
+				const viewDiv = new ViewContainerElement( 'div', null, [ viewCode, new ViewText( ' bar' ) ] );
 				const domDiv = converter.viewToDom( viewDiv, document );
 
-				expect( domDiv.innerHTML ).to.equal( '   foo   ' );
+				expect( domDiv.innerHTML ).to.equal( '<code>foo   </code> bar' );
 			} );
 		} );
 	} );