Opened 6 years ago

Closed 6 years ago

#11102 closed Bug (fixed)

CKEDITOR.template does not accept new line characters

Reported by: Piotrek Koszuliński Owned by: Marek Lewandowski
Priority: Normal Milestone: CKEditor 4.3.2
Component: General Version: 4.0 Beta
Keywords: Cc:

Description (last modified by Piotrek Koszuliński)

This will throw an error:

new CKEDITOR.template( '{foo},\n {bar}' ).output( { foo: 1, bar: 2 } );

Related: #11216.

Change History (14)

comment:1 Changed 6 years ago by Piotrek Koszuliński

Status: newconfirmed

comment:2 Changed 6 years ago by Jakub Ś

You can use below as workaround:

tpl = new CKEDITOR.template( '{foo}, \\n {bar}' );
console.log( tpl.output( { foo: '1', bar: '2'} ) );

comment:3 Changed 6 years ago by Piotrek Koszuliński

Description: modified (diff)

comment:4 Changed 6 years ago by Jakub Ś

Version: 4.0 Beta

comment:5 Changed 6 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.3.2

Duplicate closed: #11236.

comment:6 Changed 6 years ago by Marek Lewandowski

Owner: set to Marek Lewandowski
Status: confirmedassigned

comment:7 Changed 6 years ago by Marek Lewandowski

Status: assignedreview

Issues fixed in t/11216 at dev and tests. In this commit i've also moved regular expressions into separate variables, so these will be compiled only once, instead doing that in each commit.

I have mixed feelings about:

.replace( reEscapableChars, "\\$1" )
.replace( reNewLine, "\\n" )
.replace( reCarriageReturn, "\\r" )

Does any1 of you guys have any better idea how to combine these cases into single call or sth?

Also i checked performance and function does work more or less slightly faster or the same as original (except for ie8 and ff25), link to test.

Remark: at the end i couldn't apply optimisation with dot access, rather than square bracket, since some variables have names like 'z-index' etc.

Related issue:
#11102

comment:8 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_failed

And the comment still says:

Escape all quotation marks (").

Please update it.

Regarding performance - that test is far from being realistic. After first pass, template is returned from cache and the template is empty, so there's no data usage. Building interesting test case for that would take some time and the template class is not something we need to optimize, so that wouldn't help us.

comment:9 Changed 6 years ago by Marek Lewandowski

Status: review_failedreview

Updated t/11216

comment:10 Changed 6 years ago by Piotrek Koszuliński

Just one question - why is this ticket on review, when branch is called t/11216? Does this branch fixed this ticket and #11216? Then it still should be handled #11216.

comment:11 Changed 6 years ago by Marek Lewandowski

Yeah i pushed initially this solution into t/11216 since the same change fixes both tickets, we can update #11216 to review, and continue there, otherwise, after R+ we can drop msg there that it has been handled in this ticket.

comment:12 Changed 6 years ago by Piotrek Koszuliński

I pushed t/11102 branches and deleted t/11216.

comment:13 Changed 6 years ago by Piotrek Koszuliński

Status: reviewreview_passed

I updated t/11102 on dev and tests with some minor improvements.

comment:14 Changed 6 years ago by Marek Lewandowski

Resolution: fixed
Status: review_passedclosed

Fixed with git:ac305ede7397f8 to master at dev.

Note: See TracTickets for help on using tickets.
© 2003 – 2019 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy