Merge pull request #2381 from ckeditor/t/2294

Fixed issues with applying inline styles to HTML comments

CHANGES.md

@@ -10,6 +10,8 @@ Fixed Issues:
 * [#1489](https://github.com/ckeditor/ckeditor-dev/issues/1489): Fixed: [Table Selection](https://docs.ckeditor.com/ckeditor4/latest/api/CKEDITOR_plugins_tableselection.html) table contents can be removed in readonly mode.
 * [#586](https://github.com/ckeditor/ckeditor-dev/issues/586) Fixed: required attribute not being correctly recognized by form dialog. Thanks to [Roli Züger](https://github.com/rzueger)!
 * [#1264](https://github.com/ckeditor/ckeditor-dev/issues/1264) Fixed: Right-click doesn't clear selection created with [Table Selection](https://ckeditor.com/cke4/addon/tableselection) plugin.
+* [#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.
 
 API Changes:
 

core/style.js

@@ -878,8 +878,8 @@ CKEDITOR.STYLE_OBJECT = 3;
 					nodeIsReadonly = nodeName && ( currentNode.getAttribute( 'contentEditable' ) == 'false' ),
 					nodeIsNoStyle = nodeName && currentNode.getAttribute( 'data-nostyle' );
 
-				// Skip bookmarks.
-				if ( nodeName && currentNode.data( 'cke-bookmark' ) ) {
+				// Skip bookmarks or comments.
+				if ( ( nodeName && currentNode.data( 'cke-bookmark' ) ) || currentNode.type === CKEDITOR.NODE_COMMENT ) {
 					currentNode = currentNode.getNextSourceNode( true );
 					continue;
 				}
@@ -910,9 +910,9 @@ CKEDITOR.STYLE_OBJECT = 3;
 							var includedNode = currentNode;
 							var parentNode;
 
-							// This node is about to be included completelly, but,
+							// This node is about to be included completely, but,
 							// if this is the last node in its parent, we must also
-							// check if the parent itself can be added completelly
+							// check if the parent itself can be added completely
 							// to the range, otherwise apply the style immediately.
 							while (
 								( applyStyle = !includedNode.getNext( notBookmark ) ) &&

tests/core/style/applyremove.html

@@ -1 +1,42 @@
 <div id="playground" contenteditable="true"></div>
+<textarea name="html-comments-bold" id="html-comments-bold">
+	<p>foo</p><!--/end logo --><p> </p><!-- begin: body --><p>a<br />a</p>
+=>
+	<p><strong>foo</strong></p>
+	<!--/end logo -->
+	<p><strong>&nbsp;</strong></p>
+	<!-- begin: body -->
+	<p><strong>a<br />a</strong></p>
+</textarea>
+
+<textarea name="html-comments-between-blocks" id="html-comments-between-blocks">
+	<p>foo</p><!-- com --><p>bar</p>
+=>
+	<p><strong>foo</strong></p>
+	<!-- com -->
+	<p><strong>bar</strong></p>
+</textarea>
+
+<textarea name="html-comments-between-inline" id="html-comments-between-inline">
+	<p><em>foo</em><!-- com --><em>bar</em></p>
+=>
+	<p>
+		<strong>
+			<em>foo</em>
+			<!-- com -->
+			<em>bar</em>
+		</strong>
+	</p>
+</textarea>
+
+<textarea name="html-comments-tailing-inline" id="html-comments-tailing-inline">
+	<p>
+		foo
+		<!-- com -->
+	</p>
+=>
+	<p>
+		<strong>foo</strong>
+		<!-- com -->
+	</p>
+</textarea>

tests/core/style/applyremove.js

@@ -264,6 +264,15 @@
 				'<b lang="pt" style="color:red; font-size:11pt">this<b style="font-weight:700"> is some sample text</b></b>' );
 		},
 
+		// (#2294, #2380)
+		'test inline style apply to HTML comments': createInlineStyleTestCase( 'html-comments-bold' ),
+
+		// (#2294, #2380)
+		'test HTML comments between blocks': createInlineStyleTestCase( 'html-comments-between-blocks' ),
+
+		// (#2294, #2380)
+		'test HTML comments between inline': createInlineStyleTestCase( 'html-comments-between-inline' ),
+
 		test_inline_nobreak1: function() {
 			playground.setHtml( 'this is <a href="http://example.com/">some sample</a> text' );
 
@@ -1044,4 +1053,18 @@
 
 	bender.test( tcs );
 
+	function createInlineStyleTestCase( fixtureId ) {
+		return function() {
+			bender.tools.testInputOut( fixtureId, function( inputHtml, expectedHtml ) {
+				playground.setHtml( CKEDITOR.tools.trim( inputHtml ) );
+
+				var rng = new CKEDITOR.dom.range( doc );
+				rng.selectNodeContents( playground );
+
+				getStyle( { element: 'strong' } ).applyToRange( rng );
+
+				assert.beautified.html( expectedHtml, playground.getHtml() );
+			} );
+		};
+	}
 } )();

tests/plugins/basicstyles/basicstyles.html

@@ -0,0 +1,13 @@
+<textarea name="html-comments-bold" id="html-comments-bold">
+	[<p>foo</p>
+	<!--/end logo -->
+	<p> </p>
+	<!-- begin: body -->
+	<p>a<br />a</p>]
+=>
+	<p><strong>foo</strong></p>
+	<!--/end logo -->
+	<p><strong>&nbsp;</strong></p>
+	<!-- begin: body -->
+	<p><strong>a<br />a</strong></p>
+</textarea>

tests/plugins/basicstyles/basicstyles.js

@@ -1,7 +1,11 @@
 /* bender-tags: editor */
 /* bender-ckeditor-plugins: basicstyles,toolbar */
 
-bender.editor = { config: { autoParagraph: false } };
+bender.editor = {
+	config: {
+		autoParagraph: false
+	}
+};
 
 bender.test( {
 	'test apply range style across input element': function() {
@@ -10,5 +14,16 @@ bender.test( {
 		bot.setHtmlWithSelection( 'te[xt<input type="button" />te]xt' );
 		bot.execCommand( 'bold' );
 		assert.areSame( 'te<strong>xt<input type="button" />te</strong>xt', bot.getData( false, true ) );
+	},
+
+	// (#2294)
+	'test apply bold to content with HTML comments': function() {
+		var bot = this.editorBot;
+		bender.tools.testInputOut( 'html-comments-bold', function( inputHtml, expectedHtml ) {
+			bot.setHtmlWithSelection( inputHtml );
+			bot.execCommand( 'bold' );
+
+			assert.beautified.html( expectedHtml, bot.editor.getData() );
+		} );
 	}
 } );

tests/plugins/basicstyles/manual/htmlcomments.html

@@ -0,0 +1,12 @@
+<textarea cols="80" id="editor1" name="editor1" rows="10">
+&lt;p&gt;foo&lt;/p&gt;&#10;&lt;!--/end logo --&gt;&#10;&#10;&lt;p&gt;&NonBreakingSpace;&lt;/p&gt;&#10;&lt;!-- Begin: Body --&gt;&#10;&#10;&lt;p&gt;a&lt;br /&gt;a&lt;/p&gt;
+</textarea>
+
+<script>
+	if ( bender.tools.env.mobile ) {
+		bender.ignore();
+	}
+
+	CKEDITOR.replace( 'editor1' );
+
+</script>

tests/plugins/basicstyles/manual/htmlcomments.md

@@ -0,0 +1,30 @@
+@bender-tags: bug, 2294, 2380, 4.10.2
+@bender-ui: collapsed
+@bender-ckeditor-plugins: wysiwygarea, toolbar, basicstyles, sourcearea, htmlwriter
+
+1. Open the console.
+1. Focus the editor.
+1. Select all the contents (`ctrl/cmd+a`).
+1. Click the "Bold" button in the toolbar.
+
+	### Expected
+
+	No errors are thrown in the console.
+
+	### Actual
+
+	JS error is thrown, like `Uncaught TypeError: Cannot read property 'getParents' of null`
+
+1. Click the "Source" button.
+
+	### Expected
+
+	HTML comments that are ouside of paragraphs, are not wrapped with additional `p` or `strong` tags.
+
+	### Actual
+
+	Additional markup is created around HTML comments.
+
+### Notes
+
+[Edge will throw `Permission denied` exception](https://github.com/ckeditor/ckeditor-dev/issues/2035), this particular error is not related to this test.