Skip to content

Commit

Permalink
Merge branch 't/13105' into major
Browse files Browse the repository at this point in the history
  • Loading branch information
oleq committed Jun 12, 2015
2 parents d8ac852 + 9eeeb44 commit 2c45c76
Show file tree
Hide file tree
Showing 7 changed files with 195 additions and 48 deletions.
1 change: 1 addition & 0 deletions CHANGES.md
Expand Up @@ -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.
Expand Down
48 changes: 37 additions & 11 deletions core/tools.js
Expand Up @@ -20,10 +20,22 @@
ltRegex = /</g,
quoteRegex = /"/g,

ampEscRegex = /&amp;/g,
gtEscRegex = /&gt;/g,
ltEscRegex = /&lt;/g,
quoteEscRegex = /&quot;/g;
allEscRegex = /&(lt|gt|amp|quot|nbsp|shy|#\d{1,5});/g,
namedEntities = {
lt: '<',
gt: '>',
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() {
functions = [];
Expand Down Expand Up @@ -354,19 +366,32 @@
* @returns {String} The encoded string.
*/
htmlEncode: function( text ) {
// 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, '&amp;' ).replace( gtRegex, '&gt;' ).replace( ltRegex, '&lt;' );
},

/**
* Decodes HTML entities.
* Decodes HTML entities which browsers tend to encode when used in text nodes.
*
* alert( CKEDITOR.tools.htmlDecode( '&lt;a &amp; b &gt;' ) ); // '<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 ) {
return text.replace( ampEscRegex, '&' ).replace( gtEscRegex, '>' ).replace( ltEscRegex, '<' );
// 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 '&amp;lt;'
// (see http://dev.ckeditor.com/ticket/13105#DXWTF:CKEDITOR.tools.htmlEnDecodeAttr).
return text.replace( allEscRegex, allEscDecode );
},

/**
Expand All @@ -378,21 +403,22 @@
* @returns {String} The encoded value.
*/
htmlEncodeAttr: function( text ) {
return text.replace( quoteRegex, '&quot;' ).replace( ltRegex, '&lt;' ).replace( gtRegex, '&gt;' );
return CKEDITOR.tools.htmlEncode( text ).replace( quoteRegex, '&quot;' );
},

/**
* 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( '&lt;a &quot; b&gt;' ) ); // '<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 text.replace( quoteEscRegex, '"' ).replace( ltEscRegex, '<' ).replace( gtEscRegex, '>' );
return CKEDITOR.tools.htmlDecode( text );
},

/**
Expand Down
15 changes: 7 additions & 8 deletions plugins/bbcode/plugin.js
Expand Up @@ -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 )
Expand Down Expand Up @@ -146,7 +144,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 );
}
}

Expand Down Expand Up @@ -489,10 +492,6 @@

attribute: function( name, val ) {
if ( name == 'option' ) {
// Force simply ampersand in attributes.
if ( typeof val == 'string' )
val = val.replace( /&amp;/g, '&' );

this.write( '=', val );
}
},
Expand Down
71 changes: 44 additions & 27 deletions tests/core/tools.js
Expand Up @@ -8,14 +8,18 @@
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(
CKEDITOR.document.getById( elementId ).getAttribute( 'style' ) ), msg );
},

test_extend: function() {
var fakeFn = function() {};
function fakeFn() {}

var fakeObj = { fake1: 1, fake2: 2 };
var fakeArray = [ 'Test', 10, fakeFn, fakeObj ];

Expand Down Expand Up @@ -58,47 +62,61 @@
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_htmlEncode - all covered entities': function() {
assert.areSame( '&lt;b&gt;Test&amp;fun!&lt;/b&gt;', htmlEncode( '<b>Test&fun!</b>' ) );
},

'test htmlEncode - do not touch quotes': function() {
assert.areSame( 'Test\'s &amp; "quote"', htmlEncode( 'Test\'s & "quote"' ) );
},

'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.' );
},

'test getIndex - found first': function() {
assert.areSame( 2, CKEDITOR.tools.getIndex( [ 0, 1, 2, 2, 2, 3, 2, 2 ], function( el ) {
return el == 2;
} ) );
// Backwards compatibility with careless plugins like dialog or dialogui. All values must be accepted.
'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 getIndex - found on last position': function() {
assert.areSame( 2, CKEDITOR.tools.getIndex( [ 0, 1, 2 ], function( el ) {
return el == 2;
} ) );
'test htmlEncode - #3874': function() {
assert.areSame( 'line1\nline2', htmlEncode( 'line1\nline2' ) );
},

test_htmlEncode1: function() {
assert.areSame( '&lt;b&gt;Test&lt;/b&gt;', CKEDITOR.tools.htmlEncode( '<b>Test</b>' ) );
// http://dev.ckeditor.com/ticket/13105#comment:8
'test htmlDecode - all covered named entities': function() {
assert.areSame( '< a & b > c \u00a0 d \u00ad e "', htmlDecode( '&lt; a &amp; b &gt; c &nbsp; d &shy; e &quot;' ) );
},

test_htmlEncode2: function() {
assert.areSame( 'Test\'s &amp; "quote"', CKEDITOR.tools.htmlEncode( 'Test\'s & "quote"' ) );
'test htmlDecode - numeric entities': function() {
assert.areSame( '\u0001 \u000a \u00ff \uffff \u000c', htmlDecode( '&#1; &#10; &#255; &#65535; &#0012;' ) );
},

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.' );
'test htmlDecode - duplications': function() {
assert.areSame( '<a & b ><a & b >', htmlDecode( '&lt;a &amp; b &gt;&lt;a &amp; b &gt;' ) );
},

test_htmlDecode: function() {
assert.areSame( '<a & b >', CKEDITOR.tools.htmlDecode( '&lt;a &amp; b &gt;' ), 'Invalid result for htmlDecode' );
assert.areSame( '<a & b ><a & b >', CKEDITOR.tools.htmlDecode( '&lt;a &amp; b &gt;&lt;a &amp; b &gt;' ), 'Invalid result for htmlDecode' );
'test htmlDecode - double encoding': function() {
assert.areSame( '&lt; &amp; &gt; &nbsp; &shy;', htmlDecode( '&amp;lt; &amp;amp; &amp;gt; &amp;nbsp; &amp;shy;' ) );
},

test_htmlEncode_3874: function() {
assert.areSame( 'line1\nline2', CKEDITOR.tools.htmlEncode( 'line1\nline2' ) );
'test htmlDecode - triple encoding': function() {
assert.areSame( '&amp;lt; &amp;amp; &amp;gt;', htmlDecode( '&amp;amp;lt; &amp;amp;amp; &amp;amp;gt;' ) );
},

test_htmlEncodeAttr: function() {
assert.areSame( '&lt;a b=&quot;c&quot;/&gt;', CKEDITOR.tools.htmlEncodeAttr( '<a b="c"/>' ) );
'test htmlEncodeAttr - all covered entities': function() {
assert.areSame( '&lt;a b=&quot;c&amp;d&quot;/&gt;', CKEDITOR.tools.htmlEncodeAttr( '<a b="c&d"/>' ) );
},

'test htmlDecodeAttr - all covered entities': function() {
assert.areSame( '< " > & \u00a0 \u00ad \u000a', CKEDITOR.tools.htmlDecodeAttr( '&lt; &quot; &gt; &amp; &nbsp; &shy; &#10;' ) );
},

'test htmlDecodeAttr - double encoding': function() {
assert.areSame( '&lt; &quot; &gt; &amp;', CKEDITOR.tools.htmlDecodeAttr( '&amp;lt; &amp;quot; &amp;gt; &amp;amp;' ) );
},

test_cssStyleToDomStyle1: function() {
Expand Down Expand Up @@ -598,5 +616,4 @@
assert.areSame( 33, uuid.length, 'UUID.length' );
}
} );

} )();
14 changes: 13 additions & 1 deletion tests/plugins/bbcode/bbcode.js
Expand Up @@ -65,7 +65,19 @@ bender.test( {

// #8995
'test escape HTML entities in bbcode': function() {
var html = '<a href="foo&amp;bar">foo&lt;bar&gt;</a>', bbcode = '[url=foo&bar]foo<bar>[/url]';
var html = '<a href="foo&amp;bar">&amp;foo&lt;bar&gt;</a>', bbcode = '[url=foo&bar]&foo<bar>[/url]';
this.assertToHtml( html, bbcode );
this.assertToBBCode( bbcode, html );
},

'test escape HTML entities in bbcode - double encoding': function() {
var html = '<a href="foo&amp;amp;bar&amp;amp;lt;">foo&amp;lt;bar&amp;gt;bom&amp;amp;lt;</a>', bbcode = '[url=foo&amp;bar&amp;lt;]foo&lt;bar&gt;bom&amp;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; &amp;amp;amp;', bbcode = '& &amp; &amp;amp;';
this.assertToHtml( html, bbcode );
this.assertToBBCode( bbcode, html );
},
Expand Down
2 changes: 1 addition & 1 deletion tests/plugins/link/mail_link.js
Expand Up @@ -10,7 +10,7 @@ bender.editor = {
};

var protectedMailLink = '<a href=\"javascript:void(location.href=\'mailto:\'+String.fromCharCode(106,111,98,64,99,107,115,111,117,114,99,101,46,99,111,109)' +
'+\'?subject=Job%20Request&body=I\\\'m%20looking%20for%20the%20AJD%20position.\')\">AJD</a>';
'+\'?subject=Job%20Request&amp;body=I\\\'m%20looking%20for%20the%20AJD%20position.\')\">AJD</a>';

bender.test( {
'test created protected mail link': function() {
Expand Down
92 changes: 92 additions & 0 deletions tests/tickets/13105/1.js
@@ -0,0 +1,92 @@
/* bender-tags: htmldataprocessor,htmlwriter */
/* bender-ckeditor-plugins: htmlwriter */

'use strict';

var attrValue = '<p x="1&quot;2">\'"& &lt;a y="3"&gt;&amp;&lt;/a&gt;</p>',
encodedAttrValue = '&lt;p x=&quot;1&amp;quot;2&quot;&gt;\'&quot;&amp; &amp;lt;a y=&quot;3&quot;&amp;gt;&amp;amp;&amp;lt;/a&amp;gt;&lt;/p&gt;';

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,
enabled = false;

editor.editable().setHtml( '<p><span>foo</span></p>' );

editor.dataProcessor.htmlFilter.addRules( {
elements: {
span: function( el ) {
// Affect only this test.
if ( enabled ) {
el.attributes[ 'data-foo' ] = attrValue;
}
}
}
} );

enabled = true;

var data = editor.getData();
assert.areSame( '<p><span data-foo="' + encodedAttrValue + '">foo</span></p>', 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' ), 'attribute was properly preserved when loading data' );
} );
}

bender.test( {
'test inline editor with htmlwriter preserves attribute set in htmlFilter': function() {
doTestProcessorAttributeIsPreserved( bender.editorBots.htmlWriter );
},

'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( '<p><span>x</span></p>' );
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 );

var el2 = CKEDITOR.document.createElement( 'span' );
el2.setHtml( el.getOuterHtml() );
assert.areSame( attrValue, el2.getFirst().data( 'foo' ) );
}
} );

0 comments on commit 2c45c76

Please sign in to comment.