Merge pull request #62 from ckeditor/i/6550

Internal: The FontSize plugin requires numerical values in the configuration (as options) if fontSize.supportAllValues was set on true. Closes #6550.

packages/ckeditor5-font/docs/features/font.md

@@ -173,13 +173,12 @@ ClassicEditor
 
 By default, all `font-size` values that are not specified in the `config.fontSize.options` are stripped. You can enable support for all font sizes by using the {@link module:font/fontfamily~FontFamilyConfig#supportAllValues `config.fontSize.supportAllValues`} option.
 
-
 ```js
 ClassicEditor
 	.create( document.querySelector( '#editor' ), {
 		fontSize: {
 			options: [
-				// ...
+				// Numerical values.
 			],
 			supportAllValues: true
 		},
@@ -189,6 +188,10 @@ ClassicEditor
 	.catch( ... );
 ```
 
+<info-box info>
+	This option can be used only in combination with [numerical values](#using-numerical-values).
+</info-box>
+
 ## Configuring the font color and font background color features
 
 Both font color and font background color features are configurable and share the same configuration format.

packages/ckeditor5-font/src/fontsize.js

@@ -139,9 +139,12 @@ export default class FontSize extends Plugin {
  * You can preserve pasted font size values by switching the option:
  *
  *		const fontSizeConfig = {
+ *			options: [ 9, 10, 11, 12, 'default', 14, 15 ],
  *			supportAllValues: true
  *		};
  *
+ * **Note:** This option can only be used with numerical values as font size options.
+ *
  * Now, the font sizes, not specified in the editor's configuration, won't be removed when pasting the content.
  *
  * @member {Boolean} module:font/fontsize~FontSizeConfig#supportAllValues

packages/ckeditor5-font/src/fontsize/fontsizeediting.js

@@ -12,6 +12,7 @@ import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
 import FontSizeCommand from './fontsizecommand';
 import { normalizeOptions } from './utils';
 import { buildDefinition, FONT_SIZE } from '../utils';
+import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
 
 /**
  * The font size editing feature.
@@ -69,13 +70,13 @@ export default class FontSizeEditing extends Plugin {
 		const supportAllValues = editor.config.get( 'fontSize.supportAllValues' );
 
 		// Define view to model conversion.
-		const options = normalizeOptions( this.editor.config.get( 'fontSize.options' ), { supportAllValues } )
+		const options = normalizeOptions( this.editor.config.get( 'fontSize.options' ) )
 			.filter( item => item.model );
 		const definition = buildDefinition( FONT_SIZE, options );
 
 		// Set-up the two-way conversion.
 		if ( supportAllValues ) {
-			this._prepareAnyValueConverters();
+			this._prepareAnyValueConverters( definition );
 		} else {
 			editor.conversion.attributeToElement( definition );
 		}
@@ -88,11 +89,32 @@ export default class FontSizeEditing extends Plugin {
 	 * Those converters enable keeping any value found as `style="font-size: *"` as a value of an attribute on a text even
 	 * if it isn't defined in the plugin configuration.
 	 *
+	 * @param {Object} definition {@link module:engine/conversion/conversion~ConverterDefinition Converter definition} out of input data.
 	 * @private
 	 */
-	_prepareAnyValueConverters() {
+	_prepareAnyValueConverters( definition ) {
 		const editor = this.editor;
 
+		// If `fontSize.supportAllValues=true`, we do not allow to use named presets in the plugin's configuration.
+		const presets = definition.model.values.filter( value => !String( value ).match( /[\d.]+[\w%]+/ ) );
+
+		if ( presets.length ) {
+			/**
+			 * If {@link module:font/fontsize~FontSizeConfig#supportAllValues `config.fontSize.supportAllValues`} is `true`,
+			 * you need to use numerical values as font size options.
+			 *
+			 * See valid examples described in the {@link module:font/fontsize~FontSizeConfig#options plugin configuration}.
+			 *
+			 * @error font-size-invalid-use-of-named-presets
+			 * @param {Array.<String>} presets Invalid values.
+			 */
+			throw new CKEditorError(
+				'font-size-invalid-use-of-named-presets: ' +
+				'If config.fontSize.supportAllValues is set to true, you need to use numerical values as font size options.',
+				null, { presets }
+			);
+		}
+
 		editor.conversion.for( 'downcast' ).attributeToElement( {
 			model: FONT_SIZE,
 			view: ( attributeValue, writer ) => {

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

@@ -14,28 +14,16 @@ import CKEditorError from '@ckeditor/ckeditor5-utils/src/ckeditorerror';
  * to the {@link module:font/fontsize~FontSizeOption} format.
  *
  * @param {Array.<String|Number|Object>} configuredOptions An array of options taken from the configuration.
- * @param {Object} [options={}]
- * @param {Boolean} [options.supportAllValues=false]
  * @returns {Array.<module:font/fontsize~FontSizeOption>}
  */
-export function normalizeOptions( configuredOptions, options = {} ) {
-	const supportAllValues = options.supportAllValues || false;
-
+export function normalizeOptions( configuredOptions ) {
 	// Convert options to objects.
 	return configuredOptions
-		.map( item => getOptionDefinition( item, supportAllValues ) )
+		.map( item => getOptionDefinition( item ) )
 		// Filter out undefined values that `getOptionDefinition` might return.
 		.filter( option => !!option );
 }
 
-// The values should be synchronized with values specified in the "/theme/fontsize.css" file.
-export const FONT_SIZE_PRESET_UNITS = {
-	tiny: '0.7em',
-	small: '0.85em',
-	big: '1.4em',
-	huge: '1.8em'
-};
-
 // Default named presets map. Always create a new instance of the preset object in order to avoid modifying references.
 const namedPresets = {
 	get tiny() {
@@ -86,12 +74,10 @@ const namedPresets = {
 
 // Returns an option definition either from preset or creates one from number shortcut.
 // If object is passed then this method will return it without alternating it. Returns undefined for item than cannot be parsed.
-// If supportAllValues=true, model will be set to a specified unit value instead of text.
 //
 // @param {String|Number|Object} item
-// @param {Boolean} supportAllValues
 // @returns {undefined|module:font/fontsize~FontSizeOption}
-function getOptionDefinition( option, supportAllValues ) {
+function getOptionDefinition( option ) {
 	// Check whether passed option is a full item definition provided by user in configuration.
 	if ( isFullItemDefinition( option ) ) {
 		return attachPriority( option );
@@ -101,10 +87,6 @@ function getOptionDefinition( option, supportAllValues ) {
 
 	// Item is a named preset.
 	if ( preset ) {
-		if ( supportAllValues ) {
-			preset.model = FONT_SIZE_PRESET_UNITS[ option ];
-		}
-
 		return attachPriority( preset );
 	}
 

packages/ckeditor5-font/tests/fontsize/fontsizeediting.js

@@ -9,6 +9,7 @@ import Paragraph from '@ckeditor/ckeditor5-paragraph/src/paragraph';
 
 import VirtualTestEditor from '@ckeditor/ckeditor5-core/tests/_utils/virtualtesteditor';
 import { getData as getModelData, setData as setModelData } from '@ckeditor/ckeditor5-engine/src/dev-utils/model';
+import { assertCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
 describe( 'FontSizeEditing', () => {
 	let editor, doc;
@@ -68,7 +69,17 @@ describe( 'FontSizeEditing', () => {
 					.create( {
 						plugins: [ FontSizeEditing, Paragraph ],
 						fontSize: {
-							supportAllValues: true
+							supportAllValues: true,
+							options: [
+								'6.25%',
+								'8em',
+								'10px',
+								12,
+								{
+									model: 14,
+									title: '14px'
+								}
+							]
 						}
 					} )
 					.then( newEditor => {
@@ -101,6 +112,26 @@ describe( 'FontSizeEditing', () => {
 					expect( editor.getData() ).to.equal( '<p>f<span style="font-size:18px;">o</span>o</p>' );
 				} );
 			} );
+
+			it( 'should throw an error if used with default configuration of the plugin', () => {
+				return VirtualTestEditor
+					.create( {
+						plugins: [ FontSizeEditing ],
+						fontSize: {
+							supportAllValues: true
+						}
+					} )
+					.then(
+						() => {
+							throw new Error( 'Supposed to be rejected' );
+						},
+						error => {
+							assertCKEditorError( error, /font-size-invalid-use-of-named-presets/, null, {
+								presets: [ 'tiny', 'small', 'big', 'huge' ]
+							} );
+						}
+					);
+			} );
 		} );
 	} );
 

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

@@ -3,7 +3,7 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-import { normalizeOptions, FONT_SIZE_PRESET_UNITS } from '../../src/fontsize/utils';
+import { normalizeOptions } from '../../src/fontsize/utils';
 import { expectToThrowCKEditorError } from '@ckeditor/ckeditor5-utils/tests/_utils/utils';
 
 describe( 'FontSizeEditing Utils', () => {
@@ -37,18 +37,6 @@ describe( 'FontSizeEditing Utils', () => {
 				] );
 			} );
 
-			it( 'should return defined presets with units in model values if supportAllValues=true', () => {
-				const options = normalizeOptions( [ 'tiny', 'small', 'default', 'big', 'huge' ], { supportAllValues: true } );
-
-				expect( options ).to.deep.equal( [
-					{ title: 'Tiny', model: '0.7em', view: { name: 'span', classes: 'text-tiny', priority: 7 } },
-					{ title: 'Small', model: '0.85em', view: { name: 'span', classes: 'text-small', priority: 7 } },
-					{ title: 'Default', model: undefined },
-					{ title: 'Big', model: '1.4em', view: { name: 'span', classes: 'text-big', priority: 7 } },
-					{ title: 'Huge', model: '1.8em', view: { name: 'span', classes: 'text-huge', priority: 7 } }
-				] );
-			} );
-
 			it( 'should add "view" definition if missing', () => {
 				const tinyOption = {
 					title: 'Tiny',
@@ -165,10 +153,4 @@ describe( 'FontSizeEditing Utils', () => {
 			} );
 		} );
 	} );
-
-	describe( 'FONT_SIZE_PRESET_UNITS', () => {
-		it( 'provides default values', () => {
-			expect( Object.keys( FONT_SIZE_PRESET_UNITS ).length ).to.equal( 4 );
-		} );
-	} );
 } );

packages/ckeditor5-font/tests/integration.js

@@ -63,6 +63,7 @@ describe( 'Integration test Font', () => {
 						supportAllValues: true
 					},
 					fontSize: {
+						options: [ 10, 12, 14 ],
 						supportAllValues: true
 					}
 				} )
@@ -125,6 +126,7 @@ describe( 'Integration test Font', () => {
 						supportAllValues: true
 					},
 					fontSize: {
+						options: [ 10, 12, 14 ],
 						supportAllValues: true
 					}
 				} )

packages/ckeditor5-font/theme/fontsize.css

@@ -3,7 +3,6 @@
  * For licensing, see LICENSE.md or https://ckeditor.com/legal/ckeditor-oss-license
  */
 
-/* The values should be synchronized with the "FONT_SIZE_PRESET_UNITS" object in the "/src/fontsize/utils.js" file. */
 .text-tiny {
 	font-size: .7em;
 }