Font Library: fix fonts not displaying correctly (#55393)

* Add kebab casing and font family wrapping in sanitization function. Remove commented code. * Simplify how kebab casing and quote wrapping is done. * Ensure incoming font is returned. * Add kebab casing to slug in form data. * Add tests for wpKebabCase * remove redundant test * test 2 different inputs on first wpKebabCase case * Move formatting functions to utils. * reduce the scope of the function * fix the live preview (without reloading the page) of the just installed fonts in the editor * format php * Add test for format_font_family. * Format php. * Add more test cases and format. * Remove accidental badKey. * Format. --------- Co-authored-by: Vicente Canales <[email protected]> Co-authored-by: Sarah Norris <[email protected]> Co-authored-by: Sarah Norris <[email protected]> Co-authored-by: Matias Benedetto <[email protected]>
WordPress · Nov 27, 2023 · 500c9e4 · 500c9e4
1 parent 6f62e38
commit 500c9e4
Show file tree

Hide file tree

Showing 7 changed files with 191 additions and 26 deletions.
diff --git a/lib/experimental/fonts/font-library/class-wp-font-family-utils.php b/lib/experimental/fonts/font-library/class-wp-font-family-utils.php
@@ -90,4 +90,36 @@ public static function has_font_mime_type( $filepath ) {
 
 		return in_array( $filetype['type'], $allowed_mime_types, true );
 	}
+
+	/**
+	 * Format font family to make it valid CSS.
+	 *
+	 * @since 6.5.0
+	 *
+	 * @param string $font_family Font family attribute.
+	 * @return string The formatted font family attribute.
+	 */
+	public static function format_font_family( $font_family ) {
+		if ( $font_family ) {
+			$font_families         = explode( ',', $font_family );
+			$wrapped_font_families = array_map(
+				function ( $family ) {
+					$trimmed = trim( $family );
+					if ( ! empty( $trimmed ) && strpos( $trimmed, ' ' ) !== false && strpos( $trimmed, "'" ) === false && strpos( $trimmed, '"' ) === false ) {
+							return "'" . $trimmed . "'";
+					}
+					return $trimmed;
+				},
+				$font_families
+			);
+
+			if ( count( $wrapped_font_families ) === 1 ) {
+				$font_family = $wrapped_font_families[0];
+			} else {
+				$font_family = implode( ', ', $wrapped_font_families );
+			}
+		}
+
+		return $font_family;
+	}
 }
diff --git a/lib/experimental/fonts/font-library/class-wp-font-family.php b/lib/experimental/fonts/font-library/class-wp-font-family.php
@@ -308,6 +308,7 @@ private function sanitize() {
 				),
 			),
 		);
+
 		// Creates a new WP_Theme_JSON object with the new fonts to
 		// leverage sanitization and validation.
 		$fonts_json     = WP_Theme_JSON_Gutenberg::remove_insecure_properties( $fonts_json );
@@ -316,7 +317,10 @@ private function sanitize() {
 		$sanitized_font = ! empty( $theme_data['settings']['typography']['fontFamilies'] )
 			? $theme_data['settings']['typography']['fontFamilies'][0]
 			: array();
-		$this->data     = $sanitized_font;
+
+		$sanitized_font['slug']       = _wp_to_kebab_case( $sanitized_font['slug'] );
+		$sanitized_font['fontFamily'] = WP_Font_Family_Utils::format_font_family( $sanitized_font['fontFamily'] );
+		$this->data                   = $sanitized_font;
 		return $this->data;
 	}
 

diff --git a/...site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js b/...site/src/components/global-styles/font-library-modal/utils/get-intersecting-font-faces.js
@@ -47,7 +47,7 @@ export default function getIntersectingFontFaces( incoming, existing ) {
 						} );
 					}
 				);
-				matches.push( { ...existingFont, fontFace: matchingFaces } );
+				matches.push( { ...incomingFont, fontFace: matchingFaces } );
 			} else {
 				matches.push( incomingFont );
 			}

diff --git a/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js b/packages/edit-site/src/components/global-styles/font-library-modal/utils/index.js
@@ -1,8 +1,12 @@
+/**
+ * External dependencies
+ */
+import { paramCase as kebabCase } from 'change-case';
+
 /**
  * Internal dependencies
  */
 import { FONT_WEIGHTS, FONT_STYLES } from './constants';
-import { formatFontFamily } from './preview-styles';
 
 export function setUIValuesNeeded( font, extraValues = {} ) {
 	if ( ! font.name && ( font.fontFamily || font.slug ) ) {
@@ -85,14 +89,10 @@ export async function loadFontFaceInBrowser( fontFace, source, addTo = 'all' ) {
 	}
 
 	// eslint-disable-next-line no-undef
-	const newFont = new FontFace(
-		formatFontFamily( fontFace.fontFamily ),
-		dataSource,
-		{
-			style: fontFace.fontStyle,
-			weight: fontFace.fontWeight,
-		}
-	);
+	const newFont = new FontFace( fontFace.fontFamily, dataSource, {
+		style: fontFace.fontStyle,
+		weight: fontFace.fontWeight,
+	} );
 
 	const loadedFace = await newFont.load();
 
@@ -129,9 +129,20 @@ export function getDisplaySrcFromFontFace( input, urlPrefix ) {
 	return src;
 }
 
+// This function replicates one behavior of _wp_to_kebab_case().
+// Additional context: https://github.com/WordPress/gutenberg/issues/53695
+export function wpKebabCase( str ) {
+	// If a string contains a digit followed by a number, insert a dash between them.
+	return kebabCase( str ).replace(
+		/([a-zA-Z])(\d)|(\d)([a-zA-Z])/g,
+		'$1$3-$2$4'
+	);
+}
+
 export function makeFormDataFromFontFamilies( fontFamilies ) {
 	const formData = new FormData();
 	const newFontFamilies = fontFamilies.map( ( family, familyIndex ) => {
+		family.slug = wpKebabCase( family.slug );
 		if ( family?.fontFace ) {
 			family.fontFace = family.fontFace.map( ( face, faceIndex ) => {
 				if ( face.file ) {

diff --git a/...c/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js b/...c/components/global-styles/font-library-modal/utils/test/getIntersectingFontFaces.spec.js
@@ -5,7 +5,7 @@ import getIntersectingFontFaces from '../get-intersecting-font-faces';
 
 describe( 'getIntersectingFontFaces', () => {
 	it( 'returns matching font faces for matching font family', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'lobster',
 				fontFace: [
@@ -30,15 +30,15 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
-		expect( result ).toEqual( intendedFontsFamilies );
+		expect( result ).toEqual( incomingFontFamilies );
 	} );
 
 	it( 'returns empty array when there is no match', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'lobster',
 				fontFace: [
@@ -63,15 +63,15 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
 		expect( result ).toEqual( [] );
 	} );
 
 	it( 'returns matching font faces', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'lobster',
 				fontFace: [
@@ -129,15 +129,15 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
 		expect( result ).toEqual( expectedOutput );
 	} );
 
 	it( 'returns empty array when the first list is empty', () => {
-		const intendedFontsFamilies = [];
+		const incomingFontFamilies = [];
 
 		const existingFontFamilies = [
 			{
@@ -152,15 +152,15 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
 		expect( result ).toEqual( [] );
 	} );
 
 	it( 'returns empty array when the second list is empty', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'lobster',
 				fontFace: [
@@ -175,15 +175,15 @@ describe( 'getIntersectingFontFaces', () => {
 		const existingFontFamilies = [];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
 		expect( result ).toEqual( [] );
 	} );
 
 	it( 'returns intersecting font family when there are no fonfaces', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'piazzolla',
 				fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
@@ -200,15 +200,15 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 
 		expect( result ).toEqual( existingFontFamilies );
 	} );
 
 	it( 'returns intersecting if there is an intended font face and is not present in the returning it should not be returned', () => {
-		const intendedFontsFamilies = [
+		const incomingFontFamilies = [
 			{
 				slug: 'piazzolla',
 				fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
@@ -226,7 +226,7 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 
 		const result = getIntersectingFontFaces(
-			intendedFontsFamilies,
+			incomingFontFamilies,
 			existingFontFamilies
 		);
 		const expected = [
@@ -237,4 +237,35 @@ describe( 'getIntersectingFontFaces', () => {
 		];
 		expect( result ).toEqual( expected );
 	} );
+
+	it( 'updates font family definition using the incoming data', () => {
+		const incomingFontFamilies = [
+			{
+				slug: 'gothic-a1',
+				fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
+				fontFamily: "'Gothic A1', serif",
+			},
+		];
+
+		const existingFontFamilies = [
+			{
+				slug: 'gothic-a1',
+				fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
+				fontFamily: 'Gothic A1, serif',
+			},
+		];
+
+		const result = getIntersectingFontFaces(
+			incomingFontFamilies,
+			existingFontFamilies
+		);
+		const expected = [
+			{
+				slug: 'gothic-a1',
+				fontFace: [ { fontStyle: 'normal', fontWeight: '400' } ],
+				fontFamily: "'Gothic A1', serif",
+			},
+		];
+		expect( result ).toEqual( expected );
+	} );
 } );
diff --git a/.../edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js b/.../edit-site/src/components/global-styles/font-library-modal/utils/test/wpKebabCase.spec.js
@@ -0,0 +1,28 @@
+/**
+ * Internal dependencies
+ */
+import { wpKebabCase } from '../index';
+
+describe( 'wpKebabCase', () => {
+	it( 'should insert a dash between a letter and a digit', () => {
+		const input = 'abc1def';
+		const expectedOutput = 'abc-1def';
+		expect( wpKebabCase( input ) ).toEqual( expectedOutput );
+
+		const input2 = 'abc1def2ghi';
+		const expectedOutput2 = 'abc-1def-2ghi';
+		expect( wpKebabCase( input2 ) ).toEqual( expectedOutput2 );
+	} );
+
+	it( 'should not insert a dash between two letters', () => {
+		const input = 'abcdef';
+		const expectedOutput = 'abcdef';
+		expect( wpKebabCase( input ) ).toEqual( expectedOutput );
+	} );
+
+	it( 'should not insert a dash between a digit and a hyphen', () => {
+		const input = 'abc1-def';
+		const expectedOutput = 'abc-1-def';
+		expect( wpKebabCase( input ) ).toEqual( expectedOutput );
+	} );
+} );
diff --git a/phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php b/phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php
@@ -0,0 +1,59 @@
+<?php
+/**
+ * Test WP_Font_Family_Utils::format_font_family().
+ *
+ * @package WordPress
+ * @subpackage Font Library
+ *
+ * @group fonts
+ * @group font-library
+ *
+ * @covers WP_Font_Family_Utils::format_font_family
+ */
+class Tests_Fonts_WpFontsFamilyUtils_FormatFontFamily extends WP_UnitTestCase {
+
+	/**
+	 * @dataProvider data_should_format_font_family
+	 *
+	 * @param string $font_family   Font family.
+	 * @param string $expected      Expected family.
+	 */
+	public function test_should_format_font_family( $font_family, $expected ) {
+		$this->assertSame(
+			$expected,
+			WP_Font_Family_Utils::format_font_family(
+				$font_family
+			)
+		);
+	}
+
+	/**
+	 * Data provider.
+	 *
+	 * @return array[]
+	 */
+	public function data_should_format_font_family() {
+		return array(
+			'data_families_with_spaces_and_numbers' => array(
+				'font_family' => 'Rock 3D , Open Sans,serif',
+				'expected'    => "'Rock 3D', 'Open Sans', serif",
+			),
+			'data_single_font_family'               => array(
+				'font_family' => 'Rock 3D',
+				'expected'    => "'Rock 3D'",
+			),
+			'data_no_spaces'                        => array(
+				'font_family' => 'Rock3D',
+				'expected'    => 'Rock3D',
+			),
+			'data_many_spaces_and_existing_quotes'  => array(
+				'font_family' => 'Rock 3D serif, serif,sans-serif, "Open Sans"',
+				'expected'    => "'Rock 3D serif', serif, sans-serif, \"Open Sans\"",
+			),
+			'data_empty_family'                     => array(
+				'font_family' => ' ',
+				'expected'    => '',
+			),
+		);
+	}
+}