8
0
Просмотр исходного кода

Changed: DomConverter improvements in handling &nbsp; and spaces in DOM<->view conversion.

Szymon Cofalik 9 лет назад
Родитель
Сommit
e53b097603

+ 89 - 20
packages/ckeditor5-engine/src/view/domconverter.js

@@ -699,23 +699,69 @@ export default class DomConverter {
 		const prevNode = this._getTouchingViewTextNode( node, false );
 		const prevNode = this._getTouchingViewTextNode( node, false );
 		const nextNode = this._getTouchingViewTextNode( node, true );
 		const nextNode = this._getTouchingViewTextNode( node, true );
 
 
-		// If previous text node does not exist or it ends by space character...
+		// 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 ) == ' ' ) {
 		if ( !prevNode || prevNode.data.charAt( prevNode.data.length - 1 ) == ' ' ) {
-			// Replace space character at the beginning of this string by &nbsp;.
-			data = data.replace( /^ /, '\u00A0' );
+			textStart = textStart.replace( /^ /, '\u00A0' );
 		}
 		}
 
 
-		// If next text node does not exist...
-		if ( !nextNode ) {
-			// Replace space character at the end of this string by &nbsp;.
-			data = data.replace( / $/, '\u00A0' );
-		}
-		// If the text node exist, it will be later processed too.
+		// 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( /  /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;
+
+			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;
+				}
+			}
+
+			if ( textEndStartsFromNbsp ) {
+				textEnd = '\u00A0' + textEnd.substr( 0, textEnd.length - 1 );
+			}
 
 
-		// Multiple spaces.
-		data = data.replace( /  /g, ' \u00A0' );
+			textEnd = textEnd.replace( /  /g, ' \u00A0' );
+		}
 
 
-		return data;
+		return textStart + textEnd;
 	}
 	}
 
 
 	/**
 	/**
@@ -771,24 +817,47 @@ export default class DomConverter {
 			return data;
 			return data;
 		}
 		}
 
 
+		// Change all consecutive whitespace characters to a single space character. That's how multiple whitespaces
+		// are treated when rendered, so we normalize those whitespaces.
+		// Note that &nbsp; (`\u00A0`) should not be treated as a whitespace because it is rendered.
 		data = data.replace( /[^\S\u00A0]{2,}/g, ' ' );
 		data = data.replace( /[^\S\u00A0]{2,}/g, ' ' );
 
 
 		const prevNode = this._getTouchingDomTextNode( node, false );
 		const prevNode = this._getTouchingDomTextNode( node, false );
 		const nextNode = this._getTouchingDomTextNode( node, true );
 		const nextNode = this._getTouchingDomTextNode( node, true );
 
 
-		// If previous text node does not exist or it ends by space character...
-		if ( !prevNode || prevNode.data.charAt( prevNode.data.length - 1 ) == ' ' ) {
-			// Remove space character from the beginning of this string.
-			data = data.replace( /^[^\S\u00A0]/, '' );
+		// If previous dom text node does not exist or it ends by whitespace character, remove space character from the beginning
+		// of this text node. Such space character is treated as a whitespace.
+		if ( !prevNode || /[^\S\u00A0]/.test( prevNode.data.charAt( prevNode.data.length - 1 ) ) ) {
+			data = data.replace( /^ /, '' );
 		}
 		}
 
 
-		// If next text node does not exist...
+		// If next text node does not exist remove space character from the end of this text node.
 		if ( !nextNode ) {
 		if ( !nextNode ) {
-			// Remove space character from the end of this string.
-			data = data.replace( /[^\S\u00A0]$/, '' );
+			data = data.replace( / $/, '' );
+		}
+		// At this point we should have removed all whitespaces from DOM text data.
+
+		// Now we have to change &nbsp; chars, that were in DOM text data because of rendering reasons, to spaces.
+		// First, change all ` \u00A0` pairs (space + &nbsp;) to two spaces. DOM converter changes two spaces from model/view as
+		// ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them
+		// to `  ` which is what we expect to have in model/view.
+		data = data.replace( / \u00A0/g, '  ' );
+		// Then, change &nbsp; character that is at the beginning of the text node to space character.
+		// As above, that &nbsp; was created for rendering reasons but it's real meaning is just a space character.
+		// We do that replacement only if this is the first node or the previous node ends on whitespace character.
+		if ( !prevNode || /[^\S\u00A0]/.test( prevNode.data.charAt( prevNode.data.length - 1 ) ) ) {
+			data = data.replace( /^\u00A0/, ' ' );
+		}
+		// Since input text data could be: `x_ _`, we would not replace the first &nbsp; after `x` character.
+		// We have to fix it. Since we already change all ` &nbsp;`, we will have something like this at the end of text data:
+		// `x_ _ _` -> `x_    `. Find &nbsp; at the end of string (can be followed only by spaces).
+		// We do that replacement only if this is the last node or the next node starts by &nbsp;.
+		if ( !nextNode || nextNode.data.charAt( 0 ) == '\u00A0' ) {
+			data = data.replace( /\u00A0( *)$/, ' $1' );
 		}
 		}
-		// If the text node exist, it will be later processed too.
 
 
+		// At this point, all whitespaces should be removed and all &nbsp; created for rendering reasons should be
+		// changed to normal space. All left &nbsp; are &nbsp; inserted intentionally.
 		return data;
 		return data;
 	}
 	}
 
 

+ 100 - 0
packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js

@@ -221,6 +221,106 @@ describe( 'DomConverter', () => {
 				expect( viewDiv.getChild( 1 ).getChild( 0 ).data ).to.equal( 'fo o' );
 				expect( viewDiv.getChild( 1 ).getChild( 0 ).data ).to.equal( 'fo o' );
 			} );
 			} );
 
 
+			it( 'spaces in a text node', () => {
+				function test( inputTexts, output ) {
+					const domElement = createElement( document, 'div', {}, [] );
+
+					if ( typeof inputTexts == 'string' ) {
+						inputTexts = [ inputTexts ];
+					}
+
+					for ( let text of inputTexts ) {
+						domElement.appendChild( document.createTextNode( text.replace( /_/g, '\u00A0' ) ) );
+					}
+
+					const viewElement = converter.domToView( domElement );
+
+					let data = '';
+
+					for ( let child of viewElement.getChildren() ) {
+						data += child.data.replace( /\u00A0/g, '_' );
+					}
+
+					expect( data ).to.equal( output );
+				}
+
+				// At the beginning.
+				test( '_x', ' x' );
+				test( '_ x', '  x' );
+				test( '_ _x', '   x' );
+				test( '_ _ x', '    x' );
+
+				// At the end.
+				test( 'x_', 'x ' );
+				test( 'x _', 'x  ' );
+				test( 'x_ _', 'x   ' );
+				test( 'x _ _', 'x    ' );
+
+				// In the middle.
+				test( 'x x', 'x x' );
+				test( 'x _x', 'x  x' );
+				test( 'x _ x', 'x   x' );
+				test( 'x _ _x', 'x    x' );
+
+				// Complex.
+				test( '_x_', ' x ' );
+				test( '_ x _x _', '  x  x  ' );
+				test( '_ _x x _', '   x x  ' );
+				test( '_ _x x_ _', '   x x   ' );
+				test( '_ _x _ _x_', '   x    x ' );
+				test( '_', ' ' );
+
+				// With hard &nbsp;
+				test( '_x', ' x' );
+				test( '__x', ' _x' );
+				test( '___x', ' __x' );
+				test( '__ x', ' _ x' );
+
+				test( 'x_', 'x ' );
+				test( 'x__', 'x_ ' );
+				test( 'x___', 'x__ ' );
+				// This is an edge case, but it's impossible to write elegant and compact algorithm that is also
+				// 100% correct. We might assume that expected result is `x  _` but it will be converted to `x   `
+				// by the algorithm. This is acceptable, though.
+				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' );
+
+				// Two text nodes.
+				test( [ 'x', 'y' ], 'xy' );
+				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' );
+
+				// Some tests with hard &nbsp;
+				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' );
+			} );
+
 			it( 'not in preformatted blocks', () => {
 			it( 'not in preformatted blocks', () => {
 				const domDiv = createElement( document, 'div', {}, [
 				const domDiv = createElement( document, 'div', {}, [
 					createElement( document, 'pre', {}, [
 					createElement( document, 'pre', {}, [

+ 111 - 0
packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js

@@ -240,6 +240,117 @@ describe( 'DomConverter', () => {
 					'<p>&nbsp;x &nbsp;x &nbsp; x x <b>&nbsp;x </b><i><b><u>&nbsp;x&nbsp;</u></b></i></p><p>&nbsp; x &nbsp;</p>'
 					'<p>&nbsp;x &nbsp;x &nbsp; x x <b>&nbsp;x </b><i><b><u>&nbsp;x&nbsp;</u></b></i></p><p>&nbsp; x &nbsp;</p>'
 				);
 				);
 			} );
 			} );
+
+			it( 'spaces in a text node', () => {
+				function test( inputTexts, output ) {
+					const viewElement = new ViewContainerElement( 'p' );
+
+					if ( typeof inputTexts == 'string' ) {
+						inputTexts = [ inputTexts ];
+					}
+
+					for ( let text of inputTexts ) {
+						viewElement.appendChildren( new ViewText( text.replace( /_/g, '\u00A0' ) ) );
+					}
+
+					const domElement = converter.viewToDom( viewElement, document );
+					const data = domElement.innerHTML.replace( /&nbsp;/g, '_' );
+
+					expect( data ).to.equal( output );
+				}
+
+				// At the beginning.
+				test( ' x', '_x' );
+				test( '  x', '_ x' );
+				test( '   x', '_ _x' );
+				test( '    x', '_ _ x' );
+
+				// At the end.
+				test( 'x ', 'x_' );
+				test( 'x  ', 'x _' );
+				test( 'x   ', 'x_ _' );
+				test( 'x    ', 'x _ _' );
+
+				// In the middle.
+				test( 'x x', 'x x' );
+				test( 'x  x', 'x _x' );
+				test( 'x   x', 'x _ x' );
+				test( 'x    x', 'x _ _x' );
+
+				// Complex.
+				test( ' x ', '_x_' );
+				test( '  x  x  ', '_ x _x _' );
+				test( '   x x  ', '_ _x x _' );
+				test( '   x x   ', '_ _x x_ _' );
+				test( '   x    x ', '_ _x _ _x_' );
+				test( ' ', '_' );
+
+				// With hard &nbsp;
+				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' );
+
+				// Two text nodes.
+				test( [ 'x', 'y' ], 'xy' );
+				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' );
+			} );
+
+			it( 'not in preformatted blocks', () => {
+				const viewDiv = new ViewContainerElement( 'pre', null, new ViewText( '   foo   ' ) );
+				const domDiv = converter.viewToDom( viewDiv, document );
+
+				expect( domDiv.innerHTML ).to.equal( '   foo   ' );
+			} );
 		} );
 		} );
 	} );
 	} );