From 9025c508611ff2f6db252af5c515815112b70961 Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Fri, 31 May 2024 12:38:51 +0100 Subject: [PATCH 1/7] refactor theme fonts class for readability --- includes/create-theme/theme-fonts.php | 108 ++++++++++---------------- 1 file changed, 39 insertions(+), 69 deletions(-) diff --git a/includes/create-theme/theme-fonts.php b/includes/create-theme/theme-fonts.php index 04922fd0..f995c230 100644 --- a/includes/create-theme/theme-fonts.php +++ b/includes/create-theme/theme-fonts.php @@ -1,9 +1,11 @@ get_settings(); - if ( ! isset( $user_settings['typography']['fontFamilies']['custom'] ) ) { - return null; - } - - return $user_settings['typography']['fontFamilies']['custom']; + return $user_settings['typography']['fontFamilies']['custom'] ?? null; } public static function copy_activated_fonts_to_theme() { - $user_settings = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); - if ( ! isset( $user_settings['typography']['fontFamilies']['custom'] ) ) { - return null; - } - - $font_families_to_copy = $user_settings['typography']['fontFamilies']['custom']; + $user_settings = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); + $font_families_to_copy = $user_settings['typography']['fontFamilies']['custom'] ?? null; - // If there are no custom fonts, bounce out if ( is_null( $font_families_to_copy ) ) { return; } - $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_font_asset_location = get_stylesheet_directory() . '/assets/fonts/'; - // copy font face assets to theme and change the src to the new location require_once ABSPATH . 'wp-admin/includes/file.php'; - $theme_font_asset_location = get_stylesheet_directory() . '/assets/fonts/'; if ( ! file_exists( $theme_font_asset_location ) ) { - mkdir( $theme_font_asset_location, 0777, true ); + mkdir( $theme_font_asset_location, 0777, true ); } + foreach ( $font_families_to_copy as &$font_family ) { if ( ! isset( $font_family['fontFace'] ) ) { continue; @@ -138,16 +127,11 @@ public static function copy_activated_fonts_to_theme() { } } - // Copy user fonts to theme - if ( ! isset( $theme_json['settings']['typography']['fontFamilies'] ) ) { - $theme_json['settings']['typography']['fontFamilies'] = array(); - } $theme_json['settings']['typography']['fontFamilies'] = array_merge( - $theme_json['settings']['typography']['fontFamilies'], + $theme_json['settings']['typography']['fontFamilies'] ?? array(), $font_families_to_copy ); - // Remove user fonts unset( $user_settings['typography']['fontFamilies']['custom'] ); if ( empty( $user_settings['typography']['fontFamilies'] ) ) { unset( $user_settings['typography']['fontFamilies'] ); @@ -156,12 +140,8 @@ public static function copy_activated_fonts_to_theme() { unset( $user_settings['typography'] ); } - // Update the user settings CBT_Theme_JSON_Resolver::write_user_settings( $user_settings ); - - // Update theme.json CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); - } /** @@ -175,19 +155,16 @@ private static function remove_deactivated_font_assets( $font_families_to_not_re * if the theme.json file, missing, or if the theme is a child theme, in * which case the font families are inherited from the parent theme. */ - if ( null === $theme_font_families ) { + if ( is_null( $theme_font_families ) ) { return; } - // Remove font face assets from the theme that are not in the user configuration. $theme_font_asset_location = get_stylesheet_directory() . '/assets/fonts/'; - $font_families_to_remove = array_values( - array_filter( - $theme_font_families, - function( $theme_font_family ) use ( $font_families_to_not_remove ) { - return ! in_array( $theme_font_family['slug'], array_column( $font_families_to_not_remove, 'slug' ), true ); - } - ) + $font_families_to_remove = array_filter( + $theme_font_families, + function( $theme_font_family ) use ( $font_families_to_not_remove ) { + return ! in_array( $theme_font_family['slug'], array_column( $font_families_to_not_remove, 'slug' ), true ); + } ); foreach ( $font_families_to_remove as $font_family ) { @@ -195,13 +172,12 @@ function( $theme_font_family ) use ( $font_families_to_not_remove ) { foreach ( $font_family['fontFace'] as $font_face ) { // src can be a string or an array // if it is a string, cast it to an array - if ( ! is_array( $font_face['src'] ) ) { - $font_face['src'] = array( $font_face['src'] ); - } - foreach ( $font_face['src'] as $font_src ) { + $srcs = (array) $font_face['src']; + foreach ( $srcs as $font_src ) { $font_filename = basename( $font_src ); - if ( file_exists( $theme_font_asset_location . $font_filename ) ) { - unlink( $theme_font_asset_location . $font_filename ); + $file_path = $theme_font_asset_location . $font_filename; + if ( file_exists( $file_path ) ) { + unlink( $file_path ); } } } @@ -217,35 +193,29 @@ function( $theme_font_family ) use ( $font_families_to_not_remove ) { * This is because the user may have deactivated a font, but still want to use it in the future. */ public static function remove_deactivated_fonts_from_theme() { - $user_settings = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); - // If there are no deactivated theme fonts, bounce out if ( ! isset( $user_settings['typography']['fontFamilies']['theme'] ) ) { return; } $font_families_to_not_remove = $user_settings['typography']['fontFamilies']['theme']; + $theme_font_families = $theme_json['settings']['typography']['fontFamilies'] ?? null; - $theme_font_families = isset( $theme_json['settings']['typography']['fontFamilies'] ) ? $theme_json['settings']['typography']['fontFamilies'] : null; - static::remove_deactivated_font_assets( $font_families_to_not_remove, $theme_font_families ); + self::remove_deactivated_font_assets( $font_families_to_not_remove, $theme_font_families ); - // If there are font families in the theme, remove the deactivated ones - if ( null !== $theme_font_families ) { - $theme_json['settings']['typography']['fontFamilies'] = array_values( - array_filter( - $theme_font_families, - function( $theme_font_family ) use ( $font_families_to_not_remove ) { - return in_array( $theme_font_family['slug'], array_column( $font_families_to_not_remove, 'slug' ), true ); - } - ) + if ( ! is_null( $theme_font_families ) ) { + $theme_json['settings']['typography']['fontFamilies'] = array_filter( + $theme_font_families, + function( $theme_font_family ) use ( $font_families_to_not_remove ) { + return in_array( $theme_font_family['slug'], array_column( $font_families_to_not_remove, 'slug' ), true ); + } ); } CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); - // Remove deactivated fonts from user settings unset( $user_settings['typography']['fontFamilies']['theme'] ); if ( empty( $user_settings['typography']['fontFamilies'] ) ) { unset( $user_settings['typography']['fontFamilies'] ); From cd5d64f88183b2ed5bdc9ac9e345cf3edc85bdcb Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Fri, 31 May 2024 13:13:06 +0100 Subject: [PATCH 2/7] add test for src and url handling --- tests/test-theme-fonts.php | 114 ++++++++++++++++++++++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/tests/test-theme-fonts.php b/tests/test-theme-fonts.php index 24046f53..e7807393 100644 --- a/tests/test-theme-fonts.php +++ b/tests/test-theme-fonts.php @@ -22,7 +22,6 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) { ); } - public function test_copy_activated_fonts_to_theme() { wp_set_current_user( self::$admin_id ); @@ -214,6 +213,117 @@ public function test_get_all_fonts_from_theme_and_variation() { } + public function test_non_array_font_src() { + wp_set_current_user( self::$admin_id ); + + $test_theme_slug = $this->create_blank_theme(); + + // Create a theme with a non-array font src + $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_json['settings']['typography']['fontFamilies'] = array( + array( + 'slug' => 'single-src-font', + 'name' => 'Single Src Font', + 'fontFamily' => 'Single Src Font', + 'fontFace' => array( + array( + 'fontFamily' => 'Single Src Font', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'file:./assets/fonts/single-src-font.ttf', + ), + ), + ), + ); + CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); + + // Attempt to get all fonts + $fonts = CBT_Theme_Fonts::get_all_fonts(); + + // Verify that the src is correctly handled + $this->assertCount( 1, $fonts ); + $this->assertEquals( 'single-src-font', $fonts[0]['slug'] ); + $this->assertEquals( get_stylesheet_directory_uri() . '/assets/fonts/single-src-font.ttf', $fonts[0]['fontFace'][0]['src'] ); + + $this->uninstall_theme( $test_theme_slug ); + } + + public function test_array_font_src() { + wp_set_current_user( self::$admin_id ); + + $test_theme_slug = $this->create_blank_theme(); + + // Create a theme with an array font src + $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_json['settings']['typography']['fontFamilies'] = array( + array( + 'slug' => 'array-src-font', + 'name' => 'Array Src Font', + 'fontFamily' => 'Array Src Font', + 'fontFace' => array( + array( + 'fontFamily' => 'Array Src Font', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => array( + 'file:./assets/fonts/array-src-font.ttf', + 'file:./assets/fonts/array-src-font-bold.ttf', + ), + ), + ), + ), + ); + CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); + + // Attempt to get all fonts + $fonts = CBT_Theme_Fonts::get_all_fonts(); + + // Verify that the src is correctly handled + $this->assertCount( 1, $fonts ); + $this->assertEquals( 'array-src-font', $fonts[0]['slug'] ); + $this->assertIsArray( $fonts[0]['fontFace'][0]['src'] ); + $this->assertCount( 2, $fonts[0]['fontFace'][0]['src'] ); + $this->assertEquals( get_stylesheet_directory_uri() . '/assets/fonts/array-src-font.ttf', $fonts[0]['fontFace'][0]['src'][0] ); + $this->assertEquals( get_stylesheet_directory_uri() . '/assets/fonts/array-src-font-bold.ttf', $fonts[0]['fontFace'][0]['src'][1] ); + + $this->uninstall_theme( $test_theme_slug ); + } + + public function test_absolute_url_handling() { + wp_set_current_user( self::$admin_id ); + + $test_theme_slug = $this->create_blank_theme(); + + // Create a theme with an absolute URL + $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_json['settings']['typography']['fontFamilies'] = array( + array( + 'slug' => 'absolute-url-font', + 'name' => 'Absolute URL Font', + 'fontFamily' => 'Absolute URL Font', + 'fontFace' => array( + array( + 'fontFamily' => 'Absolute URL Font', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'http://example.com/fonts/absolute-url-font.ttf', + ), + ), + ), + ); + CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); + + // Attempt to get all fonts + $fonts = CBT_Theme_Fonts::get_all_fonts(); + + // Verify that the absolute URL remains unchanged + $this->assertCount( 1, $fonts ); + $this->assertEquals( 'absolute-url-font', $fonts[0]['slug'] ); + $this->assertEquals( 'http://example.com/fonts/absolute-url-font.ttf', $fonts[0]['fontFace'][0]['src'] ); + + $this->uninstall_theme( $test_theme_slug ); + } + private function save_theme() { CBT_Theme_Fonts::persist_font_settings(); } @@ -324,5 +434,5 @@ private function activate_font_in_theme_and_override_in_user() { CBT_Theme_JSON_Resolver::write_user_settings( $settings ); } - } + From ca60fbd0822b38baeb0f70d8767de30ca0b1ac13 Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Fri, 31 May 2024 13:33:37 +0100 Subject: [PATCH 3/7] add more assertions to theme deactivation fn --- tests/test-theme-fonts.php | 75 ++++++++++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 12 deletions(-) diff --git a/tests/test-theme-fonts.php b/tests/test-theme-fonts.php index e7807393..98716c81 100644 --- a/tests/test-theme-fonts.php +++ b/tests/test-theme-fonts.php @@ -63,41 +63,92 @@ public function test_remove_deactivated_fonts_from_theme() { $test_theme_slug = $this->create_blank_theme(); - $this->activate_font_in_theme_and_override_in_user(); + // Create a theme with multiple fonts + $theme_json = CBT_Theme_JSON_Resolver::get_theme_file_contents(); + $theme_json['settings']['typography']['fontFamilies'] = array( + array( + 'slug' => 'open-sans', + 'name' => 'Open Sans', + 'fontFamily' => 'Open Sans', + 'fontFace' => array( + array( + 'fontFamily' => 'Open Sans', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'file:./assets/fonts/open-sans-normal-400.ttf', + ), + ), + ), + array( + 'slug' => 'deactivated-font', + 'name' => 'Deactivated Font', + 'fontFamily' => 'Deactivated Font', + 'fontFace' => array( + array( + 'fontFamily' => 'Deactivated Font', + 'fontStyle' => 'normal', + 'fontWeight' => '400', + 'src' => 'file:./assets/fonts/deactivated-font.ttf', + ), + ), + ), + ); + CBT_Theme_JSON_Resolver::write_theme_file_contents( $theme_json ); + + // Create the font files + $font_dir = get_stylesheet_directory() . '/assets/fonts/'; + if ( ! file_exists( $font_dir ) ) { + mkdir( $font_dir, 0777, true ); + } + file_put_contents( $font_dir . 'open-sans-normal-400.ttf', 'dummy content' ); + file_put_contents( $font_dir . 'deactivated-font.ttf', 'dummy content' ); + + // Simulate user deactivating the 'deactivated-font' font + $user_settings = array(); + $user_settings['typography']['fontFamilies']['theme'] = array( + array( + 'slug' => 'open-sans', + 'name' => 'Open Sans', + 'fontFamily' => 'Open Sans', + ), + ); + CBT_Theme_JSON_Resolver::write_user_settings( $user_settings ); $user_data_before = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); $theme_data_before = CBT_Theme_JSON_Resolver::get_theme_data()->get_settings(); $merged_data_before = CBT_Theme_JSON_Resolver::get_merged_data()->get_settings(); - $theme_file_exists_before = file_exists( get_stylesheet_directory() . '/assets/fonts/open-sans-normal-400.ttf' ); + $theme_file_exists_before = file_exists( $font_dir . 'open-sans-normal-400.ttf' ); - $this->save_theme(); + // Call the method to remove deactivated fonts + CBT_Theme_Fonts::remove_deactivated_fonts_from_theme(); $user_data_after = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); $theme_data_after = CBT_Theme_JSON_Resolver::get_theme_data()->get_settings(); $merged_data_after = CBT_Theme_JSON_Resolver::get_merged_data()->get_settings(); - $theme_file_exists_after = file_exists( get_stylesheet_directory() . '/assets/fonts/open-sans-normal-400.ttf' ); + $theme_file_exists_after = file_exists( $font_dir . 'open-sans-normal-400.ttf' ); // ensure that the font was added to the theme settings and removed in user settings and therefore missing in merged settings $this->assertCount( 2, $theme_data_before['typography']['fontFamilies']['theme'] ); - $this->assertequals( 'open-sans', $theme_data_before['typography']['fontFamilies']['theme'][1]['slug'] ); + $this->assertequals( 'open-sans', $theme_data_before['typography']['fontFamilies']['theme'][0]['slug'] ); $this->assertCount( 1, $user_data_before['typography']['fontFamilies']['theme'] ); - $this->assertnotequals( 'open-sans', $user_data_before['typography']['fontFamilies']['theme'][0]['slug'] ); $this->assertCount( 1, $merged_data_before['typography']['fontFamilies']['theme'] ); - $this->assertnotequals( 'open-sans', $merged_data_before['typography']['fontFamilies']['theme'][0]['slug'] ); // ensure that the font was removed from the user settings and removed from the theme settings and therefore missing in merged settings $this->assertCount( 1, $theme_data_after['typography']['fontFamilies']['theme'] ); - $this->assertnotequals( 'open-sans', $theme_data_after['typography']['fontFamilies']['theme'][0]['slug'] ); + $this->assertEquals( 'open-sans', $theme_data_after['typography']['fontFamilies']['theme'][0]['slug'] ); $this->assertarraynothaskey( 'typography', $user_data_after ); - $this->assertnotequals( 'open-sans', $theme_data_after['typography']['fontFamilies']['theme'][0]['slug'] ); $this->assertCount( 1, $merged_data_after['typography']['fontFamilies']['theme'] ); - $this->assertnotequals( 'open-sans', $merged_data_after['typography']['fontFamilies']['theme'][0]['slug'] ); - // ensure that the file resource was removed + // ensure that the font asset was removed $this->assertTrue( $theme_file_exists_before ); - $this->assertFalse( $theme_file_exists_after ); + $this->assertFalse( file_exists( $font_dir . 'deactivated-font.ttf' ) ); + $this->assertTrue( $theme_file_exists_after ); $this->uninstall_theme( $test_theme_slug ); + + $theme_file_exists_after_uninstall = file_exists( $font_dir . 'open-sans-normal-400.ttf' ); + // ensure that the font asset was removed after uninstalling the theme + $this->assertFalse( $theme_file_exists_after_uninstall ); } public function test_get_all_fonts_just_theme() { From 2b409f875a4d5e289bcf81295f78e17959b05503 Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Fri, 31 May 2024 13:34:00 +0100 Subject: [PATCH 4/7] refactor missing array cast --- includes/create-theme/theme-fonts.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/includes/create-theme/theme-fonts.php b/includes/create-theme/theme-fonts.php index f995c230..09c59eb6 100644 --- a/includes/create-theme/theme-fonts.php +++ b/includes/create-theme/theme-fonts.php @@ -107,9 +107,7 @@ public static function copy_activated_fonts_to_theme() { foreach ( $font_family['fontFace'] as &$font_face ) { // src can be a string or an array // if it is a string, cast it to an array - if ( ! is_array( $font_face['src'] ) ) { - $font_face['src'] = array( $font_face['src'] ); - } + $font_face['src'] = (array) $font_face['src']; foreach ( $font_face['src'] as $font_src_index => &$font_src ) { $font_filename = basename( $font_src ); $font_dir = wp_get_font_dir(); From 5fee7215929c5b8bac3852d4df08dad60a3a1838 Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Tue, 4 Jun 2024 11:34:03 -0400 Subject: [PATCH 5/7] Update includes/create-theme/theme-fonts.php Co-authored-by: Jason Crist --- includes/create-theme/theme-fonts.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/includes/create-theme/theme-fonts.php b/includes/create-theme/theme-fonts.php index 09c59eb6..48a7d120 100644 --- a/includes/create-theme/theme-fonts.php +++ b/includes/create-theme/theme-fonts.php @@ -85,8 +85,7 @@ public static function get_user_activated_fonts() { } public static function copy_activated_fonts_to_theme() { - $user_settings = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); - $font_families_to_copy = $user_settings['typography']['fontFamilies']['custom'] ?? null; + $font_families_to_copy = self::get_user_activated_fonts(); if ( is_null( $font_families_to_copy ) ) { return; From 2464bc9acb93d49b3f24074e3678e6a074d1cdf2 Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Tue, 4 Jun 2024 16:39:42 +0100 Subject: [PATCH 6/7] re-add $user_settings --- includes/create-theme/theme-fonts.php | 1 + 1 file changed, 1 insertion(+) diff --git a/includes/create-theme/theme-fonts.php b/includes/create-theme/theme-fonts.php index 48a7d120..d29a68a6 100644 --- a/includes/create-theme/theme-fonts.php +++ b/includes/create-theme/theme-fonts.php @@ -129,6 +129,7 @@ public static function copy_activated_fonts_to_theme() { $font_families_to_copy ); + $user_settings = CBT_Theme_JSON_Resolver::get_user_data()->get_settings(); unset( $user_settings['typography']['fontFamilies']['custom'] ); if ( empty( $user_settings['typography']['fontFamilies'] ) ) { unset( $user_settings['typography']['fontFamilies'] ); From 899487c2377ad442f92043c5b0391f0efb91350f Mon Sep 17 00:00:00 2001 From: Vicente Canales <1157901+vcanales@users.noreply.github.com> Date: Tue, 4 Jun 2024 18:11:15 +0100 Subject: [PATCH 7/7] add additional assertion --- tests/test-theme-fonts.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test-theme-fonts.php b/tests/test-theme-fonts.php index 98716c81..a9d07d3e 100644 --- a/tests/test-theme-fonts.php +++ b/tests/test-theme-fonts.php @@ -129,7 +129,8 @@ public function test_remove_deactivated_fonts_from_theme() { // ensure that the font was added to the theme settings and removed in user settings and therefore missing in merged settings $this->assertCount( 2, $theme_data_before['typography']['fontFamilies']['theme'] ); - $this->assertequals( 'open-sans', $theme_data_before['typography']['fontFamilies']['theme'][0]['slug'] ); + $this->assertEquals( 'open-sans', $theme_data_before['typography']['fontFamilies']['theme'][0]['slug'] ); + $this->assertEquals( 'deactivated-font', $theme_data_before['typography']['fontFamilies']['theme'][1]['slug'] ); $this->assertCount( 1, $user_data_before['typography']['fontFamilies']['theme'] ); $this->assertCount( 1, $merged_data_before['typography']['fontFamilies']['theme'] );