Merge branch 't/14856'

ckeditor · Jan 4, 2017 · eab8e4b · eab8e4b
2 parents d0960ba + 7b82072
commit eab8e4b
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 109 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -9,6 +9,7 @@ New Features:
 
 Fixed Issues:
 
+* [#14856](http://dev.ckeditor.com/ticket/14856): Fixed: Font size and font family reset each other when modified at certain positions.
 * [#13818](http://dev.ckeditor.com/ticket/13818): Fixed: Allow to group [Widget](http://ckeditor.com/addon/widget) [style definitions](http://docs.ckeditor.com/#!/guide/dev_styles-section-widget-styles), so applying one style disables the other.
 * [#16745](http://dev.ckeditor.com/ticket/16745): [Edge] Fixed: List items are lost when pasted from word.
 * [#16682](http://dev.ckeditor.com/ticket/16682): [Edge] Fixed: List gets pasted as a set of paragraphs. Added [`config.pasteFromWord_heuristicsEdgeList`](http://docs.ckeditor.com/#!/api/CKEDITOR.config-cfg-pasteFromWord_heuristicsEdgeList) option.

diff --git a/plugins/font/plugin.js b/plugins/font/plugin.js
@@ -137,11 +137,10 @@
 								matching.remove();
 								range.moveToBookmark( bm );
 
-							// If we are at the boundary of the style element, just move out.
-							} else if ( startBoundary ) {
-								range.moveToPosition( matching, CKEDITOR.POSITION_BEFORE_START );
-							} else if ( endBoundary ) {
-								range.moveToPosition( matching, CKEDITOR.POSITION_AFTER_END );
+							// If we are at the boundary of the style element, move out and copy nested styles/elements.
+							} else if ( startBoundary || endBoundary ) {
+								range.moveToPosition( matching, startBoundary ? CKEDITOR.POSITION_BEFORE_START : CKEDITOR.POSITION_AFTER_END );
+								cloneSubtreeIntoRange( range, path.elements.slice(), matching );
 							} else {
 								// Split the element and clone the elements that were in the path
 								// (between the startContainer and the matching element)

diff --git a/tests/plugins/font/font.js b/tests/plugins/font/font.js
@@ -51,116 +51,64 @@
 			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>{foo}</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 48 );
-				assert.isInnerHtmlMatching( '<p><span style="font-size:48px">foo</span>@</p>',
-					bot.editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 48, false, bot, '<p><span style="font-size:48px">foo</span>@</p>' );
 		},
 
 		'test apply font size over another font size (collapsed selection)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px">f{}oo</span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-
-				this.wait( function() {
-					// We lose (dunno where) the empty span on IE8, so let's insert something.
-					editor.insertText( 'bar' );
-					assert.isInnerHtmlMatching(
-						'<p>x<span style="font-size:48px">f</span><span style="font-size:24px">bar</span><span style="font-size:48px">oo</span>x@</p>',
-						editor.editable().getHtml(), htmlMatchingOpts );
-				}, 0 );
-			} );
+			this.assertCombo( 'FontSize', 24, true, bot,
+				'<p>x<span style="font-size:48px">f</span><span style="font-size:24px">bar</span><span style="font-size:48px">oo</span>x@</p>' );
 		},
 
 		'test apply font size over another font size (collapsed selection in empty span)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px"><em>[]</em></span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-
-				this.wait( function() {
-					// We lose (dunno where) the empty span on IE8, so let's insert something.
-					editor.insertText( 'bar' );
-					assert.isInnerHtmlMatching( '<p>x<em><span style="font-size:24px">bar</span></em>x@</p>', editor.editable().getHtml(), htmlMatchingOpts );
-					assert.areSame( 1, editor.editable().find( 'span' ).count(), 'there is only one span in the editable' );
-				}, 0 );
-			} );
+			this.assertCombo( 'FontSize', 24, true, bot,
+				'<p>x<em><span style="font-size:24px">bar</span></em>x@</p>',
+				function( bot ) {
+					assert.areSame( 1, bot.editor.editable().find( 'span' ).count(), 'there is only one span in the editable' );
+				} );
 		},
 
 		'test apply font size over another font size (collapsed selection at the existing span boundary)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px">{}foo</span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-
-				this.wait( function() {
-					// We lose (dunno where) the empty span on IE8, so let's insert something.
-					editor.insertText( 'bar' );
-					assert.isInnerHtmlMatching(
-						'<p>x<span style="font-size:24px">bar</span><span style="font-size:48px">foo</span>x@</p>',
-						editor.editable().getHtml(), htmlMatchingOpts );
-				}, 0 );
-			} );
+			this.assertCombo( 'FontSize', 24, true, bot,
+				'<p>x<span style="font-size:24px">bar</span><span style="font-size:48px">foo</span>x@</p>' );
 		},
 
 		'test apply font size over another font size (text selection)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px">f{o}o</span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="font-size:48px">f</span><span style="font-size:24px">o</span><span style="font-size:48px">o</span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 24, false, bot,
+				'<p>x<span style="font-size:48px">f</span><span style="font-size:24px">o</span><span style="font-size:48px">o</span>x@</p>' );
 		},
 
 		'test apply font size over another font size (the existing span selection)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px">{foo}</span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="font-size:24px">foo</span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 24, false, bot, '<p>x<span style="font-size:24px">foo</span>x@</p>' );
 		},
 
 		'test apply font size over another font size (selection containing other span)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x{f<span style="font-size:48px">o</span>o}x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="font-size:24px">foo</span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 24, false, bot, '<p>x<span style="font-size:24px">foo</span>x@</p>' );
 		},
 
 		'test apply font size over another font size (deeply nested text selection)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px"><em>f{o}o</em></span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="font-size:48px"><em>f</em></span><span style="font-size:24px"><em>o</em></span><span style="font-size:48px"><em>o</em></span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 24, false, bot,
+				'<p>x<span style="font-size:48px"><em>f</em></span><span style="font-size:24px"><em>o</em></span><span style="font-size:48px"><em>o</em></span>x@</p>' );
 		},
 
 		'test apply font size over another font size (deeply nested collapsed selection)': function() {
@@ -169,47 +117,69 @@
 				assert.ignore();
 			}
 
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:48px"><em>f<u class="y">{}o</u>o</em></span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-
-				this.wait( function() {
-					editor.insertText( 'bar' );
-					assert.isInnerHtmlMatching(
-						'<p>x<span style="font-size:48px"><em>f</em></span>' +
-						'<em><u class="y"><span style="font-size:24px">bar</span></u></em>' +
-						'<span style="font-size:48px"><em><u class="y">o</u>o</em></span>x@</p>',
-						editor.editable().getHtml(), htmlMatchingOpts );
-				}, 0 );
-			} );
+			this.assertCombo( 'FontSize', 24, true, bot,
+				'<p>x<span style="font-size:48px"><em>f</em></span><em><u class="y"><span style="font-size:24px">bar</span></u></em>' +
+				'<span style="font-size:48px"><em><u class="y">o</u>o</em></span>x@</p>' );
 		},
 
 		'test apply font size over font family (check possible false positive match)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="' + ffArial + '">f{o}o</span>x</p>' );
-			bot.combo( 'FontSize', function( combo ) {
-				combo.onClick( 24 );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="' + ffArial + '">f<span style="font-size:24px">o</span>o</span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
-			} );
+			this.assertCombo( 'FontSize', 24, false, bot, '<p>x<span style="' + ffArial + '">f<span style="font-size:24px">o</span>o</span>x@</p>' );
 		},
 
 		'test apply font family over another font family (text selection)': function() {
-			var bot = this.editorBot,
-				editor = bot.editor;
+			var bot = this.editorBot;
 
 			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="' + ffArial + '">f{o}o</span>x</p>' );
-			bot.combo( 'Font', function( combo ) {
-				combo.onClick( 'Comic Sans MS' );
-				assert.isInnerHtmlMatching(
-					'<p>x<span style="' + ffArial + '">f</span><span style="' + ffCS + '">o</span><span style="' + ffArial + '">o</span>x@</p>',
-					editor.editable().getHtml(), htmlMatchingOpts );
+			this.assertCombo( 'Font', 'Comic Sans MS', false, bot,
+				'<p>x<span style="' + ffArial + '">f</span><span style="' + ffCS + '">o</span><span style="' + ffArial + '">o</span>x@</p>' );
+		},
+
+		// #14856
+		'test reapply font family on the beginning (collapsed selection)': function() {
+			if ( CKEDITOR.env.safari ) {
+				assert.ignore();
+			}
+
+			var bot = this.editorBot;
+
+			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="' + ffArial + '"><em>{}foo</em></span>x</p>' );
+			this.assertCombo( 'Font', 'Comic Sans MS', true, bot,
+				'<p>x<em><span style="' + ffCS + '">bar</span></em><span style="' + ffArial + '"><em>foo</em></span>x@</p>' );
+		},
+
+		'test reapply font family in the middle (collapsed selection)': function() {
+			var bot = this.editorBot;
+
+			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="' + ffArial + '"><em>fo{}o</em></span>x</p>' );
+			this.assertCombo( 'Font', 'Comic Sans MS', true, bot,
+				'<p>x<span style="' + ffArial + '"><em>fo</em></span><em><span style="' + ffCS + '">bar</span></em>' +
+				'<span style="' + ffArial + '"><em>o</em></span>x@</p>' );
+		},
+
+		'test reapply font size on the end (collapsed selection)': function() {
+			var bot = this.editorBot;
+
+			bender.tools.selection.setWithHtml( bot.editor, '<p>x<span style="font-size:12px"><em>foo{}</em></span>x</p>' );
+			this.assertCombo( 'FontSize', 24, true, bot,
+				'<p>x<span style="font-size:12px"><em>foo</em></span><em><span style="font-size:24px">bar</span></em>x@</p>' );
+		},
+
+		assertCombo: function( comboName, comboValue, collapsed, bot, resultHtml, callback ) {
+			bot.combo( comboName, function( combo ) {
+				combo.onClick( comboValue );
+
+				this.wait( function() {
+					// The empty span from collapsed selection is lost on FF and IE8, insert something to prevent that.
+					collapsed && bot.editor.insertText( 'bar' );
+					assert.isInnerHtmlMatching( resultHtml, bot.editor.editable().getHtml(), htmlMatchingOpts );
+					callback && callback( bot );
+				}, 0 );
 			} );
 		}
 	} );

diff --git a/tests/plugins/font/manual/reapplystyles.html b/tests/plugins/font/manual/reapplystyles.html
@@ -0,0 +1,7 @@
+<div id="editor">
+	<p><span style="font-family:Georgia,serif;"><span style="font-size:24px;"><strong><em>This is styled text</em></strong></span></span></p>
+</div>
+
+<script>
+	CKEDITOR.replace( 'editor', { height: 400 } );
+</script>
diff --git a/tests/plugins/font/manual/reapplystyles.md b/tests/plugins/font/manual/reapplystyles.md
@@ -0,0 +1,29 @@
+@bender-tags: 4.6.2, tc, 14856
+@bender-ui: collapsed
+@bender-ckeditor-plugins: wysiwygarea, toolbar, font, enterkey, elementspath, basicstyles
+
+## Scenario 1
+
+1. Place caret at the beginning of the text.
+1. Change font family using Font dropdown.
+1. Type something.
+
+## Scenario 2
+
+1. Place caret in the middle of the text.
+1. Change font family using Font dropdown.
+1. Type something.
+
+## Scenario 3
+
+1. Place caret at the end of the text so that existing styles are recognized.
+1. Change font size using Size dropdown.
+1. Type something.
+
+### Expected:
+Styles are preserved and typed text is styled in the same way as the rest of the text (with the exception of the styles that were changed).
+
+### Unexpected:
+Some styles are lost after changing font family/size. Typed text is styled differently
+(except the one changed style) than the rest of the text.
+