From a389ff3c6cc72e6ab7b0812c921d66529fc02b6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Thu, 26 Mar 2015 22:00:55 +0100 Subject: [PATCH 01/14] Tests: Added tests how attributes are preserved. --- tests/tickets/13105/1.js | 68 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 tests/tickets/13105/1.js diff --git a/tests/tickets/13105/1.js b/tests/tickets/13105/1.js new file mode 100644 index 00000000000..7d94609e051 --- /dev/null +++ b/tests/tickets/13105/1.js @@ -0,0 +1,68 @@ +/* bender-tags: htmldataprocessor,htmlwriter */ +/* bender-ckeditor-plugins: htmlwriter */ + +var attrValue = '

\'"& <a y="3">&</a>

'; + +bender.editors = { + htmlWriter: { + name: 'editor_htmlwriter', + creator: 'inline', + config: { + allowedContent: true, + extraPlugins: 'htmlwriter', + enterMode: CKEDITOR.ENTER_P + } + }, + + basicWriter: { + name: 'editor_basicwriter', + creator: 'inline', + config: { + allowedContent: true, + removePlugins: 'htmlwriter', + enterMode: CKEDITOR.ENTER_P + } + } +}; + +function doTestProcessorAttributeIsPreserved( bot ) { + var editor = bot.editor; + + editor.editable().setHtml( '

foo

' ); + + editor.dataProcessor.htmlFilter.addRules( { + elements: { + span: function( el ) { + el.attributes[ 'data-foo' ] = attrValue; + } + } + } ); + + bot.setData( editor.getData(), function() { + var span = editor.editable().findOne( 'span' ); + + assert.areSame( attrValue, span.data( 'foo' ) ); + } ); +} + +bender.test( { + 'test inline editor with htmlwriter preserves attribute': function() { + doTestProcessorAttributeIsPreserved( bender.editorBots.htmlWriter ); + }, + + 'test inline editor with basicwriter preserves attribute': function() { + doTestProcessorAttributeIsPreserved( bender.editorBots.basicWriter ); + }, + + 'test browser preserves attribute': function() { + var el = CKEDITOR.document.createElement( 'span' ); + el.data( 'foo', attrValue ); + + var el2 = CKEDITOR.document.createElement( 'span' ); + el2.setHtml( el.getOuterHtml() ); + assert.areSame( attrValue, el2.getFirst().data( 'foo' ) ); + assert.areSame( + '', + el.getOuterHtml().toLowerCase() ); + } +} ); \ No newline at end of file From f7cfca39c4d64cd916916b94a72a74b0b9513b47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 27 Mar 2015 10:22:54 +0100 Subject: [PATCH 02/14] Ampersands must be decoded last. --- core/tools.js | 4 ++-- tests/core/tools.js | 38 ++++++++++++++++++++++++++------------ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/core/tools.js b/core/tools.js index 6bead994ad3..6dc1ec90d95 100644 --- a/core/tools.js +++ b/core/tools.js @@ -354,7 +354,7 @@ * @returns {String} The encoded string. */ htmlEncode: function( text ) { - return String( text ).replace( ampRegex, '&' ).replace( gtRegex, '>' ).replace( ltRegex, '<' ); + return text.replace( ampRegex, '&' ).replace( gtRegex, '>' ).replace( ltRegex, '<' ); }, /** @@ -366,7 +366,7 @@ * @returns {String} The decoded string. */ htmlDecode: function( text ) { - return text.replace( ampEscRegex, '&' ).replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ); + return text.replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ).replace( ampEscRegex, '&' ); }, /** diff --git a/tests/core/tools.js b/tests/core/tools.js index 6c6d3743d13..21150830b6a 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -8,6 +8,9 @@ CKEDITOR.env.ie ? '-ms-' : ''; + var htmlEncode = CKEDITOR.tools.htmlEncode, + htmlDecode = CKEDITOR.tools.htmlDecode; + bender.test( { assertNormalizedCssText: function( expected, elementId, msg ) { assert.areSame( expected, CKEDITOR.tools.normalizeCssText( @@ -15,7 +18,8 @@ }, test_extend: function() { - var fakeFn = function() {}; + function fakeFn() {} + var fakeObj = { fake1: 1, fake2: 2 }; var fakeArray = [ 'Test', 10, fakeFn, fakeObj ]; @@ -77,24 +81,35 @@ }, test_htmlEncode1: function() { - assert.areSame( '<b>Test</b>', CKEDITOR.tools.htmlEncode( 'Test' ) ); + assert.areSame( '<b>Test</b>', htmlEncode( 'Test' ) ); }, - + test_htmlEncode2: function() { - assert.areSame( 'Test\'s & "quote"', CKEDITOR.tools.htmlEncode( 'Test\'s & "quote"' ) ); + assert.areSame( 'Test\'s & "quote"', htmlEncode( 'Test\'s & "quote"' ) ); }, - + test_htmlEncode3: function() { - assert.areSame( 'A B \n\n\t\tC\n \t D', CKEDITOR.tools.htmlEncode( 'A B \n\n\t\tC\n \t D' ), 'Tab should not be touched.' ); + assert.areSame( 'A B \n\n\t\tC\n \t D', htmlEncode( 'A B \n\n\t\tC\n \t D' ), 'Tab should not be touched.' ); }, - + test_htmlDecode: function() { - assert.areSame( '', CKEDITOR.tools.htmlDecode( '<a & b >' ), 'Invalid result for htmlDecode' ); - assert.areSame( '', CKEDITOR.tools.htmlDecode( '<a & b ><a & b >' ), 'Invalid result for htmlDecode' ); + assert.areSame( '', htmlDecode( '<a & b >' ) ); }, - + + test_htmlDecode2: function() { + assert.areSame( '', htmlDecode( '<a & b ><a & b >' ) ); + }, + + test_htmlDecode3: function() { + assert.areSame( '< & >', htmlDecode( '&lt; &amp; &gt;' ) ); + }, + + test_htmlDecode4: function() { + assert.areSame( '&lt; &amp; &gt;', htmlDecode( '&amp;lt; &amp;amp; &amp;gt;' ) ); + }, + test_htmlEncode_3874: function() { - assert.areSame( 'line1\nline2', CKEDITOR.tools.htmlEncode( 'line1\nline2' ) ); + assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); }, test_htmlEncodeAttr: function() { @@ -598,5 +613,4 @@ assert.areSame( 33, uuid.length, 'UUID.length' ); } } ); - } )(); \ No newline at end of file From 9bb1a0f3a59b404c4cbc9d14040476d341764ffc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 27 Mar 2015 10:47:05 +0100 Subject: [PATCH 03/14] Tests: Missing tests for htmlDecodeAttr. --- tests/core/tools.js | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/tests/core/tools.js b/tests/core/tools.js index 21150830b6a..d780a9bde23 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -62,24 +62,6 @@ assert.isFalse( CKEDITOR.tools.isArray( window.x ) ); }, - 'test getIndex - not found': function() { - assert.areSame( -1, CKEDITOR.tools.getIndex( [ 1, 2, 3 ], function( el ) { - return el == 4; - } ) ); - }, - - 'test getIndex - found first': function() { - assert.areSame( 2, CKEDITOR.tools.getIndex( [ 0, 1, 2, 2, 2, 3, 2, 2 ], function( el ) { - return el == 2; - } ) ); - }, - - 'test getIndex - found on last position': function() { - assert.areSame( 2, CKEDITOR.tools.getIndex( [ 0, 1, 2 ], function( el ) { - return el == 2; - } ) ); - }, - test_htmlEncode1: function() { assert.areSame( '<b>Test</b>', htmlEncode( 'Test' ) ); }, @@ -111,10 +93,18 @@ test_htmlEncode_3874: function() { assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); }, - + test_htmlEncodeAttr: function() { assert.areSame( '<a b="c"/>', CKEDITOR.tools.htmlEncodeAttr( '' ) ); }, + + test_htmlDecodeAttr: function() { + assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); + }, + + test_htmlDecodeAttr2: function() { + assert.areSame( '&lt; &quot; &gt; &amp;', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp;' ) ); + }, test_cssStyleToDomStyle1: function() { assert.areSame( 'backgroundColor', CKEDITOR.tools.cssStyleToDomStyle( 'background-color' ) ); From 7af57fc3feded06a2aced4f3d228bcbd95447a33 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 24 Apr 2015 09:56:21 +0200 Subject: [PATCH 04/14] Tests: Added one test and fixed others (note: Safari encodes attrs differently than all other browsers). --- tests/tickets/13105/1.js | 44 +++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/tests/tickets/13105/1.js b/tests/tickets/13105/1.js index 7d94609e051..ab804be0182 100644 --- a/tests/tickets/13105/1.js +++ b/tests/tickets/13105/1.js @@ -1,7 +1,10 @@ /* bender-tags: htmldataprocessor,htmlwriter */ /* bender-ckeditor-plugins: htmlwriter */ -var attrValue = '

\'"& <a y="3">&</a>

'; +'use strict'; + +var attrValue = '

\'"& <a y="3">&</a>

', + encodedAttrValue = '<p x="1&quot;2">\'"& &lt;a y="3"&gt;&amp;&lt;/a&gt;</p>'; bender.editors = { htmlWriter: { @@ -26,34 +29,58 @@ bender.editors = { }; function doTestProcessorAttributeIsPreserved( bot ) { - var editor = bot.editor; + var editor = bot.editor, + enabled = false; editor.editable().setHtml( '

foo

' ); editor.dataProcessor.htmlFilter.addRules( { elements: { span: function( el ) { - el.attributes[ 'data-foo' ] = attrValue; + // Affect only this test. + if ( enabled ) { + el.attributes[ 'data-foo' ] = attrValue; + } } } } ); - bot.setData( editor.getData(), function() { + enabled = true; + + var data = editor.getData(); + assert.areSame( '

foo

', data, 'attribute was properly encoded when getting data' ); + + bot.setData( data, function() { + enabled = false; var span = editor.editable().findOne( 'span' ); - assert.areSame( attrValue, span.data( 'foo' ) ); + assert.areSame( attrValue, span.data( 'foo' ), 'attribute was properly preserved when loading data' ); } ); } bender.test( { - 'test inline editor with htmlwriter preserves attribute': function() { + 'test inline editor with htmlwriter preserves attribute set in htmlFilter': function() { doTestProcessorAttributeIsPreserved( bender.editorBots.htmlWriter ); }, - 'test inline editor with basicwriter preserves attribute': function() { + 'test inline editor with basicwriter preserves attribute set in htmlFilter': function() { doTestProcessorAttributeIsPreserved( bender.editorBots.basicWriter ); }, + 'test editor preserves attribute set in DOM': function() { + var bot = bender.editorBots.basicWriter, + editor = bot.editor; + + editor.editable().setHtml( '

x

' ); + editor.editable().findOne( 'span' ).setAttribute( 'data-bar', attrValue ); + + bot.setData( editor.getData(), function() { + var span = editor.editable().findOne( 'span' ); + + assert.areSame( attrValue, span.data( 'bar' ) ); + } ); + }, + 'test browser preserves attribute': function() { var el = CKEDITOR.document.createElement( 'span' ); el.data( 'foo', attrValue ); @@ -61,8 +88,5 @@ bender.test( { var el2 = CKEDITOR.document.createElement( 'span' ); el2.setHtml( el.getOuterHtml() ); assert.areSame( attrValue, el2.getFirst().data( 'foo' ) ); - assert.areSame( - '', - el.getOuterHtml().toLowerCase() ); } } ); \ No newline at end of file From 7752ceeb7954c36c573c274753c9bb01c7932f7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 24 Apr 2015 14:51:19 +0200 Subject: [PATCH 05/14] Ampersand must be encoded in attributes because other entities may be lost. --- core/tools.js | 4 ++-- tests/core/tools.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/core/tools.js b/core/tools.js index 6dc1ec90d95..7b7b7681855 100644 --- a/core/tools.js +++ b/core/tools.js @@ -378,7 +378,7 @@ * @returns {String} The encoded value. */ htmlEncodeAttr: function( text ) { - return text.replace( quoteRegex, '"' ).replace( ltRegex, '<' ).replace( gtRegex, '>' ); + return CKEDITOR.tools.htmlEncode( text ).replace( quoteRegex, '"' ); }, /** @@ -392,7 +392,7 @@ * @returns {String} The decoded text. */ htmlDecodeAttr: function( text ) { - return text.replace( quoteEscRegex, '"' ).replace( ltEscRegex, '<' ).replace( gtEscRegex, '>' ); + return CKEDITOR.tools.htmlDecode( text.replace( quoteEscRegex, '"' ) ); }, /** diff --git a/tests/core/tools.js b/tests/core/tools.js index d780a9bde23..9dce1efb7d2 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -95,15 +95,15 @@ }, test_htmlEncodeAttr: function() { - assert.areSame( '<a b="c"/>', CKEDITOR.tools.htmlEncodeAttr( '' ) ); + assert.areSame( '<a b="c&d"/>', CKEDITOR.tools.htmlEncodeAttr( '' ) ); }, test_htmlDecodeAttr: function() { - assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); + assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); }, test_htmlDecodeAttr2: function() { - assert.areSame( '&lt; &quot; &gt; &amp;', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp;' ) ); + assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp;' ) ); }, test_cssStyleToDomStyle1: function() { From 6ff688b3a050dd04a7d8cb32d85330c33659fbad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Fri, 24 Apr 2015 15:12:45 +0200 Subject: [PATCH 06/14] Brought back support for passing non-string values to encodeHtml because in dozen places in the dialog plugins incorrect values are passed there. --- core/tools.js | 8 +++++++- tests/core/tools.js | 28 ++++++++++++++++++---------- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/core/tools.js b/core/tools.js index 7b7b7681855..535b159c73b 100644 --- a/core/tools.js +++ b/core/tools.js @@ -354,7 +354,13 @@ * @returns {String} The encoded string. */ htmlEncode: function( text ) { - return text.replace( ampRegex, '&' ).replace( gtRegex, '>' ).replace( ltRegex, '<' ); + // Backwards compatibility - accept also non-string values (casting is done below). + // Since 4.4.8 we return empty string for null and undefined because these values make no sense. + if ( text === undefined || text === null ) { + return ''; + } + + return String( text ).replace( ampRegex, '&' ).replace( gtRegex, '>' ).replace( ltRegex, '<' ); }, /** diff --git a/tests/core/tools.js b/tests/core/tools.js index 9dce1efb7d2..4a08afef6fb 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -65,43 +65,51 @@ test_htmlEncode1: function() { assert.areSame( '<b>Test</b>', htmlEncode( 'Test' ) ); }, - + test_htmlEncode2: function() { assert.areSame( 'Test\'s & "quote"', htmlEncode( 'Test\'s & "quote"' ) ); }, - + test_htmlEncode3: function() { assert.areSame( 'A B \n\n\t\tC\n \t D', htmlEncode( 'A B \n\n\t\tC\n \t D' ), 'Tab should not be touched.' ); }, - + + // Backwards compatibility with careless plugins like dialog or dialogui. All values must be accepted. + test_htmlEncode4: function() { + assert.areSame( '', htmlEncode( undefined ), 'undef' ); + assert.areSame( '', htmlEncode( null ), 'null' ); + assert.areSame( '3', htmlEncode( 3 ), '3' ); + assert.areSame( '0', htmlEncode( 0 ), '0' ); + }, + test_htmlDecode: function() { assert.areSame( '', htmlDecode( '<a & b >' ) ); }, - + test_htmlDecode2: function() { assert.areSame( '', htmlDecode( '<a & b ><a & b >' ) ); }, - + test_htmlDecode3: function() { assert.areSame( '< & >', htmlDecode( '&lt; &amp; &gt;' ) ); }, - + test_htmlDecode4: function() { assert.areSame( '&lt; &amp; &gt;', htmlDecode( '&amp;lt; &amp;amp; &amp;gt;' ) ); }, - + test_htmlEncode_3874: function() { assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); }, - + test_htmlEncodeAttr: function() { assert.areSame( '<a b="c&d"/>', CKEDITOR.tools.htmlEncodeAttr( '' ) ); }, - + test_htmlDecodeAttr: function() { assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); }, - + test_htmlDecodeAttr2: function() { assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp;' ) ); }, From b0f5b494190b0bb9c8818d98b1538edf86fd92f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 11 May 2015 18:06:48 +0200 Subject: [PATCH 07/14] Fixed encoding issue in BBCode plugin. --- plugins/bbcode/plugin.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/bbcode/plugin.js b/plugins/bbcode/plugin.js index f7991dfcba1..2aa6dc9a533 100644 --- a/plugins/bbcode/plugin.js +++ b/plugins/bbcode/plugin.js @@ -146,7 +146,12 @@ styles[ stylesMap[ part ] ] = optionPart; attribs.style = serializeStyleText( styles ); } else if ( attributesMap[ part ] ) { - attribs[attributesMap[part]] = optionPart; + // All the input BBCode is encoded at the beginning so <> characters in the textual part + // are later correctly preserved in HTML. However... it affects parts that now become + // attributes, so we need to revert that. As a matter of fact, the content should not be + // encoded at the beginning, but only later when creating text nodes (encoding should be more precise) + // but it's too late not for such changes. + attribs[ attributesMap[ part ] ] = CKEDITOR.tools.htmlDecode( optionPart ); } } From 485d36b9b5291f03bb779d0f31b00dffc32fb8b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 11 May 2015 18:26:57 +0200 Subject: [PATCH 08/14] More tests and more fixes for BBCode. The entire plugin based on a completely broken behaviour of encoding/decoding mechanism so it's now like a pandora box. --- plugins/bbcode/plugin.js | 8 +------- tests/plugins/bbcode/bbcode.js | 14 +++++++++++++- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/plugins/bbcode/plugin.js b/plugins/bbcode/plugin.js index 2aa6dc9a533..c8d386e7bed 100644 --- a/plugins/bbcode/plugin.js +++ b/plugins/bbcode/plugin.js @@ -70,9 +70,7 @@ var regex = [], entities = { nbsp: '\u00A0', // IE | FF - shy: '\u00AD', // IE - gt: '\u003E', // IE | FF | -- | Opera - lt: '\u003C' // IE | FF | Safari | Opera + shy: '\u00AD' // IE }; for ( var entity in entities ) @@ -494,10 +492,6 @@ attribute: function( name, val ) { if ( name == 'option' ) { - // Force simply ampersand in attributes. - if ( typeof val == 'string' ) - val = val.replace( /&/g, '&' ); - this.write( '=', val ); } }, diff --git a/tests/plugins/bbcode/bbcode.js b/tests/plugins/bbcode/bbcode.js index 063fd505aaa..20a8a5127f8 100644 --- a/tests/plugins/bbcode/bbcode.js +++ b/tests/plugins/bbcode/bbcode.js @@ -65,7 +65,19 @@ bender.test( { // #8995 'test escape HTML entities in bbcode': function() { - var html = 'foo<bar>', bbcode = '[url=foo&bar]foo[/url]'; + var html = '&foo<bar>', bbcode = '[url=foo&bar]&foo[/url]'; + this.assertToHtml( html, bbcode ); + this.assertToBBCode( bbcode, html ); + }, + + 'test escape HTML entities in bbcode - double encoding': function() { + var html = 'foo&lt;bar&gt;bom&amp;lt;', bbcode = '[url=foo&bar&lt;]foo<bar>bom&lt;[/url]'; + this.assertToHtml( html, bbcode ); + this.assertToBBCode( bbcode, html ); + }, + + 'test escape HTML entities in bbcode - ampresands in the text': function() { + var html = '& &amp; &amp;amp;', bbcode = '& & &amp;'; this.assertToHtml( html, bbcode ); this.assertToBBCode( bbcode, html ); }, From 3bb55a047906aa6fa4d275bb6fa0a793411f6212 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 11 May 2015 18:29:05 +0200 Subject: [PATCH 09/14] Tests: Fixed URL pattern. --- tests/plugins/link/mail_link.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/link/mail_link.js b/tests/plugins/link/mail_link.js index 46986ba9a2e..7afcca56642 100644 --- a/tests/plugins/link/mail_link.js +++ b/tests/plugins/link/mail_link.js @@ -10,7 +10,7 @@ bender.editor = { }; var protectedMailLink = 'AJD'; + '+\'?subject=Job%20Request&body=I\\\'m%20looking%20for%20the%20AJD%20position.\')\">AJD'; bender.test( { 'test created protected mail link': function() { From fd904627d4df2578aec38215b2b42c689a415591 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 8 Jun 2015 11:36:59 +0200 Subject: [PATCH 10/14] Tests: Improved test case naming. --- tests/core/tools.js | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/tests/core/tools.js b/tests/core/tools.js index 4a08afef6fb..ea5945199dd 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -62,55 +62,55 @@ assert.isFalse( CKEDITOR.tools.isArray( window.x ) ); }, - test_htmlEncode1: function() { - assert.areSame( '<b>Test</b>', htmlEncode( 'Test' ) ); + 'test_htmlEncode - all covered entities': function() { + assert.areSame( '<b>Test&fun!</b>', htmlEncode( 'Test&fun!' ) ); }, - test_htmlEncode2: function() { + 'test htmlEncode - do not touch quotes': function() { assert.areSame( 'Test\'s & "quote"', htmlEncode( 'Test\'s & "quote"' ) ); }, - test_htmlEncode3: function() { + 'test htmlEncode - tabs': function() { assert.areSame( 'A B \n\n\t\tC\n \t D', htmlEncode( 'A B \n\n\t\tC\n \t D' ), 'Tab should not be touched.' ); }, // Backwards compatibility with careless plugins like dialog or dialogui. All values must be accepted. - test_htmlEncode4: function() { + 'test htmlEncode - backwards compat': function() { assert.areSame( '', htmlEncode( undefined ), 'undef' ); assert.areSame( '', htmlEncode( null ), 'null' ); assert.areSame( '3', htmlEncode( 3 ), '3' ); assert.areSame( '0', htmlEncode( 0 ), '0' ); }, - test_htmlDecode: function() { + 'test htmlEncode - #3874': function() { + assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); + }, + + 'test htmlDecode - all covered entities': function() { assert.areSame( '', htmlDecode( '<a & b >' ) ); }, - test_htmlDecode2: function() { + 'test htmlDecode - duplications': function() { assert.areSame( '', htmlDecode( '<a & b ><a & b >' ) ); }, - test_htmlDecode3: function() { + 'test htmlDecode - double encoding': function() { assert.areSame( '< & >', htmlDecode( '&lt; &amp; &gt;' ) ); }, - test_htmlDecode4: function() { + 'test htmlDecode - triple encoding': function() { assert.areSame( '&lt; &amp; &gt;', htmlDecode( '&amp;lt; &amp;amp; &amp;gt;' ) ); }, - test_htmlEncode_3874: function() { - assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); - }, - - test_htmlEncodeAttr: function() { + 'test htmlEncodeAttr - all covered entities': function() { assert.areSame( '<a b="c&d"/>', CKEDITOR.tools.htmlEncodeAttr( '' ) ); }, - test_htmlDecodeAttr: function() { + 'test htmlDecodeAttr - all covered entities': function() { assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); }, - test_htmlDecodeAttr2: function() { + 'test htmlDecodeAttr - double encoding': function() { assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp;' ) ); }, From 053b55ba65dcba4e2b4ddeb0c4bb196d2c51c124 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 8 Jun 2015 11:49:56 +0200 Subject: [PATCH 11/14] Added   and ­ decoding. --- core/tools.js | 9 ++++++++- tests/core/tools.js | 6 +++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/core/tools.js b/core/tools.js index 535b159c73b..9e774226981 100644 --- a/core/tools.js +++ b/core/tools.js @@ -23,6 +23,8 @@ ampEscRegex = /&/g, gtEscRegex = />/g, ltEscRegex = /</g, + nbspEscRegex = / /g, + shyEscRegex = /­/g, quoteEscRegex = /"/g; CKEDITOR.on( 'reset', function() { @@ -372,7 +374,12 @@ * @returns {String} The decoded string. */ htmlDecode: function( text ) { - return text.replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ).replace( ampEscRegex, '&' ); + // See http://dev.ckeditor.com/ticket/13105#comment:8 + + return text + .replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ) + .replace( nbspEscRegex, '\u00a0' ).replace( shyEscRegex, '\u00ad' ) + .replace( ampEscRegex, '&' ); }, /** diff --git a/tests/core/tools.js b/tests/core/tools.js index ea5945199dd..2bdc0034def 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -87,7 +87,7 @@ }, 'test htmlDecode - all covered entities': function() { - assert.areSame( '', htmlDecode( '<a & b >' ) ); + assert.areSame( '< a & b > c \u00a0 d \u00ad', htmlDecode( '< a & b > c   d ­' ) ); }, 'test htmlDecode - duplications': function() { @@ -95,7 +95,7 @@ }, 'test htmlDecode - double encoding': function() { - assert.areSame( '< & >', htmlDecode( '&lt; &amp; &gt;' ) ); + assert.areSame( '< & >   ­', htmlDecode( '&lt; &amp; &gt; &nbsp; &shy;' ) ); }, 'test htmlDecode - triple encoding': function() { @@ -107,7 +107,7 @@ }, 'test htmlDecodeAttr - all covered entities': function() { - assert.areSame( '< " > &', CKEDITOR.tools.htmlDecodeAttr( '< " > &' ) ); + assert.areSame( '< " > & \u00a0 \u00ad', CKEDITOR.tools.htmlDecodeAttr( '< " > &   ­' ) ); }, 'test htmlDecodeAttr - double encoding': function() { From eb891e3bf7fdf6c22869e86a09a2bb0356a89803 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 8 Jun 2015 12:49:50 +0200 Subject: [PATCH 12/14] Added numeric entities decoding. --- core/tools.js | 9 ++++++++- tests/core/tools.js | 9 +++++++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/core/tools.js b/core/tools.js index 9e774226981..6a582d872f8 100644 --- a/core/tools.js +++ b/core/tools.js @@ -25,7 +25,11 @@ ltEscRegex = /</g, nbspEscRegex = / /g, shyEscRegex = /­/g, - quoteEscRegex = /"/g; + quoteEscRegex = /"/g, + numericEscRegex = /&#(\d{1,5});/g, + numericEscDecode = function( match, code ) { + return String.fromCharCode( parseInt( code, 10 ) ); + }; CKEDITOR.on( 'reset', function() { functions = []; @@ -375,10 +379,13 @@ */ htmlDecode: function( text ) { // See http://dev.ckeditor.com/ticket/13105#comment:8 + // PS. The order of execution will fail if we have &amp; which should return & but will return &. + // We can ignore it though, because none of the engines seem to encode & with a numeric entity. return text .replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ) .replace( nbspEscRegex, '\u00a0' ).replace( shyEscRegex, '\u00ad' ) + .replace( numericEscRegex, numericEscDecode ) .replace( ampEscRegex, '&' ); }, diff --git a/tests/core/tools.js b/tests/core/tools.js index 2bdc0034def..186a2222562 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -86,10 +86,15 @@ assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) ); }, - 'test htmlDecode - all covered entities': function() { + // http://dev.ckeditor.com/ticket/13105#comment:8 + 'test htmlDecode - all covered named entities': function() { assert.areSame( '< a & b > c \u00a0 d \u00ad', htmlDecode( '< a & b > c   d ­' ) ); }, + 'test htmlDecode - numeric entities': function() { + assert.areSame( '\u0001 \u000a \u00ff \uffff \u000c', htmlDecode( ' ÿ ￿ ' ) ); + }, + 'test htmlDecode - duplications': function() { assert.areSame( '', htmlDecode( '<a & b ><a & b >' ) ); }, @@ -107,7 +112,7 @@ }, 'test htmlDecodeAttr - all covered entities': function() { - assert.areSame( '< " > & \u00a0 \u00ad', CKEDITOR.tools.htmlDecodeAttr( '< " > &   ­' ) ); + assert.areSame( '< " > & \u00a0 \u00ad \u000a', CKEDITOR.tools.htmlDecodeAttr( '< " > &   ­ ' ) ); }, 'test htmlDecodeAttr - double encoding': function() { From 5bb8284e463e71441ef51e56ce473989ec1d0a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotrek=20Koszuli=C5=84ski?= Date: Mon, 8 Jun 2015 13:36:38 +0200 Subject: [PATCH 13/14] Optimized and simplified the implementation. --- core/tools.js | 52 +++++++++++++++++++++++++-------------------- tests/core/tools.js | 2 +- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/core/tools.js b/core/tools.js index 6a582d872f8..90853feafc2 100644 --- a/core/tools.js +++ b/core/tools.js @@ -20,15 +20,21 @@ ltRegex = /', + amp: '&', + quot: '"', + nbsp: '\u00a0', + shy: '\u00ad' + }, + allEscDecode = function( match, code ) { + if ( code[ 0 ] == '#' ) { + return String.fromCharCode( parseInt( code.slice( 1 ), 10 ) ); + } else { + return namedEntities[ code ]; + } }; CKEDITOR.on( 'reset', function() { @@ -370,23 +376,22 @@ }, /** - * Decodes HTML entities. + * Decodes HTML entities which browsers tend to encode when used in text nodes. * * alert( CKEDITOR.tools.htmlDecode( '<a & b >' ) ); // '' * + * Read more about chosen entities in the [research](http://dev.ckeditor.com/ticket/13105#comment:8). + * * @param {String} The string to be decoded. * @returns {String} The decoded string. */ htmlDecode: function( text ) { - // See http://dev.ckeditor.com/ticket/13105#comment:8 - // PS. The order of execution will fail if we have &amp; which should return & but will return &. - // We can ignore it though, because none of the engines seem to encode & with a numeric entity. - - return text - .replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' ) - .replace( nbspEscRegex, '\u00a0' ).replace( shyEscRegex, '\u00ad' ) - .replace( numericEscRegex, numericEscDecode ) - .replace( ampEscRegex, '&' ); + // See: + // * http://dev.ckeditor.com/ticket/13105#comment:8 and comment:9, + // * http://jsperf.com/wth-is-going-on-with-jsperf JSPerf has some serious problems, but you can observe + // that combined regexp tends to be quicker (except on V8). It will also not be prone to fail on '&lt;' + // (see http://dev.ckeditor.com/ticket/13105#DXWTF:CKEDITOR.tools.htmlEnDecodeAttr). + return text.replace( allEscRegex, allEscDecode ); }, /** @@ -402,17 +407,18 @@ }, /** - * Replace HTML entities previously encoded by - * {@link #htmlEncodeAttr htmlEncodeAttr} back to their plain character - * representation. + * Decodes HTML entities which browsers tend to encode when used in attributes. * * alert( CKEDITOR.tools.htmlDecodeAttr( '<a " b>' ) ); // '' * + * Since CKEditor 4.5.0 this method simply executes {@link #htmlDecode} which covers + * all necessary entities. + * * @param {String} text The text to be decoded. * @returns {String} The decoded text. */ htmlDecodeAttr: function( text ) { - return CKEDITOR.tools.htmlDecode( text.replace( quoteEscRegex, '"' ) ); + return CKEDITOR.tools.htmlDecode( text ); }, /** diff --git a/tests/core/tools.js b/tests/core/tools.js index 186a2222562..1bf90a826be 100644 --- a/tests/core/tools.js +++ b/tests/core/tools.js @@ -88,7 +88,7 @@ // http://dev.ckeditor.com/ticket/13105#comment:8 'test htmlDecode - all covered named entities': function() { - assert.areSame( '< a & b > c \u00a0 d \u00ad', htmlDecode( '< a & b > c   d ­' ) ); + assert.areSame( '< a & b > c \u00a0 d \u00ad e "', htmlDecode( '< a & b > c   d ­ e "' ) ); }, 'test htmlDecode - numeric entities': function() { From 9eeeb44c7a5ff4716e655337bf93b3136540685d Mon Sep 17 00:00:00 2001 From: Aleksander Nowodzinski Date: Fri, 12 Jun 2015 10:48:16 +0200 Subject: [PATCH 14/14] Changelog entry. --- CHANGES.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGES.md b/CHANGES.md index 020935be65c..c6de7ecadd0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -26,6 +26,7 @@ Fixed Issues: * [#13176](http://dev.ckeditor.com/ticket/13176): [IE8] Fixed: Errors while drag&drop of embed widgets. * [#13015](http://dev.ckeditor.com/ticket/13015): Fixed: Dropping image file on [Enhanced Image](http://ckeditor.com/addon/image2) causes page reload. * [#13080](http://dev.ckeditor.com/ticket/13080): Fixed: Ugly notification when response has HTML content. +* [#13105](http://dev.ckeditor.com/ticket/13105): Fixed: Various issues related to [`CKEDITOR.tools.htmlEncode()`](http://docs.ckeditor.com/#!/api/CKEDITOR.tools-method-htmlEncode) and [`CKEDITOR.tools.htmlDecode()`](http://docs.ckeditor.com/#!/api/CKEDITOR.tools-method-htmlDecode). * [#13128](http://dev.ckeditor.com/ticket/13128): Fixed: Various issues regarding cloning IDs of elements: * Fixed the default behavior of [`range.cloneContents()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-cloneContents), [`range.extractContents()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-extractContents) methods which now clone IDs similarly to their native counterparts. * Added `cloneId` arguments to the above methods and [`range.splitBlock()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.range-method-splitBlock) and [`element.breakParent()`](http://docs.ckeditor.com/#!/api/CKEDITOR.dom.element-method-breakParent). Mind the default values and special behavior in the `extracContents()` method.