Merge branch 't/10750'

CHANGES.md

@@ -5,6 +5,7 @@ CKEditor 4 Changelog
 
 Fixed Issues:
 
+* [#10750](http://dev.ckeditor.com/ticket/10750): Fixed: The editor don't unquote the font-family style property.
 * [#14413](http://dev.ckeditor.com/ticket/14413): Fixed: Autogrow plugin with `autoGrow_onStartup` option set to `true` does not work properly for not visible editor.
 * [#14451](http://dev.ckeditor.com/ticket/14451): Fixed: Numeric element id not escaped properly. Thanks to Jakub Chalupa!
 * [#14701](http://dev.ckeditor.com/ticket/14701): Fixed: More precise labels for image and placeholder widgets.

core/style.js

@@ -1794,15 +1794,28 @@ CKEDITOR.STYLE_OBJECT = 3;
 	// is treated as a wildcard which will match any value.
 	// @param {Object/String} source
 	// @param {Object/String} target
+	// @returns {Boolean}
 	function compareCssText( source, target ) {
+		function filter( string, propertyName ) {
+			// In case of font-families we'll skip quotes. (#10750)
+			return propertyName.toLowerCase() == 'font-family' ? string.replace( /["']/g, '' ) : '';
+		}
+
 		if ( typeof source == 'string' )
 			source = CKEDITOR.tools.parseCssText( source );
 		if ( typeof target == 'string' )
 			target = CKEDITOR.tools.parseCssText( target, true );
 
 		for ( var name in source ) {
-			if ( !( name in target && ( target[ name ] == source[ name ] || source[ name ] == 'inherit' || target[ name ] == 'inherit' ) ) )
+			if ( !( name in target ) ) {
+				return false;
+			}
+
+			if ( !( filter( target[ name ], name ) == filter( source[ name ], name ) ||
+				source[ name ] == 'inherit' ||
+				target[ name ] == 'inherit' ) ) {
 				return false;
+			}
 		}
 		return true;
 	}

core/tools.js

@@ -1028,10 +1028,9 @@
 			styleText.replace( /"/g, '"' ).replace( /\s*([^:;\s]+)\s*:\s*([^;]+)\s*(?=;|$)/g, function( match, name, value ) {
 				if ( normalize ) {
 					name = name.toLowerCase();
-					// Normalize font-family property, ignore quotes and being case insensitive. (#7322)
-					// http://www.w3.org/TR/css3-fonts/#font-family-the-font-family-property
+					// Drop extra whitespacing from font-family.
 					if ( name == 'font-family' )
-						value = value.toLowerCase().replace( /["']/g, '' ).replace( /\s*,\s*/g, ',' );
+						value = value.replace( /\s*,\s*/g, ',' );
 					value = CKEDITOR.tools.trim( value );
 				}
 

tests/core/style/checkelementmatch.html

@@ -2,3 +2,4 @@ <h2 id="italictitle"  style="font-style:italic;" data-cke-editable="true">Italic
 <h3 id="subtitle" style="color:#a1a2a3;font-style:italic;" data-cke-editable="true">Subtitle</h3>
 <div id="specialcontainer" style="background:#eee;border:1px solid #ccc;padding:5px 10px;" data-cke-editable="true">Special container</div>
 <span id="yellow" style="color: rgb(255, 255, 0);" data-cke-editable="true">Yellow</span>
+<span id="font" style="font-family: 'Univers LT Std'" data-cke-editable="true">Font</span>

tests/core/style/checkelementmatch.js

@@ -21,6 +21,12 @@
 			assert.isTrue( stylesYellow.checkElementMatch( CKEDITOR.document.getById( 'yellow' ), true ), 'Yellow styles match (short).' );
 			assert.isTrue( stylesYellowUpper.checkElementMatch( CKEDITOR.document.getById( 'yellow' ), true ), 'Yellow styles match (long, upper-case).' );
 			assert.isTrue( stylesYellowUpperShort.checkElementMatch( CKEDITOR.document.getById( 'yellow' ), true ), 'Yellow styles match (short, upper-case).' );
+		},
+		'test styles font-family ignore quotes': function() {
+			var styleFontMatch = new CKEDITOR.style( { element: 'span', attributes: { 'style': 'font-family:Univers LT Std;' } } ),
+				styleFontNoMatch = new CKEDITOR.style( { element: 'span', attributes: { 'style': 'font-family:Verdana;' } } );
+			assert.isTrue( styleFontMatch.checkElementMatch( CKEDITOR.document.getById( 'font' ), true ), 'Font styles match.' );
+			assert.isFalse( styleFontNoMatch.checkElementMatch( CKEDITOR.document.getById( 'font' ), true ), 'Font styles match.' );
 		}
 	} );
 } )();

tests/core/style/checkelementremovable.js

@@ -83,10 +83,10 @@ bender.test( {
 	},
 
 	test_checkElementRemovable_fontFamily: function() {
-		var element = CKEDITOR.dom.element.createFromHtml( '<span style="font-family: georgia,  serif">Test Font</span>', doc );
+		var element = CKEDITOR.dom.element.createFromHtml( '<span style="font-family: Georgia,  serif">Test Font</span>', doc );
 		playground.append( element );
 
 		var style = new CKEDITOR.style( { element: 'span', styles: { 'font-family': '#(family)' } }, { family: 'Georgia, serif;' } );
 		assert.isTrue( style.checkElementRemovable( element, true ) );
 	}
-} );
+} );

tests/core/tools.js

@@ -11,12 +11,13 @@
 	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 );
-		},
+	function assertNormalizeCssText( expected, input, message ) {
+		return function() {
+			assert.areSame( expected, CKEDITOR.tools.normalizeCssText( input ), message );
+		};
+	}
 
+	bender.test( {
 		test_extend: function() {
 			function fakeFn() {}
 
@@ -294,12 +295,7 @@
 			assert.isTrue( c instanceof A && c instanceof B && c instanceof C, 'check instanceof both A & B & C' );
 		},
 
-		testNormalizeCssText: function() {
-			this.assertNormalizedCssText(
-				'color:red;font-size:10px;width:10.5em;', 'style1', 'order, lowercase and white spaces' );
-
-			this.assertNormalizedCssText( 'color:red;font-family:arial black,helvetica,georgia;', 'style2', 'font names' );
-		},
+		testNormalizeCssText: assertNormalizeCssText( 'color:red;font-size:10px;width:10.5em;', ' width: 10.5em ; COLOR : red; font-size:10px  ; ', 'order, lowercase and white spaces' ),
 
 		testNormalizeCssText2: function() {
 			var n = CKEDITOR.tools.normalizeCssText;
@@ -332,6 +328,47 @@
 				n( 'color: red; width: 10px; margin: 0.5em; float: left', true ), 'various' );
 		},
 
+		testQuoteEntity: assertNormalizeCssText( 'font-family:"foo";', 'font-family: "foo";', '' ),
+
+		// (#10750)
+		'test Normalize double quote': assertNormalizeCssText( 'font-family:"crazy font";', 'font-family: "crazy font";',
+			'quoted font name' ),
+		'test Normalize single quote': assertNormalizeCssText( 'font-family:\'crazy font\';', 'font-family: \'crazy font\';',
+			'single-quoted font name' ),
+
+		'test Normalize generic family name serif': assertNormalizeCssText( 'font-family:serif;', 'font-family: serif;',
+			'generic-family name is not escaped' ),
+		'test Normalize generic family name sans-serif': assertNormalizeCssText( 'font-family:sans-serif;', 'font-family: sans-serif;',
+			'generic-family name is not escaped' ),
+		'test Normalize generic family name cursive': assertNormalizeCssText( 'font-family:cursive;', 'font-family: cursive;',
+			'generic-family name is not escaped' ),
+		'test Normalize generic family name fantasy': assertNormalizeCssText( 'font-family:fantasy;', 'font-family: fantasy;',
+			'generic-family name is not escaped' ),
+		'test Normalize generic family name monospace': assertNormalizeCssText( 'font-family:monospace;', 'font-family: monospace;',
+			'generic-family name is not escaped' ),
+
+		'test Normalize generic and non-generic mix': assertNormalizeCssText( 'font-family:"foo",serif;', 'font-family: "foo", serif;',
+			'family-name and generic-family mix' ),
+		'test Normalize letter casing sensitivity': assertNormalizeCssText( 'font-family:"FFo baR";', 'font-family: "FFo baR";',
+			'letter casing sensivity' ),
+		// It's also possible to use font named as any generic-family member as long as it's enclosed within quotes.
+		'test Normalize generic-family token as family-name': assertNormalizeCssText( 'font-family:"serif";', 'font-family:"serif";',
+			'accept generic-family token as family-name' ),
+		'test Normalize unquoted family name with hyphen': assertNormalizeCssText( 'font-family:my-cool-font;', 'font-family:my-cool-font;',
+			'unquoted family name with hyphen' ),
+		'test Normalize font name with multiple spaces': assertNormalizeCssText( 'font-family:"Space    font";', 'font-family:"Space    font";',
+			'font name with multiple spaces' ),
+
+		'test Normalize family name with quotes': assertNormalizeCssText( 'font-family:"\'Sarcasm\'";', 'font-family:"\'Sarcasm\'";',
+			'family name with quotes' ),
+		'test Normalize family name with special characters': assertNormalizeCssText( 'font-family:"\'This is   -!$   custom Font\'";', 'font-family:"\'This is   -!$   custom Font\'";',
+			'family name with special characters' ),
+
+		// If there's a syntax error in the style - just leave it like that.
+		'test Normalize syntax error': assertNormalizeCssText( 'font-family:"crazy font",;', 'font-family:"crazy font",;',
+			'style syntax error' ),
+
+
 		testConvertRgbToHex: function() {
 			var c = CKEDITOR.tools.convertRgbToHex;
 

tests/plugins/font/font.js

@@ -12,8 +12,8 @@
 	var htmlMatchingOpts = {
 			fixStyles: true
 		},
-		ffArial = 'font-family:arial,helvetica,sans-serif',
-		ffCS = 'font-family:comic sans ms,cursive';
+		ffArial = 'font-family:Arial,Helvetica,sans-serif',
+		ffCS = 'font-family:Comic Sans MS,cursive';
 
 	bender.test( {
 		'test apply font size (collapsed selection)': function() {

tests/plugins/font/manual/quotedfont.html

@@ -0,0 +1,13 @@
+<div id="editor">
+	<p>A paragraph with default font.</p>
+	<p><span style="font-family: 'Univers LT Std', sans-serif;">This is styled part.</span></p>
+</div>
+<script>
+	CKEDITOR.replace( 'editor', {
+		height: 400,
+		font_names: 'Arial/Arial, Helvetica, sans-serif;' +
+		'Univers LT Std/\'Univers LT Std\', sans-serif;' +
+		'Times New Roman/Times New Roman, Times, serif;' +
+		'Verdana'
+	} );
+</script>

tests/plugins/font/manual/quotedfont.md

@@ -0,0 +1,11 @@
+@bender-tags: 4.5.10, tc, 10750
+@bender-ui: collapsed
+@bender-ckeditor-plugins: wysiwygarea, toolbar, font, enterkey, elementspath, sourcearea
+
+1. Click on the second paragraph.
+	* The font name in the combo box should be set.
+
+1. Switch to source mode.
+	* Somewhere in the source should be a `font-family` style with the value `font-family: 'Univers LT Std', sans-serif;`
+
+This test should pass regardless of whether `Univers LT Std` is available.