Merge pull request #7367 from ckeditor/i/7285

Fix (font): The Font Family feature should apply the complete family value from the configuration when `config.fontFamily.supportAllValues` is `true`. Closes #7285.

packages/ckeditor5-build-decoupled-document/tests/ckeditor.js

@@ -200,7 +200,7 @@ describe( 'DecoupledEditor build', () => {
 
 				editor.setData( data );
 				expect( editor.getData() ).to.equal( data );
-				expect( editor.model.document.selection.getAttribute( 'fontFamily' ) ).to.equal( 'Georgia' );
+				expect( editor.model.document.selection.getAttribute( 'fontFamily' ) ).to.equal( 'Georgia, serif' );
 			} );
 
 			it( 'font background color works', () => {

packages/ckeditor5-font/src/fontfamily.js

@@ -120,7 +120,7 @@ export default class FontFamily extends Plugin {
  *
  * You can preserve pasted font family values by switching the `supportAllValues` option to `true`:
  *
- *		const fontSizeConfig = {
+ *		const fontFamilyConfig = {
  *			supportAllValues: true
  *		};
  *

packages/ckeditor5-font/src/fontfamily/utils.js

@@ -66,7 +66,7 @@ function generateFontPreset( fontDefinition ) {
 
 	return {
 		title: firstFontName,
-		model: firstFontName,
+		model: cssFontNames,
 		view: {
 			name: 'span',
 			styles: {

packages/ckeditor5-font/tests/fontfamily/fontfamilyediting.js

@@ -104,9 +104,9 @@ describe( 'FontFamilyEditing', () => {
 				} );
 
 				it( 'should convert defined fontFamily attribute values', () => {
-					setModelData( doc, '<paragraph>f<$text fontFamily="Arial">o</$text>o</paragraph>' );
+					setModelData( doc, '<paragraph>f<$text fontFamily="Arial, Helvetica, sans-serif">o</$text>o</paragraph>' );
 
-					expect( editor.getData() ).to.equal( '<p>f<span style="font-family:Arial;">o</span>o</p>' );
+					expect( editor.getData() ).to.equal( '<p>f<span style="font-family:Arial, Helvetica, sans-serif;">o</span>o</p>' );
 				} );
 			} );
 
@@ -174,10 +174,12 @@ describe( 'FontFamilyEditing', () => {
 		} );
 
 		it( 'should convert fontFamily attribute to configured complex preset', () => {
-			setModelData( doc, '<paragraph>f<$text fontFamily="Lucida Sans Unicode">o</$text>o</paragraph>' );
+			const fontFamily = '\'Lucida Sans Unicode\', \'Lucida Grande\', sans-serif';
+
+			setModelData( doc, `<paragraph>f<$text fontFamily="${ fontFamily }">o</$text>o</paragraph>` );
 
 			expect( editor.getData() )
-				.to.equal( '<p>f<span style="font-family:\'Lucida Sans Unicode\', \'Lucida Grande\', sans-serif;">o</span>o</p>' );
+				.to.equal( `<p>f<span style="font-family:${ fontFamily };">o</span>o</p>` );
 		} );
 
 		it( 'should convert fontFamily attribute from user defined settings', () => {

packages/ckeditor5-font/tests/fontfamily/fontfamilyui.js

@@ -10,9 +10,11 @@ import FontFamilyUI from '../../src/fontfamily/fontfamilyui';
 
 import fontFamilyIcon from '../../theme/icons/font-family.svg';
 
+import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 import ClassicTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/classictesteditor';
 import testUtils from '@ckeditor/ckeditor5-core/tests/_utils/utils';
 import { add as addTranslations, _clear as clearTranslations } from '@ckeditor/ckeditor5-utils/src/translation-service';
+import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
 
 describe( 'FontFamilyUI', () => {
 	let editor, command, element;
@@ -97,47 +99,67 @@ describe( 'FontFamilyUI', () => {
 			expect( listView.items.map( item => item.children.first.isOn ) )
 				.to.deep.equal( [ true, false, false, false, false, false, false, false, false ] );
 
-			command.value = 'Arial';
+			command.value = 'Arial, Helvetica, sans-serif';
 
 			// The second item is 'Arial' font family.
 			expect( listView.items.map( item => item.children.first.isOn ) )
 				.to.deep.equal( [ false, true, false, false, false, false, false, false, false ] );
 		} );
 
-		it( 'should activate current option in dropdown for full font family definitions', () => {
-			const element = document.createElement( 'div' );
-			document.body.appendChild( element );
+		describe( 'with supportAllValues=true', () => {
+			let editor, element, command, dropdown;
 
-			return ClassicTestEditor
-				.create( element, {
-					plugins: [ FontFamilyEditing, FontFamilyUI ],
-					fontSize: {
-						supportAllValues: true
-					}
-				} )
-				.then( editor => {
-					const command = editor.commands.get( 'fontFamily' );
-					const dropdown = editor.ui.componentFactory.create( 'fontFamily' );
+			beforeEach( async () => {
+				element = document.createElement( 'div' );
+				document.body.appendChild( element );
+
+				editor = await ClassicTestEditor
+					.create( element, {
+						plugins: [ Paragraph, FontFamilyEditing, FontFamilyUI ],
+						fontSize: {
+							supportAllValues: true
+						}
+					} );
+
+				command = editor.commands.get( 'fontFamily' );
+				dropdown = editor.ui.componentFactory.create( 'fontFamily' );
+			} );
+
+			afterEach( async () => {
+				await editor.destroy();
+				element.remove();
+			} );
 
-					const listView = dropdown.listView;
+			it( 'should activate the current option in the dropdown for full font family definitions', () => {
+				const listView = dropdown.listView;
+
+				command.value = undefined;
+
+				// The first item is 'default' font family.
+				expect( listView.items.map( item => item.children.first.isOn ) )
+					.to.deep.equal( [ true, false, false, false, false, false, false, false, false ] );
+
+				command.value = '\'Courier New\', Courier, monospace';
 
-					command.value = undefined;
+				// The third item is 'Courier New' font family.
+				expect( listView.items.map( item => item.children.first.isOn ) )
+					.to.deep.equal( [ false, false, true, false, false, false, false, false, false ] );
+			} );
+
+			it( 'should apply the complete font-family value (list of font-families)', () => {
+				const listView = dropdown.listView;
+				const fontFamilyArialButton = listView.items.get( 1 ).children.first;
 
-					// The first item is 'default' font family.
-					expect( listView.items.map( item => item.children.first.isOn ) )
-						.to.deep.equal( [ true, false, false, false, false, false, false, false, false ] );
+				setModelData( editor.model, '<paragraph>f[oo]</paragraph>' );
 
-					command.value = '\'Courier New\', Courier, monospace';
+				fontFamilyArialButton.fire( 'execute' );
 
-					// The third item is 'Courier New' font family.
-					expect( listView.items.map( item => item.children.first.isOn ) )
-						.to.deep.equal( [ false, false, true, false, false, false, false, false, false ] );
+				expect( getModelData( editor.model ) ).to.equal(
+					'<paragraph>f[<$text fontFamily="Arial, Helvetica, sans-serif">oo</$text>]</paragraph>'
+				);
 
-					return editor.destroy();
-				} )
-				.then( () => {
-					element.remove();
-				} );
+				expect( editor.getData() ).to.equal( '<p>f<span style="font-family:Arial, Helvetica, sans-serif;">oo</span></p>' );
+			} );
 		} );
 
 		describe( 'model to command binding', () => {

packages/ckeditor5-font/tests/fontfamily/utils.js

@@ -62,7 +62,7 @@ describe( 'FontFamily utils', () => {
 					},
 					{
 						title: 'Comic Sans MS',
-						model: 'Comic Sans MS',
+						model: '\'Comic Sans MS\', sans-serif',
 						view: {
 							name: 'span',
 							styles: {
@@ -73,7 +73,7 @@ describe( 'FontFamily utils', () => {
 					},
 					{
 						title: 'Lucida Console',
-						model: 'Lucida Console',
+						model: '\'Lucida Console\', \'Courier New\', Courier, monospace',
 						view: {
 							name: 'span',
 							styles: {

packages/ckeditor5-font/tests/integration.js

@@ -37,7 +37,8 @@ describe( 'Integration test Font', () => {
 		it( 'should render one span element for all types of font features', () => {
 			setModelData( model,
 				'<paragraph>' +
-					'<$text fontColor="#123456" fontBackgroundColor="rgb(10,20,30)" fontSize="big" fontFamily="Arial">foo</$text>' +
+					'<$text fontColor="#123456" fontBackgroundColor="rgb(10,20,30)" fontSize="big" ' +
+						'fontFamily="Arial, Helvetica, sans-serif">foo</$text>' +
 				'</paragraph>'
 			);
 

packages/ckeditor5-font/tests/manual/font-family.md

@@ -14,7 +14,8 @@ Try to:
 ### Converters mode
 
 The "Restricted value matching" option means that all font-family values that aren't defined in the plugin's configuration will be removed (e.g. when pasted from Google Docs).
-This behaviour can be disabled by selecting the "Disabled value matching" option.
+
+This behaviour can be disabled by selecting the "Disabled value matching" option, which sets ["`supportAllValues: true`"](https://ckeditor.com/docs/ckeditor5/latest/api/module_font_fontfamily-FontFamilyConfig.html#member-supportAllValues) in the font family configuration.
 
 The `Docs-Roboto, Arial` font-family is not specified in the plugin's configuration and should be restored to default font when the "Restricted value matching" option is selected.