Sfoglia il codice sorgente

Merge branch 'master' into t/922

Piotrek Koszuliński 8 anni fa
parent
commit
0a70abd46b

+ 7 - 4
packages/ckeditor5-engine/src/view/domconverter.js

@@ -982,10 +982,11 @@ export default class DomConverter {
 			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   (`\u00A0`) should not be treated as a whitespace because it is rendered.
-		data = data.replace( /[^\S\u00A0]{2,}/g, ' ' );
+		// Change all consecutive whitespace characters (from the [ \n\t\r] set –
+		// see https://github.com/ckeditor/ckeditor5-engine/issues/822#issuecomment-311670249) to a single space character.
+		// That's how multiple whitespaces are treated when rendered, so we normalize those whitespaces.
+		// We're replacing 1+ (and not 2+) to also normalize singular \n\t\r characters (#822).
+		data = data.replace( /[ \n\t\r]{1,}/g, ' ' );
 
 		const prevNode = this._getTouchingDomTextNode( node, false );
 		const nextNode = this._getTouchingDomTextNode( node, true );
@@ -1007,12 +1008,14 @@ export default class DomConverter {
 		// ` \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   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 ( !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   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).

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

@@ -212,6 +212,111 @@ describe( 'DomConverter', () => {
 				expect( viewDiv.getChild( 1 ).getChild( 0 ).data ).to.equal( 'foo' );
 			} );
 
+			it( 'after a block element', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'foo' )
+					] ),
+					document.createTextNode( ' ' )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
+			it( 'after a block element (new line)', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'foo' )
+					] ),
+					document.createTextNode( '\n' )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
+			it( 'after a block element (carriage return)', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'foo' )
+					] ),
+					document.createTextNode( '\r' )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
+			it( 'after a block element (tab)', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'foo' )
+					] ),
+					document.createTextNode( '\t' )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
+			// See https://github.com/ckeditor/ckeditor5-engine/issues/822#issuecomment-311670249
+			it( 'but preserve all except " \\n\\r\\t"', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'x\fx\vx\u00a0x\u1680x\u2000x\u200ax\u2028x\u2029x\u202fx\u205fx\u3000x\ufeffx' )
+					] ),
+					createElement( document, 'p', {}, [
+						// x<two spaces>x because it behaved differently than "x<space>x" when I've been fixing this
+						document.createTextNode( 'x\f\vx\u00a0\u1680x\u2000\u200ax\u2028\u2029x\u202f\u205fx\u3000\ufeffx' )
+					] )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 2 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data )
+					.to.equal( 'x\fx\vx\u00a0x\u1680x\u2000x\u200ax\u2028x\u2029x\u202fx\u205fx\u3000x\ufeffx' );
+				expect( viewDiv.getChild( 1 ).getChild( 0 ).data )
+					.to.equal( 'x\f\vx\u00a0\u1680x\u2000\u200ax\u2028\u2029x\u202f\u205fx\u3000\ufeffx' );
+			} );
+
+			it( 'before a block element', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					document.createTextNode( ' ' ),
+					createElement( document, 'p', {}, [
+						document.createTextNode( ' foo' )
+					] )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
+			it( 'before a block element (new line)', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					document.createTextNode( '\n' ),
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'foo' )
+					] )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'foo' );
+			} );
+
 			it( 'multiple consecutive whitespaces changed to one', () => {
 				const domDiv = createElement( document, 'div', {}, [
 					createElement( document, 'p', {}, [
@@ -228,6 +333,21 @@ describe( 'DomConverter', () => {
 				expect( viewDiv.getChild( 1 ).getChild( 0 ).data ).to.equal( 'fo o' );
 			} );
 
+			it( 'multiple consecutive whitespaces changed to one (tab, new line, carriage return)', () => {
+				const domDiv = createElement( document, 'div', {}, [
+					document.createTextNode( '\n\n \t\r\n' ),
+					createElement( document, 'p', {}, [
+						document.createTextNode( 'f\n\t\r\n\to\n\n\no' )
+					] ),
+					document.createTextNode( '\n\n \t\r\n' )
+				] );
+
+				const viewDiv = converter.domToView( domDiv );
+
+				expect( viewDiv.childCount ).to.equal( 1 );
+				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( 'f o o' );
+			} );
+
 			function test( inputTexts, output ) {
 				if ( typeof inputTexts == 'string' ) {
 					inputTexts = [ inputTexts ];
@@ -339,6 +459,10 @@ describe( 'DomConverter', () => {
 
 				expect( viewDiv.getChild( 0 ).getChild( 0 ).data ).to.equal( '   foo\n   foo  ' );
 			} );
+
+			//
+			// See also whitespace-handling-integration.js.
+			//
 		} );
 	} );
 
@@ -602,6 +726,8 @@ describe( 'DomConverter', () => {
 
 			expect( viewSelection.rangeCount ).to.equal( 1 );
 			expect( stringify( viewP, viewSelection.getFirstRange() ) ).to.equal( '<p>f{oo<b>ba}r</b></p>' );
+
+			domP.remove();
 		} );
 
 		it( 'should convert empty selection to empty selection', () => {
@@ -638,6 +764,8 @@ describe( 'DomConverter', () => {
 			expect( viewSelection.anchor.offset ).to.equal( 2 );
 			expect( viewSelection.focus.offset ).to.equal( 1 );
 			expect( viewSelection.isBackward ).to.be.true;
+
+			domP.remove();
 		} );
 
 		it( 'should not add null ranges', () => {
@@ -659,6 +787,8 @@ describe( 'DomConverter', () => {
 			const viewSelection = converter.domSelectionToView( domSelection );
 
 			expect( viewSelection.rangeCount ).to.equal( 0 );
+
+			domP.remove();
 		} );
 
 		it( 'should return fake selection', () => {
@@ -679,6 +809,8 @@ describe( 'DomConverter', () => {
 			const bindViewSelection = converter.domSelectionToView( domSelection );
 
 			expect( bindViewSelection.isEqual( viewSelection ) ).to.be.true;
+
+			domContainer.remove();
 		} );
 
 		it( 'should return fake selection if selection is placed inside text node', () => {
@@ -699,6 +831,8 @@ describe( 'DomConverter', () => {
 			const bindViewSelection = converter.domSelectionToView( domSelection );
 
 			expect( bindViewSelection.isEqual( viewSelection ) ).to.be.true;
+
+			domContainer.remove();
 		} );
 	} );
 } );

+ 148 - 0
packages/ckeditor5-engine/tests/view/domconverter/whitespace-handling-integration.js

@@ -0,0 +1,148 @@
+/**
+ * @license Copyright (c) 2003-2017, CKSource - Frederico Knabben. All rights reserved.
+ * For licensing, see LICENSE.md.
+ */
+
+import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
+
+import { getData } from '../../../src/dev-utils/model';
+
+describe( 'DomConverter – whitespace handling – integration', () => {
+	let editor;
+
+	// See https://github.com/ckeditor/ckeditor5-engine/issues/822.
+	describe( 'data loading', () => {
+		beforeEach( () => {
+			return VirtualTestEditor
+				.create( { plugins: [ Paragraph ] } )
+				.then( newEditor => {
+					editor = newEditor;
+				} );
+		} );
+
+		afterEach( () => {
+			return editor.destroy();
+		} );
+
+		it( 'new line at the end of the content is ignored', () => {
+			editor.setData( '<p>foo</p>\n' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'whitespaces at the end of the content are ignored', () => {
+			editor.setData( '<p>foo</p>\n\r\n \t' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		// Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987.
+		it( 'nbsp at the end of the content is not ignored', () => {
+			editor.setData( '<p>foo</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'new line at the beginning of the content is ignored', () => {
+			editor.setData( '\n<p>foo</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'whitespaces at the beginning of the content are ignored', () => {
+			editor.setData( '\n\n \t<p>foo</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		// Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987.
+		it( 'nbsp at the beginning of the content is not ignored', () => {
+			editor.setData( '<p>foo</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'new line between blocks is ignored', () => {
+			editor.setData( '<p>foo</p>\n<p>bar</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
+		} );
+
+		it( 'whitespaces between blocks are ignored', () => {
+			editor.setData( '<p>foo</p>\n\n \t<p>bar</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
+		} );
+
+		// Controversial result. See https://github.com/ckeditor/ckeditor5-engine/issues/987.
+		it( 'nbsp between blocks is not ignored', () => {
+			editor.setData( '<p>foo</p>&nbsp;<p>bar</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
+		} );
+
+		it( 'new lines inside blocks are ignored', () => {
+			editor.setData( '<p>\nfoo\n</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'whitespaces inside blocks are ignored', () => {
+			editor.setData( '<p>\n\n \tfoo\n\n \t</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p>' );
+		} );
+
+		it( 'nbsp inside blocks are not ignored', () => {
+			editor.setData( '<p>&nbsp;foo&nbsp;</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph> foo </paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>&nbsp;foo&nbsp;</p>' );
+		} );
+
+		it( 'all whitespaces together are ignored', () => {
+			editor.setData( '\n<p>foo\n\r\n \t</p>\n<p> bar</p>' );
+
+			expect( getData( editor.document, { withoutSelection: true } ) )
+				.to.equal( '<paragraph>foo</paragraph><paragraph>bar</paragraph>' );
+
+			expect( editor.getData() ).to.equal( '<p>foo</p><p>bar</p>' );
+		} );
+	} );
+} );