From 5689b536715274a06e03dd90d2602ae1123cfd8e Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Tue, 19 Sep 2023 11:55:04 -0500 Subject: [PATCH 1/7] Gets name from fontFamily as fallback check. If the "name" setting does not exist, then get it from the "fontFamily" setting. --- .../font-face/class-wp-font-face-resolver.php | 58 ++++++-- .../wpFontFaceResolver/getFontFamilyName.php | 140 ++++++++++++++++++ 2 files changed, 189 insertions(+), 9 deletions(-) create mode 100644 phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php diff --git a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php index 7f397663df961..393818d5e0478 100644 --- a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php +++ b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php @@ -56,30 +56,70 @@ private static function parse_settings( array $settings ) { foreach ( $settings['typography']['fontFamilies'] as $font_families ) { foreach ( $font_families as $definition ) { - // Skip if font-family "name" is not defined. - if ( empty( $definition['name'] ) ) { - continue; - } - // Skip if "fontFace" is not defined, meaning there are no variations. if ( empty( $definition['fontFace'] ) ) { continue; } - $font_family = $definition['name']; + $font_family_name = static::get_font_family_name( $definition ); + + // Skip if no font family is defined. + if ( empty( $font_family_name ) ) { + continue; + } // Prepare the fonts array structure for this font-family. - if ( ! array_key_exists( $font_family, $fonts ) ) { - $fonts[ $font_family ] = array(); + if ( ! array_key_exists( $font_family_name, $fonts ) ) { + $fonts[ $font_family_name ] = array(); } - $fonts[ $font_family ] = static::convert_font_face_properties( $definition['fontFace'], $font_family ); + $fonts[ $font_family_name ] = static::convert_font_face_properties( $definition['fontFace'], $font_family_name ); } } return $fonts; } + /** + * Get the font-family name. + * + * @since 6.4.0 + * + * @param array $definition The font-family definition. + * @return string Font family name. + */ + private static function get_font_family_name( $definition ) { + if ( ! empty( $definition['name'] ) ) { + return $definition['name']; + } + + // If the `'name'` setting was not defined, check the `fontFamily` setting. + if ( ! empty( $definition['fontFamily'] ) ) { + return static::maybe_parse_name_from_comma_separated_list( $definition['fontFamily'] ); + } + + return ''; + } + + /** + * Parse font-family name from comma-separated lists. + * + * If the given `fontFamily` is a comma-separated lists (example: "Inter, sans-serif" ), + * parse and return the fist font from the list. + * + * @since 6.4.0 + * + * @param string $font_family Font family `fontFamily' to parse. + * @return string Font-family name. + */ + private static function maybe_parse_name_from_comma_separated_list( $font_family ) { + if ( str_contains( $font_family, ',' ) ) { + $font_family = explode( ',', $font_family )[0]; + } + + return $font_family; + } + /** * Converts font-face properties from theme.json format. * diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php new file mode 100644 index 0000000000000..5054b291ac137 --- /dev/null +++ b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php @@ -0,0 +1,140 @@ +setAccessible( true ); + + parent::set_up_before_class(); + } + + public static function tear_down_after_class() { + self::$resolver->setAccessible( false ); + + parent::tear_down_after_class(); + } + + /** + * @dataProvider data_should_return_font_family_name + * + * @param array $definition The font-family definition to test. + * @param string $expected Expected font-family name. + */ + public function test_should_return_font_family_name( $definition, $expected ) { + $actual = static::$resolver->invokeArgs( null, array( $definition ) ); + + $this->assertSame( $actual, $expected ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_should_return_font_family_name() { + return array( + 'name and fontFamily declared' => array( + 'definition' => array( + 'fontFamily' => 'DM Sans', + 'name' => 'DM Sans', + 'slug' => 'dm-sans', + 'fontFace' => array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ), + 'expected' => 'DM Sans', + ), + 'only name declared' => array( + 'definition' => array( + 'name' => 'DM Sans', + 'fontFace' => array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ), + 'expected' => 'DM Sans', + ), + 'only fontFamily declared' => array( + 'definition' => array( + 'fontFamily' => 'DM Sans', + 'slug' => 'dm-sans', + 'fontFace' => array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ), + 'expected' => 'DM Sans', + ), + 'fontFamily comma-separated list' => array( + 'definition' => array( + 'fontFamily' => 'DM Sans, sans-serif', + 'slug' => 'dm-sans', + 'fontFace' => array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ), + 'expected' => 'DM Sans', + ), + ); + } + + public function test_should_return_empty_string_on_failure() { + $definition = array( + 'slug' => 'dm-sans', + 'fontFace' => array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ); + + $actual = static::$resolver->invokeArgs( null, array( $definition ) ); + $expected = ''; + $this->assertSame( $actual, $expected ); + } +} From 370caf01a5944d6229d0b8f73b92d5bac1d448a2 Mon Sep 17 00:00:00 2001 From: hellofromtonya Date: Tue, 19 Sep 2023 13:36:21 -0500 Subject: [PATCH 2/7] Move tests into existing test. To avoid reflection issues on different PHP versions and potential contributor confusion, merging the new tests into the existing test. --- .../wpFontFaceResolver/getFontFamilyName.php | 140 ------------------ .../getFontsFromThemeJson.php | 104 +++++++++++++ 2 files changed, 104 insertions(+), 140 deletions(-) delete mode 100644 phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php deleted file mode 100644 index 5054b291ac137..0000000000000 --- a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontFamilyName.php +++ /dev/null @@ -1,140 +0,0 @@ -setAccessible( true ); - - parent::set_up_before_class(); - } - - public static function tear_down_after_class() { - self::$resolver->setAccessible( false ); - - parent::tear_down_after_class(); - } - - /** - * @dataProvider data_should_return_font_family_name - * - * @param array $definition The font-family definition to test. - * @param string $expected Expected font-family name. - */ - public function test_should_return_font_family_name( $definition, $expected ) { - $actual = static::$resolver->invokeArgs( null, array( $definition ) ); - - $this->assertSame( $actual, $expected ); - } - - /** - * Data provider. - * - * @return array - */ - public function data_should_return_font_family_name() { - return array( - 'name and fontFamily declared' => array( - 'definition' => array( - 'fontFamily' => 'DM Sans', - 'name' => 'DM Sans', - 'slug' => 'dm-sans', - 'fontFace' => array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ), - 'expected' => 'DM Sans', - ), - 'only name declared' => array( - 'definition' => array( - 'name' => 'DM Sans', - 'fontFace' => array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ), - 'expected' => 'DM Sans', - ), - 'only fontFamily declared' => array( - 'definition' => array( - 'fontFamily' => 'DM Sans', - 'slug' => 'dm-sans', - 'fontFace' => array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ), - 'expected' => 'DM Sans', - ), - 'fontFamily comma-separated list' => array( - 'definition' => array( - 'fontFamily' => 'DM Sans, sans-serif', - 'slug' => 'dm-sans', - 'fontFace' => array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ), - 'expected' => 'DM Sans', - ), - ); - } - - public function test_should_return_empty_string_on_failure() { - $definition = array( - 'slug' => 'dm-sans', - 'fontFace' => array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ); - - $actual = static::$resolver->invokeArgs( null, array( $definition ) ); - $expected = ''; - $this->assertSame( $actual, $expected ); - } -} diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php index 481c451bbafa0..a3861e6102db5 100644 --- a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php +++ b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php @@ -101,4 +101,108 @@ public function data_should_replace_src_file_placeholder() { ), ); } + + /** + * @dataProvider data_should_get_font_family_name + * + * @param array $fonts Fonts to test. + * @param string $expected_name Expected font-family name. + */ + public function test_should_get_font_family_name( $fonts, $expected_name ) { + switch_theme( static::FONTS_THEME ); + + $replace_fonts = static function ( $theme_json_data ) use ( $fonts ) { + $data = $theme_json_data->get_data(); + + // Replace typography.fontFamilies. + $data['settings']['typography']['fontFamilies']['theme'] = $fonts; + + return new WP_Theme_JSON_Data_Gutenberg( $data ); + }; + add_filter( 'wp_theme_json_data_theme', $replace_fonts ); + $fonts = WP_Font_Face_Resolver::get_fonts_from_theme_json(); + remove_filter( 'wp_theme_json_data_theme', $replace_fonts ); + + $this->assertArrayHasKey( $expected_name, $fonts ); + } + + /** + * Data provider. + * + * @return array + */ + public function data_should_get_font_family_name() { + $font_face = array( + array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Regular.woff2', + ), + ), + array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '400', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Regular-Italic.woff2', + ), + ), + array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold.woff2', + ), + ), + array( + 'fontFamily' => 'DM Sans', + 'fontStretch' => 'normal', + 'fontStyle' => 'italic', + 'fontWeight' => '700', + 'src' => array( + 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', + ), + ), + ); + + return array( + 'name declared' => array( + 'fonts' => array( + array( + 'fontFamily' => 'DM Sans', + 'name' => 'DM Sans', + 'slug' => 'dm-sans', + 'fontFace' => $font_face, + ), + ), + 'expected_name' => 'DM Sans', + ), + 'name not declared' => array( + 'fonts' => array( + array( + 'fontFamily' => 'DM Sans', + 'slug' => 'dm-sans', + 'fontFace' => $font_face, + ), + ), + 'expected_name' => 'DM Sans', + ), + 'fontFamily comma-separated list' => array( + 'fonts' => array( + array( + 'fontFamily' => 'DM Sans, sans-serif', + 'slug' => 'dm-sans', + 'fontFace' => $font_face, + ), + ), + 'expected_name' => 'DM Sans', + ), + ); + } } From 1c1b96e4b9016ed2bfa67877c91760b26b6a3590 Mon Sep 17 00:00:00 2001 From: Tonya Mork Date: Tue, 19 Sep 2023 15:13:39 -0500 Subject: [PATCH 3/7] Do not use "name". Do not the "name" settting. Instead only use the required "fontFamily" setting. --- .../font-face/class-wp-font-face-resolver.php | 28 ++++--------------- 1 file changed, 6 insertions(+), 22 deletions(-) diff --git a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php index 393818d5e0478..071dce3152684 100644 --- a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php +++ b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php @@ -61,7 +61,12 @@ private static function parse_settings( array $settings ) { continue; } - $font_family_name = static::get_font_family_name( $definition ); + // Skip if "fontFamily" is not defined. + if ( empty( $definition['fontFamily'] ) ) { + continue; + } + + $font_family_name = static::maybe_parse_name_from_comma_separated_list( $definition['fontFamily'] ); // Skip if no font family is defined. if ( empty( $font_family_name ) ) { @@ -80,27 +85,6 @@ private static function parse_settings( array $settings ) { return $fonts; } - /** - * Get the font-family name. - * - * @since 6.4.0 - * - * @param array $definition The font-family definition. - * @return string Font family name. - */ - private static function get_font_family_name( $definition ) { - if ( ! empty( $definition['name'] ) ) { - return $definition['name']; - } - - // If the `'name'` setting was not defined, check the `fontFamily` setting. - if ( ! empty( $definition['fontFamily'] ) ) { - return static::maybe_parse_name_from_comma_separated_list( $definition['fontFamily'] ); - } - - return ''; - } - /** * Parse font-family name from comma-separated lists. * From 8483b7f1381f67b54c4198c6d9cd5037df5296ef Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Wed, 20 Sep 2023 16:37:14 -0300 Subject: [PATCH 4/7] trim quotes on font family name Co-authored-by: Brian Alexander <824344+ironprogrammer@users.noreply.github.com> --- .../fonts/font-face/class-wp-font-face-resolver.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php index 071dce3152684..01b9d4566aa67 100644 --- a/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php +++ b/lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php @@ -101,7 +101,7 @@ private static function maybe_parse_name_from_comma_separated_list( $font_family $font_family = explode( ',', $font_family )[0]; } - return $font_family; + return trim( $font_family, "\"'" ); } /** From e0f88c0fbca82de7b884e861d97036f0929a78a3 Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Wed, 20 Sep 2023 16:37:53 -0300 Subject: [PATCH 5/7] making sure the name value differs from fontFamily to validate that the former is not being used Co-authored-by: Brian Alexander <824344+ironprogrammer@users.noreply.github.com> --- .../font-face/wpFontFaceResolver/getFontsFromThemeJson.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php index a3861e6102db5..1e9cfbb43ce6a 100644 --- a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php +++ b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php @@ -176,7 +176,7 @@ public function data_should_get_font_family_name() { 'fonts' => array( array( 'fontFamily' => 'DM Sans', - 'name' => 'DM Sans', + 'name' => 'DM Sans Family', 'slug' => 'dm-sans', 'fontFace' => $font_face, ), From 60a5a50946520143a4caa9220533e1fe6ea52c21 Mon Sep 17 00:00:00 2001 From: Matias Benedetto Date: Wed, 20 Sep 2023 16:39:29 -0300 Subject: [PATCH 6/7] validate trimming changes Co-authored-by: Brian Alexander <824344+ironprogrammer@users.noreply.github.com> --- .../font-face/wpFontFaceResolver/getFontsFromThemeJson.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php index 1e9cfbb43ce6a..0888bce86a82b 100644 --- a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php +++ b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php @@ -196,7 +196,7 @@ public function data_should_get_font_family_name() { 'fontFamily comma-separated list' => array( 'fonts' => array( array( - 'fontFamily' => 'DM Sans, sans-serif', + 'fontFamily' => '"DM Sans", sans-serif', 'slug' => 'dm-sans', 'fontFace' => $font_face, ), From 29df9f8f5c6cd6cf4aabfca24cfd866574858e25 Mon Sep 17 00:00:00 2001 From: Brian Alexander <824344+ironprogrammer@users.noreply.github.com> Date: Fri, 22 Sep 2023 15:30:55 -0700 Subject: [PATCH 7/7] Remove test temporarily Because `WP_Font_Face_Resolver` already exists in Core release code, this update cannot be tested in CI without migrating the code into a temporary `_Gutenberg` resolver, which will complicate syncing back to Core. Test will be added to a follow-up PR, to merge once this update is synced to Core. Note that this test does succeed when run locally against WP `trunk` (currently 6.4-alpha). --- .../getFontsFromThemeJson.php | 104 ------------------ 1 file changed, 104 deletions(-) diff --git a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php index 0888bce86a82b..481c451bbafa0 100644 --- a/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php +++ b/phpunit/tests/fonts/font-face/wpFontFaceResolver/getFontsFromThemeJson.php @@ -101,108 +101,4 @@ public function data_should_replace_src_file_placeholder() { ), ); } - - /** - * @dataProvider data_should_get_font_family_name - * - * @param array $fonts Fonts to test. - * @param string $expected_name Expected font-family name. - */ - public function test_should_get_font_family_name( $fonts, $expected_name ) { - switch_theme( static::FONTS_THEME ); - - $replace_fonts = static function ( $theme_json_data ) use ( $fonts ) { - $data = $theme_json_data->get_data(); - - // Replace typography.fontFamilies. - $data['settings']['typography']['fontFamilies']['theme'] = $fonts; - - return new WP_Theme_JSON_Data_Gutenberg( $data ); - }; - add_filter( 'wp_theme_json_data_theme', $replace_fonts ); - $fonts = WP_Font_Face_Resolver::get_fonts_from_theme_json(); - remove_filter( 'wp_theme_json_data_theme', $replace_fonts ); - - $this->assertArrayHasKey( $expected_name, $fonts ); - } - - /** - * Data provider. - * - * @return array - */ - public function data_should_get_font_family_name() { - $font_face = array( - array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'normal', - 'fontWeight' => '400', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Regular.woff2', - ), - ), - array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '400', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Regular-Italic.woff2', - ), - ), - array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold.woff2', - ), - ), - array( - 'fontFamily' => 'DM Sans', - 'fontStretch' => 'normal', - 'fontStyle' => 'italic', - 'fontWeight' => '700', - 'src' => array( - 'file:./assets/fonts/dm-sans/DMSans-Bold-Italic.woff2', - ), - ), - ); - - return array( - 'name declared' => array( - 'fonts' => array( - array( - 'fontFamily' => 'DM Sans', - 'name' => 'DM Sans Family', - 'slug' => 'dm-sans', - 'fontFace' => $font_face, - ), - ), - 'expected_name' => 'DM Sans', - ), - 'name not declared' => array( - 'fonts' => array( - array( - 'fontFamily' => 'DM Sans', - 'slug' => 'dm-sans', - 'fontFace' => $font_face, - ), - ), - 'expected_name' => 'DM Sans', - ), - 'fontFamily comma-separated list' => array( - 'fonts' => array( - array( - 'fontFamily' => '"DM Sans", sans-serif', - 'slug' => 'dm-sans', - 'fontFace' => $font_face, - ), - ), - 'expected_name' => 'DM Sans', - ), - ); - } }