From 40629c5caf89ffb7b99e5f43278407ff5deec9b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomek=20Wytr=C4=99bowicz?= Date: Tue, 19 May 2020 13:33:27 +0200 Subject: [PATCH] Use text nodes content directly instead of stringifying using data processor, as suggested by @oleq at https://github.com/ckeditor/ckeditor5/issues/5901#issuecomment-624605013. Ignore HTML elements inside `
`.
Closes: #5901.
---
 .../src/codeblockediting.js                   |  2 +-
 .../ckeditor5-code-block/src/converters.js    | 23 +++++++-----------
 .../tests/codeblockediting.js                 | 24 +++++++++++++------
 3 files changed, 26 insertions(+), 23 deletions(-)

diff --git a/packages/ckeditor5-code-block/src/codeblockediting.js b/packages/ckeditor5-code-block/src/codeblockediting.js
index e819e08933a..46169f48b98 100644
--- a/packages/ckeditor5-code-block/src/codeblockediting.js
+++ b/packages/ckeditor5-code-block/src/codeblockediting.js
@@ -128,7 +128,7 @@ export default class CodeBlockEditing extends Plugin {
 		editor.editing.downcastDispatcher.on( 'insert:codeBlock', modelToViewCodeBlockInsertion( model, normalizedLanguagesDefs, true ) );
 		editor.data.downcastDispatcher.on( 'insert:codeBlock', modelToViewCodeBlockInsertion( model, normalizedLanguagesDefs ) );
 		editor.data.downcastDispatcher.on( 'insert:softBreak', modelToDataViewSoftBreakInsertion( model ), { priority: 'high' } );
-		editor.data.upcastDispatcher.on( 'element:pre', dataViewToModelCodeBlockInsertion( editor.data, normalizedLanguagesDefs ) );
+		editor.data.upcastDispatcher.on( 'element:pre', dataViewToModelCodeBlockInsertion( editor.editing.view, normalizedLanguagesDefs ) );
 
 		// Intercept the clipboard input (paste) when the selection is anchored in the code block and force the clipboard
 		// data to be pasted as a single plain text. Otherwise, the code lines will split the code block and
diff --git a/packages/ckeditor5-code-block/src/converters.js b/packages/ckeditor5-code-block/src/converters.js
index 7ee4a6324f5..416e549a20e 100644
--- a/packages/ckeditor5-code-block/src/converters.js
+++ b/packages/ckeditor5-code-block/src/converters.js
@@ -126,12 +126,12 @@ export function modelToDataViewSoftBreakInsertion( model ) {
  *
  *		foo();bar();
  *
- * @param {module:engine/controller/datacontroller~DataController} dataController
+ * @param {module:engine/view/view~View} editingView
  * @param {Array.} languageDefs The normalized language
  * configuration passed to the feature.
  * @returns {Function} Returns a conversion callback.
  */
-export function dataViewToModelCodeBlockInsertion( dataController, languageDefs ) {
+export function dataViewToModelCodeBlockInsertion( editingView, languageDefs ) {
 	// Language names associated with CSS classes:
 	//
 	//		{
@@ -183,8 +183,12 @@ export function dataViewToModelCodeBlockInsertion( dataController, languageDefs
 			writer.setAttribute( 'language', defaultLanguageName, codeBlock );
 		}
 
-		const stringifiedElement = dataController.processor.toData( viewChild );
-		const textData = extractDataFromCodeElement( stringifiedElement );
+		// HTML elements are invalid content for ``.
+		// Read only text nodes.
+		const textData = [ ...editingView.createRangeIn( viewChild ) ]
+			.filter( current => current.type === 'text' )
+			.map( ( { item } ) => item.data )
+			.join( '' );
 		const fragment = rawSnippetTextToModelDocumentFragment( writer, textData );
 
 		writer.append( fragment, codeBlock );
@@ -234,14 +238,3 @@ export function dataViewToModelCodeBlockInsertion( dataController, languageDefs
 		}
 	};
 }
-
-// Returns content of `
` with unescaped html inside.
-//
-// @param {String} stringifiedElement
-function extractDataFromCodeElement( stringifiedElement ) {
-	const data = new RegExp( /^]*>([\S\s]*)<\/code>$/ ).exec( stringifiedElement )[ 1 ];
-
-	return data
-		.replace( /</g, '<' )
-		.replace( />/g, '>' );
-}
diff --git a/packages/ckeditor5-code-block/tests/codeblockediting.js b/packages/ckeditor5-code-block/tests/codeblockediting.js
index 8cd86abd9eb..738f7d93898 100644
--- a/packages/ckeditor5-code-block/tests/codeblockediting.js
+++ b/packages/ckeditor5-code-block/tests/codeblockediting.js
@@ -889,29 +889,39 @@ describe( 'CodeBlockEditing', () => {
 			);
 		} );
 
+		// Undesired by expected. There is an issue with identifying the correct filler type.
+		//  is inline, so dom-to-view converter expects an inline filler.
+		it( 'should convert pre > code with only   inside to a codeBlock with  ', () => {
+			editor.setData( '
 
' ); + + expect( getModelData( model ) ).to.equal( + '[]\u00a0' + ); + } ); + it( 'should convert pre > code with HTML inside', () => { editor.setData( '

Foo

\n

Bar

' ); expect( getModelData( model ) ).to.equal( '[]' + - '

Foo

' + + 'Foo' + '' + - '

Bar

' + + 'Bar' + '
' ); } ); - it( 'should convert pre > code tag with HTML and nested pre > code tag', () => { - editor.setData( '

Foo

Bar

Biz

' ); + it( 'should convert pre > code tag with HTML and nested pre > code tag and use only the text content of invalid HTML tags', () => { + editor.setData( '

Foo

Bar

Biz

' ); expect( getModelData( model ) ).to.equal( - '[]

Foo

Bar

Biz

' ); + '[]FooBarBiz' ); } ); it( 'should convert pre > code tag with escaped html content', () => { - editor.setData( '
<div><p>Foo</p></div>
' ); + editor.setData( '
<div><p>Foo's&"bar"</p></div>
' ); - expect( getModelData( model ) ).to.equal( '[]

Foo

' ); + expect( getModelData( model ) ).to.equal( '[]

Foo\'s&"bar"

' ); } ); it( 'should be overridable', () => {