Opened 11 years ago
Closed 11 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 )
This will throw an error:
new CKEDITOR.template( '{foo},\n {bar}' ).output( { foo: 1, bar: 2 } );
Related: #11216.
Change History (14)
comment:1 Changed 11 years ago by
Status: | new → confirmed |
---|
comment:2 Changed 11 years ago by
comment:3 Changed 11 years ago by
Description: | modified (diff) |
---|
comment:4 Changed 11 years ago by
Version: | → 4.0 Beta |
---|
comment:6 Changed 11 years ago by
Owner: | set to Marek Lewandowski |
---|---|
Status: | confirmed → assigned |
comment:7 Changed 11 years ago by
Status: | assigned → review |
---|
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 11 years ago by
Status: | review → review_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:10 Changed 11 years ago by
comment:11 Changed 11 years ago by
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:13 Changed 11 years ago by
Status: | review → review_passed |
---|
I updated t/11102 on dev and tests with some minor improvements.
comment:14 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | review_passed → closed |
Fixed with git:ac305ede7397f8 to master at dev.
You can use below as workaround: