Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Got error message "There has been a critical error on this website". when I pushed "Save Changes to Theme" button after I changed font setting in the chilid theme. #659

Closed
megane9988 opened this issue May 30, 2024 · 2 comments · Fixed by #660
Assignees
Labels
bug Something isn't working

Comments

@megane9988
Copy link

megane9988 commented May 30, 2024

Steps

  • Create child theme from TT4
  • Off the check of Cardo Bold from the Typography setting
  • Push the Save Changes to Theme button

Displaying error

<p></p><p><a href="https://wordpress.org/documentation/article/faq-troubleshooting/">Learn more about troubleshooting WordPress.</a></p>

error log

[30-May-2024 16:59:29 UTC] PHP Fatal error:  Uncaught TypeError: array_filter(): Argument #1 ($array) must be of type array, null given in /Users/9988megane/Local Sites/cbt-error/app/public/wp-content/plugins/create-block-theme/includes/create-theme/theme-fonts.php:186
Stack trace:
#0 /Users/9988megane/Local Sites/cbt-error/app/public/wp-content/plugins/create-block-theme/includes/create-theme/theme-fonts.php(186): array_filter(NULL, Object(Closure))
#1 /Users/9988megane/Local Sites/cbt-error/app/public/wp-content/plugins/create-block-theme/includes/create-theme/theme-fonts.php(80): CBT_Theme_Fonts::remove_deactivated_fonts_from_theme()
#2 /Users/9988megane/Local Sites/cbt-error/app/public/wp-content/plugins/create-block-theme/includes/class-create-block-theme-api.php(495): CBT_Theme_Fonts::persist_font_settings()
#3 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/rest-api/class-wp-rest-server.php(1230): CBT_Theme_API->rest_save_theme(Object(WP_REST_Request))
#4 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/rest-api/class-wp-rest-server.php(1063): WP_REST_Server->respond_to_request(Object(WP_REST_Request), '/create-block-t...', Array, NULL)
#5 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/rest-api/class-wp-rest-server.php(439): WP_REST_Server->dispatch(Object(WP_REST_Request))
#6 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/rest-api.php(428): WP_REST_Server->serve_request('/create-block-t...')
#7 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/class-wp-hook.php(324): rest_api_loaded(Object(WP))
#8 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters('', Array)
#9 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/plugin.php(565): WP_Hook->do_action(Array)
#10 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/class-wp.php(418): do_action_ref_array('parse_request', Array)
#11 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/class-wp.php(813): WP->parse_request('')
#12 /Users/9988megane/Local Sites/cbt-error/app/public/wp-includes/functions.php(1336): WP->main('')
#13 /Users/9988megane/Local Sites/cbt-error/app/public/wp-blog-header.php(16): wp()
#14 /Users/9988megane/Local Sites/cbt-error/app/public/index.php(17): require('/Users/9988mega...')
#15 {main}
  thrown in /Users/9988megane/Local Sites/cbt-error/app/public/wp-content/plugins/create-block-theme/includes/create-theme/theme-fonts.php on line 186
@megane9988 megane9988 changed the title Got error message There has been a critical error on this website. when I pushed Save Changes to Theme after I changed font setting in the chilid theme. May 30, 2024
@megane9988 megane9988 changed the title Got error message There has been a critical error on this website. when I pushed Save Changes to Theme after I changed font setting in the chilid theme. May 30, 2024
@vcanales vcanales self-assigned this May 30, 2024
@vcanales vcanales added the bug Something isn't working label May 30, 2024
@t-hamano
Copy link
Contributor

Thanks for the report.

I think this problem occurs when a child theme is created based on a theme that has not been customized in any way, and the following theme.json file is generated for the child theme:

{
	"version": 2,
	"$schema": "https://schemas.wp.org/wp/6.6/theme.json"
}

And the error is caused by the value here not being an array.

The error itself can be addressed by making the following changes:

diff --git a/includes/create-theme/theme-fonts.php b/includes/create-theme/theme-fonts.php
index 0e403b6..315f56d 100644
--- a/includes/create-theme/theme-fonts.php
+++ b/includes/create-theme/theme-fonts.php
@@ -175,12 +175,13 @@ class CBT_Theme_Fonts {
                }
 
                $font_families_to_not_remove = $user_settings['typography']['fontFamilies']['theme'];
+               $theme_font_families         = $theme_json['settings']['typography']['fontFamilies'] ?? array();
 
                // Remove font assets from theme
                $theme_font_asset_location = get_stylesheet_directory() . '/assets/fonts/';
                $font_families_to_remove   = array_values(
                        array_filter(
-                               $theme_json['settings']['typography']['fontFamilies'],
+                               $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 );
                                }
@@ -207,7 +208,7 @@ class CBT_Theme_Fonts {
                // Remove user fonts from theme
                $theme_json['settings']['typography']['fontFamilies'] = array_values(
                        array_filter(
-                               $theme_json['settings']['typography']['fontFamilies'],
+                               $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 );
                                }

However, I don't think this is a fundamental solution. If we make the above changes and save the child theme, for example, the child theme's theme.json will have the following data:

{
	"settings": {
		"typography": {
			"fontFamilies": [
				{
					"fontFace": [
						{
							"fontFamily": "ABeeZee",
							"fontStyle": "normal",
							"fontWeight": "400",
							"src": [
								"file:./assets/fonts/esDR31xSG-6AGleN6teukbcHCpE.woff2"
							]
						},
						{
							"fontFamily": "ABeeZee",
							"fontStyle": "italic",
							"fontWeight": "400",
							"src": [
								"file:./assets/fonts/esDT31xSG-6AGleN2tCkkJUCGpG-GQ.woff2"
							]
						}
					],
					"fontFamily": "ABeeZee, sans-serif",
					"name": "ABeeZee",
					"slug": "abeezee"
				}
			]
		}
	},
	"version": 2,
	"$schema": "https://schemas.wp.org/wp/6.6/theme.json"
}

There are two problems here:

  • The parent theme's font families are missing
  • The font file pointed to by the font face path does not exist in the child theme

Perhaps it is necessary to add a process that takes into account whether it is a child theme or not when regenerating the font file and theme.json.

@vcanales
Copy link
Member

However, I don't think this is a fundamental solution. If we make the above changes and save the child theme, for example, the child theme's theme.json will have the following data...

There are two problems here:

  • The parent theme's font families are missing
  • The font file pointed to by the font face path does not exist in the child theme

I was working on #660 which deals with these concerns. The changes make sure that we're only deactivating and removing assets if they're actually present. The child theme's theme.json file is not modified when it does not have theme fonts, and the child theme still relies on the parent's fonts and their assets instead of trying to set them.

vcanales added a commit that referenced this issue Jun 4, 2024
* Check if theme fonts present before removing

Fixes: #659

* make sure remove_deactivated_font_assets is static

* use correct condition for asset removal

ie. bail on null

* remove unused variable

* add comments and reword some of the existing ones
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
3 participants