Make sure theme families are array before attempting to merge them

[vcanales]

Follow up to #55981

[github-actions]

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/experimental/fonts-api/class-wp-fonts-resolver.php

[anton-vlasenko]

Excuse me, but I don't understand why this change is necessary.
If $settings['typography']['fontFamilies']['theme'] is null, it will be set to an array here (around line 205):

if ( ! isset( $settings['typography']['fontFamilies']['theme'] ) || ! is_array( $settings['typography']['fontFamilies']['theme'] ) ) {
    $settings['typography']['fontFamilies']['theme'] = array();
}

[vcanales]

My best guess is that it's needed because the line you mention depends on us being within the editor:

gutenberg/lib/experimental/fonts-api/class-wp-fonts-resolver.php

Line 190 in 787f8ea

$set_theme_structure = true;

gutenberg/lib/experimental/fonts-api/class-wp-fonts-resolver.php

Line 200 in 787f8ea

if ( $set_theme_structure ) {

gutenberg/lib/experimental/fonts-api/class-wp-fonts-resolver.php

Line 201 in 787f8ea

$set_theme_structure = false;

If I understand that logic correctly, the second time we go through this condition, $set_theme_structure could be false, hence $settings['typography']['fontFamilies']['theme'] never getting initialized as an array. Another solution would be to move that initialization out of the conditional on $set_theme_structure.

[anton-vlasenko]

$settings['typography']['fontFamilies']['theme'] never getting initialized as an array.

Yes, $set_theme_structure is false on the second iteration of the foreach loop.

However, I don't understand how this affects $settings['typography']['fontFamilies']['theme'].
$settings['typography']['fontFamilies']['theme'] is set to an array during the first iteration of the loop and remains an array in subsequent iterations. array_merge() always returns an array value.

[vcanales]

gutenberg/lib/experimental/fonts-api/class-wp-fonts-resolver.php

Line 222 in 787f8ea

$settings['typography']['fontFamilies'] = array_unique( $settings['typography']['fontFamilies'] );

After array_unique on line 222, $settings['typography']['fontFamilies']['theme'] is going null, and it doesn't get reset on the next iteration. It's probably going null because array_unique is getting rid of a duplicate key that contains the the theme array, because the function isn't really comparing the value? I don't think array_unique works well with multidimensional arrays.

[arthur791004]

Yes, the main issue is the $settings['typography']['fontFamilies']['theme'] becomes null after array_unique as it removes the key with the duplicate value.

Hence, I proposed #56067 to

1. Set the flag of array_unique to SORT_REGULAR. The default flag is SORT_STRING and it means that the elements are converted to a string to do the comparison. However, the value after the conversion becomes Array and it leads to $settings['typography']['fontFamilies']['theme'] being null if the $settings['typography']['fontFamilies'] contains multiple keys.

   $settings['typography']['fontFamilies'] = array(
     'default' => array(),
     'theme' => array(),
   );
   
   print_r( (string) $settings['typography']['fontFamilies']['default'] ); // Array
   print_r( (string) $settings['typography']['fontFamilies']['theme'] ); // Array
   

2. Reset the $settings['typography']['fontFamilies']['theme'] after the array_unique call to avoid it becoming null in the next iteration. WDYT?

[anton-vlasenko]

@vcanales

After array_unique on line 222, $settings['typography']['fontFamilies']['theme'] is going null,

I'm not able to reproduce this.

Can you provide the values of $settings and $variations at which this issue occurs? Please see this code snippet: https://3v4l.org/InOuS#v7.3.0

After array_unique is applied on line 222, $settings['typography']['fontFamilies']['theme'] becomes null and does not reset in the subsequent iteration.

Can you provide an example of such behavior? Specifically, what exact array should I pass to array_unique() in order to make $settings['typography']['fontFamilies']['theme'] a null value?

[vcanales]

Closing in favor of #56067, which seems like a more robust solution.