Skip to content

Commit

Permalink
Use text nodes content directly instead of stringifying using data pr…
Browse files Browse the repository at this point in the history
…ocessor,

as suggested by @oleq at #5901 (comment).
Ignore HTML elements inside `<pre><code>`.
Closes: #5901.
  • Loading branch information
tomalec committed May 19, 2020
1 parent e73554a commit 40629c5
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 23 deletions.
2 changes: 1 addition & 1 deletion packages/ckeditor5-code-block/src/codeblockediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 8 additions & 15 deletions packages/ckeditor5-code-block/src/converters.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,12 @@ export function modelToDataViewSoftBreakInsertion( model ) {
*
* <codeBlock language="javascript">foo();<softBreak></softBreak>bar();</codeBlock>
*
* @param {module:engine/controller/datacontroller~DataController} dataController
* @param {module:engine/view/view~View} editingView
* @param {Array.<module:code-block/codeblock~CodeBlockLanguageDefinition>} 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:
//
// {
Expand Down Expand Up @@ -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 `<code>`.
// 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 );
Expand Down Expand Up @@ -234,14 +238,3 @@ export function dataViewToModelCodeBlockInsertion( dataController, languageDefs
}
};
}

// Returns content of `<pre></pre>` with unescaped html inside.
//
// @param {String} stringifiedElement
function extractDataFromCodeElement( stringifiedElement ) {
const data = new RegExp( /^<code[^>]*>([\S\s]*)<\/code>$/ ).exec( stringifiedElement )[ 1 ];

return data
.replace( /&lt;/g, '<' )
.replace( /&gt;/g, '>' );
}
24 changes: 17 additions & 7 deletions packages/ckeditor5-code-block/tests/codeblockediting.js
Original file line number Diff line number Diff line change
Expand Up @@ -889,29 +889,39 @@ describe( 'CodeBlockEditing', () => {
);
} );

// Undesired by expected. There is an issue with identifying the correct filler type.
// <code> is inline, so dom-to-view converter expects an inline filler.
it( 'should convert pre > code with only &nbsp; inside to a codeBlock with &nbsp;', () => {
editor.setData( '<pre><code>&nbsp;</code></pre>' );

expect( getModelData( model ) ).to.equal(
'<codeBlock language="plaintext">[]\u00a0</codeBlock>'
);
} );

it( 'should convert pre > code with HTML inside', () => {
editor.setData( '<pre><code><p>Foo</p>\n<p>Bar</p></code></pre>' );

expect( getModelData( model ) ).to.equal(
'<codeBlock language="plaintext">[]' +
'<p>Foo</p>' +
'Foo' +
'<softBreak></softBreak>' +
'<p>Bar</p>' +
'Bar' +
'</codeBlock>'
);
} );

it( 'should convert pre > code tag with HTML and nested pre > code tag', () => {
editor.setData( '<pre><code><p>Foo</p><pre>Bar</pre><p>Biz</p></code></pre>' );
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( '<pre><code><p>Foo</p><pre><code>Bar</code></pre><p>Biz</p></code></pre>' );

expect( getModelData( model ) ).to.equal(
'<codeBlock language="plaintext">[]<p>Foo</p><pre>Bar</pre><p>Biz</p></codeBlock>' );
'<codeBlock language="plaintext">[]FooBarBiz</codeBlock>' );
} );

it( 'should convert pre > code tag with escaped html content', () => {
editor.setData( '<pre><code>&lt;div&gt;&lt;p&gt;Foo&lt;/p&gt;&lt;/div&gt;</code></pre>' );
editor.setData( '<pre><code>&lt;div&gt;&lt;p&gt;Foo&apos;s&amp;&quot;bar&quot;&lt;/p&gt;&lt;/div&gt;</code></pre>' );

expect( getModelData( model ) ).to.equal( '<codeBlock language="plaintext">[]<div><p>Foo</p></div></codeBlock>' );
expect( getModelData( model ) ).to.equal( '<codeBlock language="plaintext">[]<div><p>Foo\'s&"bar"</p></div></codeBlock>' );
} );

it( 'should be overridable', () => {
Expand Down

0 comments on commit 40629c5

Please sign in to comment.