Add quotes to font family names (#476)

* Add quotes to families ending in number on gfonts

* Add quotes to uploaded font family name

* Check for special characters on form upload

* Trim font names

* Avoid re-rendering due to function definitions

Moves the definition of onFormDataChange inside the effect where it's
used so that it won't be re defined on render.

Also, wraps isFormValid on useCallback for the same reason.

* Add quotes to all font name strings

#476 (comment)

* Add tests

Add dependencies required for test and linting as well:

- @wordpress/scripts
- @wordpress/eslint-plugin

* Temporarily disable eslint rule

Out of scope for this PR; follow-up to come.

* Add quotes to demo style font name

Author: vcanales

package-lock.json

@@ -30,13 +30,14 @@
 		"@wordpress/browserslist-config": "^5.16.0",
 		"@wordpress/element": "^5.10.0",
 		"@wordpress/env": "^8.3.0",
-		"@wordpress/prettier-config": "^2.16.0",
-		"@wordpress/scripts": "^26.4.0",
+		"@wordpress/eslint-plugin": "^17.2.0",
+		"@wordpress/prettier-config": "^2.25.13",
+		"@wordpress/scripts": "^26.16.0",
 		"@wordpress/stylelint-config": "^21.16.0",
 		"babel-plugin-inline-json-import": "^0.3.2",
 		"husky": "^8.0.3",
 		"lint-staged": "^13.2.2",
-		"prettier": "npm:wp-prettier@2.6.2",
+		"prettier": "npm:wp-prettier@3.0.3",
 		"simple-git": "^3.18.0"
 	},
 	"scripts": {
@@ -52,11 +53,12 @@
 		"start": "wp-scripts start src/index.js src/plugin-sidebar.js src/wp-org-theme-directory.js",
 		"update-version": "node update-version-and-changelog.js",
 		"prepare": "husky install",
-		"wp-env": "wp-env"
+		"wp-env": "wp-env",
+		"test:unit": "wp-scripts test-unit-js --config test/unit/jest.config.js"
 	},
 	"lint-staged": {
 		"*.{js,json,yml}": [
-			"npx wp-scripts format"
+			"wp-scripts format"
 		],
 		"*.js": [
 			"npm run lint:js"
@@ -68,7 +70,7 @@
 			"npm run lint:php"
 		],
 		"package.json": [
-			"npx wp-scripts lint-pkg-json"
+			"wp-scripts lint-pkg-json"
 		]
 	}
 }

package.json

@@ -1,6 +1,6 @@
 import { useEffect } from '@wordpress/element';
 import Demo from '../demo-text-input/demo';
-import { localizeFontStyle } from '../utils';
+import { addQuotesToName, localizeFontStyle } from '../utils';
 
 function FontVariant( { font, variant, isSelected, handleToggle } ) {
 	const style = variant.includes( 'italic' ) ? 'italic' : 'normal';
@@ -17,21 +17,30 @@ function FontVariant( { font, variant, isSelected, handleToggle } ) {
 	};
 
 	useEffect( () => {
-		const newFont = new FontFace( font.family, `url( ${ variantUrl } )`, {
-			style,
-			weight,
-		} );
-		newFont
-			.load()
-			.then( function ( loadedFace ) {
+		const sanitizedFontFamily = addQuotesToName( font.family );
+
+		const newFont = new FontFace(
+			sanitizedFontFamily,
+			`url( ${ variantUrl } )`,
+			{
+				style,
+				weight,
+			}
+		);
+
+		const loadNewFont = async () => {
+			try {
+				const loadedFace = await newFont.load();
 				document.fonts.add( loadedFace );
-			} )
-			.catch( function ( error ) {
+			} catch ( error ) {
 				// TODO: show error in the UI
 				// eslint-disable-next-line
 				console.error( error );
-			} );
-	}, [ font, variant ] );
+			}
+		};
+
+		loadNewFont();
+	}, [ font, style, variant, variantUrl, weight ] );
 
 	const formattedFontFamily = font.family.toLowerCase().replace( ' ', '-' );
 	const fontId = `${ formattedFontFamily }-${ variant }`;
@@ -55,8 +64,10 @@ function FontVariant( { font, variant, isSelected, handleToggle } ) {
 				<label htmlFor={ fontId }>{ localizeFontStyle( style ) }</label>
 			</td>
 			<td className="demo-cell">
+				{ /* @TODO: associate label with control for accessibility */ }
+				{ /* eslint-disable-next-line jsx-a11y/label-has-associated-control */ }
 				<label htmlFor={ fontId }>
-					<Demo style={ previewStyles } />
+					<Demo id={ fontId } style={ previewStyles } />
 				</label>
 			</td>
 		</tr>

src/google-fonts/font-variant.js

@@ -1,11 +1,12 @@
 import { __ } from '@wordpress/i18n';
-import { useEffect, useState } from '@wordpress/element';
+import { useCallback, useEffect, useState } from '@wordpress/element';
 import UploadFontForm from './upload-font-form';
 import './local-fonts.css';
 import DemoTextInput from '../demo-text-input';
 import Demo from '../demo-text-input/demo';
 import { variableAxesToCss } from '../demo-text-input/utils';
 import BackButton from '../manage-fonts/back-button';
+import { addQuotesToName } from '../utils';
 
 const INITIAL_FORM_DATA = {
 	file: null,
@@ -33,18 +34,26 @@ function LocalFonts() {
 		setAxes( newAxes );
 	};
 
-	const isFormValid = () => {
-		return (
-			formData.file && formData.name && formData.weight && formData.style
-		);
-	};
+	const isFormValid = useCallback( () => {
+		// Check if font name is present and is alphanumeric.
+		const alphanumericRegex = /^[a-z0-9 ]+$/i;
+
+		if (
+			! formData.name ||
+			( formData.name && ! alphanumericRegex.test( formData.name ) )
+		) {
+			return false;
+		}
+
+		return formData.file && formData.weight && formData.style;
+	}, [ formData ] );
 
 	const demoStyle = () => {
 		if ( ! isFormValid() ) {
 			return {};
 		}
 		const style = {
-			fontFamily: formData.name,
+			fontFamily: addQuotesToName( formData.name ),
 			fontWeight: formData.weight,
 			fontStyle: formData.style,
 		};
@@ -54,32 +63,32 @@ function LocalFonts() {
 		return style;
 	};
 
-	// load the local font in the browser to make the preview work
-	const onFormDataChange = async () => {
-		if ( ! isFormValid() ) {
-			return;
-		}
+	useEffect( () => {
+		// load the local font in the browser to make the preview work
+		const onFormDataChange = async () => {
+			if ( ! isFormValid() ) {
+				return;
+			}
 
-		const data = await formData.file.arrayBuffer();
-		const newFont = new FontFace( formData.name, data, {
-			style: formData.style,
-			weight: formData.weight,
-		} );
-		newFont
-			.load()
-			.then( function ( loadedFace ) {
+			const data = await formData.file.arrayBuffer();
+			const sanitizedFontFamily = addQuotesToName( formData.name );
+			const newFont = new FontFace( sanitizedFontFamily, data, {
+				style: formData.style,
+				weight: formData.weight,
+			} );
+
+			try {
+				const loadedFace = await newFont.load();
 				document.fonts.add( loadedFace );
-			} )
-			.catch( function ( error ) {
+			} catch ( error ) {
 				// TODO: show error in the UI
 				// eslint-disable-next-line
 				console.error( error );
-			} );
-	};
+			}
+		};
 
-	useEffect( () => {
 		onFormDataChange();
-	}, [ formData ] );
+	}, [ formData, isFormValid ] );
 
 	return (
 		<div className="layout">

src/local-fonts/index.js

@@ -0,0 +1,30 @@
+import { addQuotesToName } from '../utils';
+
+describe( 'addQuotesToName', () => {
+	const easyFonts = [ 'Roboto' ];
+	const complicatedFonts = [
+		'Roboto Mono',
+		'Open Sans Condensed',
+		'Exo 2',
+		'Libre Barcode 128 Text',
+		'Press Start 2P',
+		'Rock 3D',
+		'Rubik 80s Fade',
+	];
+
+	it( 'should add quotes to all font names', () => {
+		[ ...easyFonts, ...complicatedFonts ].forEach( ( font ) => {
+			expect( addQuotesToName( font ) ).toEqual( `'${ font }'` );
+		} );
+	} );
+
+	it( 'should avoid FontFace objects with empty font name', () => {
+		complicatedFonts.forEach( ( font ) => {
+			const quoted = addQuotesToName( font );
+			const fontObject = new FontFace( quoted, {} );
+
+			expect( fontObject ).toBeInstanceOf( FontFace );
+			expect( fontObject.family ).toEqual( quoted );
+		} );
+	} );
+} );

src/test/unit.js

@@ -76,3 +76,13 @@ export async function downloadFile( response ) {
 		}, 100 );
 	}
 }
+
+/*
+ * Add quotes to font name.
+ * @param {string} familyName The font family name.
+ * @return {string} The font family name with quotes.
+ */
+
+export function addQuotesToName( familyName ) {
+	return `'${ familyName }'`.trim();
+}

src/utils.js

@@ -0,0 +1,10 @@
+module.exports = {
+	testEnvironment: 'jsdom',
+	rootDir: '../../',
+	testMatch: [ '<rootDir>/src/test/**/*.js' ],
+	moduleFileExtensions: [ 'js' ],
+	moduleNameMapper: {
+		'^@/(.*)$': '<rootDir>/src/$1',
+	},
+	setupFiles: [ '<rootDir>/test/unit/setup.js' ],
+};

test/unit/jest.config.js

@@ -0,0 +1,7 @@
+// Stub out the FontFace class for tests.
+global.FontFace = class {
+	constructor( family, source ) {
+		this.family = family;
+		this.source = source;
+	}
+};