Fixed config.forceSimpleAmpersand

CHANGES.md

@@ -13,6 +13,7 @@ Fixed Issues:
 * [#2380](https://github.com/ckeditor/ckeditor-dev/issues/2380) Fixed: Styling HTML comments in top level element result with extra paragraphs.
 * [#2294](https://github.com/ckeditor/ckeditor-dev/issues/2294) Fixed: Pasting content from MS Outlook and then bolding it results with an error.
 * [#2035](https://github.com/ckeditor/ckeditor-dev/issues/2035) [Edge] Fixed: `Permission denied` is thrown when opening [Panel](https://ckeditor.com/cke4/addon/panel) instance.
+* [#965](https://github.com/ckeditor/ckeditor-dev/issues/965) Fixed: The [`config.forceSimpleAmpersand`](https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_config.html#cfg-forceSimpleAmpersand) option does no longer work. Thanks to [Alex Maris](https://github.com/alexmaris)!
 
 API Changes:
 

plugins/htmlwriter/plugin.js

@@ -179,9 +179,13 @@ CKEDITOR.htmlWriter = CKEDITOR.tools.createClass( {
 		attribute: function( attName, attValue ) {
 
 			if ( typeof attValue == 'string' ) {
-				this.forceSimpleAmpersand && ( attValue = attValue.replace( /&/g, '&' ) );
 				// Browsers don't always escape special character in attribute values. (https://dev.ckeditor.com/ticket/4683, https://dev.ckeditor.com/ticket/4719).
 				attValue = CKEDITOR.tools.htmlEncodeAttr( attValue );
+
+				// Run ampersand replacement after the htmlEncodeAttr, otherwise the results are overwritten (#965).
+				if ( this.forceSimpleAmpersand ) {
+					attValue = attValue.replace( /&/g, '&' );
+				}
 			}
 
 			this._.output.push( ' ', attName, '="', attValue, '"' );

tests/plugins/htmlwriter/htmlwriter.js

@@ -39,5 +39,28 @@ bender.test( {
 				assert.areSame( afterFormat, bot.getData( false, false ) );
 			} );
 		} );
+	},
+
+	// (#965)
+	'test config.forceSimpleAmpersand works in HTML element attributes': function() {
+		var data = '<p><a href="http://www.blah.com?foo=1&bar=2">Test link</a></p>';
+
+		bender.editorBot.create( {
+			name: 'forceSimpleAmpersand',
+			formattedOutput: true,
+			config: {
+				extraAllowedContent: 'a[href]',
+				forceSimpleAmpersand: true
+			}
+		}, function( bot ) {
+			bot.editor.dataProcessor.writer.setRules( 'p', {
+				indent: false,
+				breakAfterClose: false
+			} );
+
+			bot.setData( data, function() {
+				assert.areSame( data, bot.getData( false, false ) );
+			} );
+		} );
 	}
 } );

tests/plugins/htmlwriter/manual/forcesimpleampersand.html

@@ -0,0 +1,33 @@
+<h2>Divarea editor:</h2>
+<div contenteditable="true" id="editor1">
+	<p><a href="https://foo.bar?foo=1&bar=2">Test link</a></p>
+</div>
+
+<h2>classic editor:</h2>
+<textarea id="editor2">
+	<p><a href="https://foo.bar?foo=1&bar=2">Test link</a></p>
+</textarea>
+
+<h2>Inline editor:</h2>
+<div contenteditable="true" id="editor3" style="width:500px">
+	<p><a href="https://foo.bar?foo=1&bar=2">Test link</a></p>
+</div>
+
+<script>
+	CKEDITOR.replace( 'editor1', {
+		forceSimpleAmpersand: true,
+		extraPlugins: 'divarea',
+		width: 500
+	} );
+
+	CKEDITOR.replace( 'editor2', {
+		forceSimpleAmpersand: true,
+		width: 500
+	} );
+
+	CKEDITOR.inline( 'editor3', {
+		forceSimpleAmpersand: true,
+		extraPlugins: 'floatingspace,sourcedialog',
+		width: 500
+	} );
+</script>

tests/plugins/htmlwriter/manual/forcesimpleampersand.md

@@ -0,0 +1,13 @@
+@bender-ui: collapsed
+@bender-tags: 4.10.2, 965, bug,htmlwriter
+@bender-ckeditor-plugins: wysiwygarea,toolbar,link,htmlwriter,sourcearea
+
+1. Click the "Source" button.
+	## Expected
+
+	`a[href]` attribute is equal to `https://foo.bar?foo=1&bar=2` note `&` character it should not be encoded into an entity.
+
+	## Unexpected
+
+	`&` character in `a[href]` gets encoded to `&`.
+1. Repeat steps for other editors.