Skip to content
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
[not verified] Be more defensive about when renaming
  • Loading branch information
simison committed Dec 19, 2022
commit af4ccca95406d88928f3358e19b253da94b3c963
18 changes: 17 additions & 1 deletion projects/plugins/jetpack/modules/google-fonts.php
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,23 @@ function jetpack_add_google_fonts_provider() {
* @return WP_Theme_JSON_Data_Gutenberg Class with updated Global Styles settings.
*/
function jetpack_rename_google_font_names( $theme_json ) {
$raw_data = $theme_json->get_data();
// Skip at front of the site; renaming is needed only when in the editor and API requests
if ( ! is_admin() || ! ( defined( 'REST_REQUEST' ) && REST_REQUEST ) ) {
return $theme_json;
}

$raw_data = $theme_json->get_data();

// Skip if fontFamilies are not defined in the variation.
if (
empty( $raw_data['settings'] ) ||
empty( $raw_data['settings']['typography'] ) ||
empty( $raw_data['settings']['typography']['fontFamilies'] ) ||
empty( $raw_data['settings']['typography']['fontFamilies']['theme'] )
) {
return $theme_json;
}

$font_families = $raw_data['settings']['typography']['fontFamilies']['theme'];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do all block themes have all of these keys defined? If not it might make sense to return early if an isset() returns false.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will JETPACK_GOOGLE_FONTS_LIST be part of $font_families? Or is $font_families the static representation of that theme json data?

Copy link
Member Author

@simison simison Dec 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are always defined because before this filter we registered the fonts, and they end up to theme style object when registered with with wp_register_webfonts() — even from a plugin. A bit confusing. :-)

That said, something that could in theory happen is that all Jetpack fonts or typography settings get filtered out or shortcirquitted by some plugin or theme. The shape of the data could also change later on in Gutenberg. $theme_json->update_with() takes care of versioning later on but here we don't have that safeguard. I'll add a check!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few checks;

  • tests if array keys exist
  • Skip renaming entirely for non wp-admin/rest API requests.


$renamed_fonts = array(
Expand Down