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.

Sign up for GitHub

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

Fonts API: Add is_array check before using array_merge #55974

Closed
wants to merge 2 commits into from
Closed

Fonts API: Add is_array check before using array_merge #55974

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffabdfa
Add is_array check around array_merge
mikachan Nov 8, 2023
e50f0a5
Refactor to continue if values are not arrays
mikachan Nov 8, 2023
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
5 changes: 5 additions & 0 deletions lib/experimental/fonts-api/class-wp-fonts-resolver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,11 @@ private static function get_settings() {
$settings = static::set_tyopgraphy_settings_array_structure( $settings );
}

// Skip if typography.fontFamilies.theme from the global settings and the variation are not arrays.
if ( ! is_array( $settings['typography']['fontFamilies']['theme'] ) && ! is_array( $variation['settings']['typography']['fontFamilies']['theme'] ) ) {
Copy link
Contributor

@azaozz azaozz Nov 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Seems this should be || not && as one of these may be null or perhaps not set? Also perhaps checking them separately may be better? Then the $settings['typography']['fontFamilies']['theme'] can probably be above the loop as it cannot change to non-array in the loop.

Copy link
Member Author

Choose a reason for hiding this comment

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

This makes sense, thank you!

I noticed #55981 is probably closer to a better fix, so happy to close this PR is favour of that. Or also happy to continue working on this one if it ends up being needed.

continue;
}

// Merge the variation settings with the global settings.
$settings['typography']['fontFamilies']['theme'] = array_merge(
$settings['typography']['fontFamilies']['theme'],
Expand Down