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

Other: Changed how   are generated on view->DOM rendering.

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

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

@@ -889,7 +889,8 @@ export default class DomConverter {
 	 *
 	 * * a space at the beginning is changed to ` ` 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 ` ` if this is the last text node in its container,
+	 * * space at the end of the text node is changed to ` ` if there are two spaces at the end of a node or if next node
+	 * starts with a space,
 	 * * remaining spaces are replaced to a chain of spaces and ` ` (e.g. `'x   x'` becomes `'x   x'`).
 	 *
 	 * Content of {@link #preElements} is not processed.
@@ -918,15 +919,17 @@ export default class DomConverter {
 			}
 		}
 
-		// 2. Replace the last space with a nbsp if this is the last text node (container element boundary).
+		// 2. Replace the last space with nbsp if there are two spaces at the end or if the next node starts with space or there is no
+		// next node (container element boundary).
 		if ( data.charAt( data.length - 1 ) == ' ' ) {
 			const nextNode = this._getTouchingViewTextNode( node, true );
 
-			if ( !nextNode ) {
+			if ( data.charAt( data.length - 2 ) == ' ' || !nextNode || nextNode.data.charAt( 0 ) == ' ' ) {
 				data = data.substr( 0, data.length - 1 ) + '\u00A0';
 			}
 		}
 
+		// 3. Create space+nbsp pairs.
 		return data.replace( / {2}/g, ' \u00A0' );
 	}
 
@@ -955,7 +958,8 @@ export default class DomConverter {
 	 * * multiple whitespaces are replaced to a single space,
 	 * * space at the beginning of a text node is removed if it is the first text node in its container
 	 * element or if the previous text node ends with a space character,
-	 * * space at the end of the text node is removed, if it is the last text node in its container.
+	 * * space at the end of the text node is removed, if it is the last text node in its container,
+	 * * nbsps are converted to spaces.
 	 *
 	 * @param {Node} node DOM text node to process.
 	 * @returns {String} Processed data.
@@ -998,28 +1002,25 @@ export default class DomConverter {
 		data = getDataWithoutFiller( new Text( data ) );
 
 		// At this point we should have removed all whitespaces from DOM text data.
-
-		// Now we have to change   chars, that were in DOM text data because of rendering reasons, to spaces.
-		// First, change all ` \u00A0` pairs (space +  ) 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.
+		//
+		// Now, We will reverse the process that happens in `_processDataFromViewText`.
+		//
+		// We have to change   chars, that were in DOM text data because of rendering reasons, to spaces.
+		// First, change all ` \u00A0` pairs (space +  ) to two spaces. DOM converter changes two spaces from model/view to
+		// ` \u00A0` to ensure proper rendering. Since here we convert back, we recognize those pairs and change them back to `  `.
 		data = data.replace( / \u00A0/g, '  ' );
 
+		// Then, let's change the last nbsp to a space.
+		if ( /( |\u00A0)\u00A0$/.test( data ) || !nextNode || ( nextNode.data && nextNode.data.charAt( 0 ) == ' ' ) ) {
+			data = data.replace( /\u00A0$/, ' ' );
+		}
+
 		// Then, change   character that is at the beginning of the text node to space character.
-		// As above, that   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 ( shouldLeftTrim ) {
 			data = data.replace( /^\u00A0/, ' ' );
 		}
 
-		// Since input text data could be: `x_ _`, we would not replace the first   after `x` character.
-		// We have to fix it. Since we already change all `  `, we will have something like this at the end of text data:
-		// `x_ _ _` -> `x_    `. Find   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 with &nbsp; or is a <br>.
-		if ( isText( nextNode ) ? nextNode.data.charAt( 0 ) == '\u00A0' : true ) {
-			data = data.replace( /\u00A0( *)$/, ' $1' );
-		}
-
 		// 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;
@@ -1048,7 +1049,7 @@ export default class DomConverter {
 	 * be trimmed from the right side.
 	 *
 	 * @param {Node} node
-	 * @param {Node} prevNode
+	 * @param {Node} nextNode
 	 */
 	_checkShouldRightTrimDomText( node, nextNode ) {
 		if ( nextNode ) {

+ 14 - 19
packages/ckeditor5-engine/tests/view/domconverter/dom-to-view.js

@@ -451,7 +451,7 @@ describe( 'DomConverter', () => {
 			// At the end.
 			test( 'x_', 'x ' );
 			test( 'x _', 'x  ' );
-			test( 'x_ _', 'x   ' );
+			test( 'x __', 'x   ' );
 			test( 'x _ _', 'x    ' );
 
 			// In the middle.
@@ -464,7 +464,7 @@ 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 ' );
 			test( '_', ' ' );
 
@@ -477,10 +477,6 @@ describe( 'DomConverter', () => {
 			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' );
@@ -496,26 +492,25 @@ describe( 'DomConverter', () => {
 			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' );
 
 			// 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' );
 			test( [ 'x_ _', ' y' ], 'x_   y' );
 
 			it( 'not in preformatted blocks', () => {
@@ -605,9 +600,9 @@ describe( 'DomConverter', () => {
 				expect( viewP.getChild( 0 ).data ).to.equal( 'foo ' );
 			} );
 
-			it( 'not before a <br> (nbsp+space)', () => {
+			it( 'not before a <br> (space+nbsp)', () => {
 				const domP = createElement( document, 'p', {}, [
-					document.createTextNode( 'foo\u00a0 ' ),
+					document.createTextNode( 'foo \u00a0' ),
 					createElement( document, 'br' )
 				] );
 

+ 35 - 34
packages/ckeditor5-engine/tests/view/domconverter/view-to-dom.js

@@ -212,7 +212,7 @@ describe( 'DomConverter', () => {
 
 				const domDiv = converter.viewToDom( viewDiv, document );
 
-				expect( domDiv.innerHTML ).to.equal( 'x &nbsp;x &nbsp; x x <b>&nbsp;x </b><i><b><u>&nbsp;x</u></b></i>' );
+				expect( domDiv.innerHTML ).to.equal( 'x &nbsp;x &nbsp; x x&nbsp;<b> x&nbsp;</b><i><b><u> x</u></b></i>' );
 			} );
 
 			it( 'all together', () => {
@@ -232,7 +232,7 @@ describe( 'DomConverter', () => {
 				const domDiv = converter.viewToDom( viewDiv, document );
 
 				expect( domDiv.innerHTML ).to.equal(
-					'<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&nbsp;<b> x&nbsp;</b><i><b><u> x&nbsp;</u></b></i></p><p>&nbsp; x &nbsp;</p>'
 				);
 			} );
 
@@ -321,86 +321,87 @@ describe( 'DomConverter', () => {
 			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' );
+
+			// Two text nodes.
+			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' );
 			test( [ ' ',	'  x'	], '_ _x' );
-			test( [ '  ',	'  x'	], '_ _ x' );
+			test( [ '  ',	'  x'	], '__ _x' );
 			test( [ '   ',	'  x'	], '_ _ _x' );
 			test( [ ' ',	'   x'	], '_ _ x' );
-			test( [ '  ',	'   x'	], '_ _ _x' );
+			test( [ '  ',	'   x'	], '__ _ x' );
 			test( [ '   ',	'   x'	], '_ _ _ x' );
 
+			// "Non-empty" + "empty" text nodes.
 			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' );
+			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( [ '  ', '  ' ], '__ _' );
+			test( [ '  ', '   ' ], '__ __' );
 			test( [ '   ', '  ' ], '_ _ _' );
 			test( [ '   ', '   ' ], '_ _ __' );
 

+ 5 - 14
packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js

@@ -165,20 +165,20 @@ describe( 'DomConverter – whitespace handling – integration', () => {
 			return editor.destroy();
 		} );
 
-		it( 'single spaces around <br>', () => {
+		it( 'single spaces around <br> #1', () => {
 			editor.setData( '<p>foo&nbsp;<br>&nbsp;bar</p>' );
 
 			expect( getData( editor.model, { withoutSelection: true } ) )
-				.to.equal( '<paragraph>foo <softBreak></softBreak> bar</paragraph>' );
+				.to.equal( '<paragraph>foo\u00A0<softBreak></softBreak> bar</paragraph>' );
 
 			expect( editor.getData() ).to.equal( '<p>foo&nbsp;<br>&nbsp;bar</p>' );
 		} );
 
-		it( 'single spaces around <br> (normalization)', () => {
+		it( 'single spaces around <br> #2', () => {
 			editor.setData( '<p>foo&nbsp;<br> bar</p>' );
 
 			expect( getData( editor.model, { withoutSelection: true } ) )
-				.to.equal( '<paragraph>foo <softBreak></softBreak>bar</paragraph>' );
+				.to.equal( '<paragraph>foo\u00A0<softBreak></softBreak>bar</paragraph>' );
 
 			expect( editor.getData() ).to.equal( '<p>foo&nbsp;<br>bar</p>' );
 		} );
@@ -192,16 +192,7 @@ describe( 'DomConverter – whitespace handling – integration', () => {
 			expect( editor.getData() ).to.equal( '<p>foo &nbsp;<br>bar</p>' );
 		} );
 
-		it( 'two spaces before a <br> (normalization)', () => {
-			editor.setData( '<p>foo&nbsp; <br>bar</p>' );
-
-			expect( getData( editor.model, { withoutSelection: true } ) )
-				.to.equal( '<paragraph>foo  <softBreak></softBreak>bar</paragraph>' );
-
-			expect( editor.getData() ).to.equal( '<p>foo &nbsp;<br>bar</p>' );
-		} );
-
-		it( 'two spaces before a <br> (normalization to a model nbsp)', () => {
+		it( 'two nbsps before a <br>', () => {
 			editor.setData( '<p>foo&nbsp;&nbsp;<br>bar</p>' );
 
 			expect( getData( editor.model, { withoutSelection: true } ) )

+ 13 - 13
packages/ckeditor5-engine/tests/view/renderer.js

@@ -1372,7 +1372,7 @@ describe( 'Renderer', () => {
 			expect( domFocusSpy.called ).to.be.false;
 		} );
 
-		it( 'should render NBSP as first space in inline element after another space', () => {
+		it( 'should render NBSP as last space in the previous inline element', () => {
 			const viewP = parse( '<container:p>x <attribute:b>y</attribute:b></container:p>' );
 
 			viewRoot._appendChild( viewP );
@@ -1395,9 +1395,9 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewP );
 			renderer.render();
 
-			expect( domP.childNodes[ 0 ].data ).to.equal( 'x ' );
-			expect( domB.childNodes[ 0 ].data ).to.equal( '\u00A0y' );
-			expect( domP.innerHTML ).to.equal( 'x <b>&nbsp;y</b>' );
+			expect( domP.childNodes[ 0 ].data ).to.equal( 'x\u00A0' );
+			expect( domB.childNodes[ 0 ].data ).to.equal( ' y' );
+			expect( domP.innerHTML ).to.equal( 'x&nbsp;<b> y</b>' );
 		} );
 
 		it( 'should update sibling after, when node before is removed', () => {
@@ -1473,9 +1473,9 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewP );
 			renderer.render();
 
-			expect( domP.childNodes[ 0 ].data ).to.equal( 'x ' );
-			expect( domB.childNodes[ 0 ].data ).to.equal( '\u00A0y' );
-			expect( domP.innerHTML ).to.equal( 'x <b>&nbsp;y</b>' );
+			expect( domP.childNodes[ 0 ].data ).to.equal( 'x\u00A0' );
+			expect( domB.childNodes[ 0 ].data ).to.equal( ' y' );
+			expect( domP.innerHTML ).to.equal( 'x&nbsp;<b> y</b>' );
 		} );
 
 		// #1093
@@ -1502,9 +1502,9 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewP );
 			renderer.render();
 
-			expect( domB.childNodes[ 0 ].data ).to.equal( 'x ' );
-			expect( domP.childNodes[ 1 ].data ).to.equal( '\u00A0y' );
-			expect( domP.innerHTML ).to.equal( '<b>x </b>&nbsp;y' );
+			expect( domB.childNodes[ 0 ].data ).to.equal( 'x\u00A0' );
+			expect( domP.childNodes[ 1 ].data ).to.equal( ' y' );
+			expect( domP.innerHTML ).to.equal( '<b>x&nbsp;</b> y' );
 		} );
 
 		// #1093
@@ -1532,9 +1532,9 @@ describe( 'Renderer', () => {
 			renderer.markToSync( 'children', viewP );
 			renderer.render();
 
-			expect( domB.childNodes[ 0 ].data ).to.equal( 'x ' );
-			expect( domI.childNodes[ 0 ].data ).to.equal( '\u00A0y' );
-			expect( domP.innerHTML ).to.equal( '<b>x </b><i>&nbsp;y</i>' );
+			expect( domB.childNodes[ 0 ].data ).to.equal( 'x\u00A0' );
+			expect( domI.childNodes[ 0 ].data ).to.equal( ' y' );
+			expect( domP.innerHTML ).to.equal( '<b>x&nbsp;</b><i> y</i>' );
 		} );
 
 		// #1125