#56078 closed defect (bug) (wontfix)
PHP Warning when adding fonts to fontFamilies in wp_global_styles post
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Priority: | normal | |
Severity: | normal | Version: | 6.0 |
Component: | Editor | Keywords: | |
Focuses: | administration, template | Cc: |
Description (last modified by )
I am injecting additional Google Fonts to the Full Site Editor global styles by adding elements to ['settings']['typography']['fontFamilies']
of the wp_global_styles post type in the database (rather than users having to modify the theme.json or having to create a child theme).
Which works! :)
However, it is presenting the following error ...
[27-Jun-2022 14:08:45 UTC] PHP Warning: Array to string conversion in /wp-includes/script-loader.php on line 3100
I suspect this is because on line 3100 of script-loader.php ...
<?php $settings['typography']['fontFamilies'] = array_unique( $settings['typography']['fontFamilies'] );
... is being used to make sure the font families are not duplicated ... however, you can only use array_unique with an array of string values, but "fontFamilies" is an array of arrays. I believe this line should be changed to ...
<?php $settings['typography']['fontFamilies'] = array_unique( $settings['typography']['fontFamilies'], SORT_REGULAR);
Please confirm and I'll create a patch file if you like.
Thanks,
Oliver
Attachments (1)
Change History (21)
#2
@
3 years ago
- Keywords has-patch added
I've been using the proposed fix for over a month and it works for me here. I've added as a patch to this ticket. Hope that's OK?
Oliver
#4
@
2 years ago
Please note that v6.1 still has this bug and the fix now needs to be applied to line 3229.
Oliver
This ticket was mentioned in Slack in #core by domainsupport. View the logs.
2 years ago
#8
@
2 years ago
@domainsupport could you please share a snippet of your implementation of additional Google Fonts, to help testing the patch?
Thank you!
#9
follow-up:
↓ 10
@
2 years ago
Sure ... so here's the post_content
of a wp_global_styles
post for twentytwentytwo theme which successfully adds "La Belle Aurore" and "Open Sans" to the Global Styles font family drop down ...
{"version":2,"isGlobalStylesUserThemeJSON":true,"settings":{"typography":{"fontFamilies":[{"fontFamily":"\"La Belle Aurore\"","name":"La Belle Aurore","slug":"la-belle-aurore"},{"fontFamily":"\"Open Sans\"","name":"Open Sans","slug":"open-sans"},{"fontFamily":"\"Source Serif Pro\", serif","name":"Source Serif Pro","slug":"source-serif-pro","fontFace":[{"fontFamily":"Source Serif Pro","fontWeight":"200 900","fontStyle":"normal","fontStretch":"normal","src":["file:.\/assets\/fonts\/source-serif-pro\/SourceSerif4Variable-Roman.ttf.woff2"]},{"fontFamily":"Source Serif Pro","fontWeight":"200 900","fontStyle":"italic","fontStretch":"normal","src":["file:.\/assets\/fonts\/source-serif-pro\/SourceSerif4Variable-Italic.ttf.woff2"]}]}]}}}
As soon as you have this in the database, the PHP notices will start.
Notice that default theme fonts are also in there.
Oliver
#10
in reply to:
↑ 9
;
follow-up:
↓ 11
@
2 years ago
Replying to domainsupport:
Sure ... so here's the
post_content
of a...
Thanks, this is helpful.
I think what @audrasjb meant was to show an example of the code that adds these extra fonts.
injecting additional Google Fonts to the Full Site Editor global styles by adding elements to
['settings']['typography']['fontFamilies']
of the wp_global_styles post type in the database
Hmmm, not sure if this is supported/expected/documented way of adding more fonts? :)
you can only use array_unique with an array of string values
Right, array_unique()
cannot be used for multi-dimensional arrays. There's an example of a function in the PHP manual that may be helpful here: https://www.php.net/manual/en/function.array-unique.php#116302.
#11
in reply to:
↑ 10
;
follow-up:
↓ 12
@
2 years ago
Replying to azaozz:
I think what @audrasjb meant was to show an example of the code that adds these extra fonts.
Oh right, sure. OK, well the easiest way to demonstrate that is to install Template Editor plugin https://wordpress.org/plugins/template-editor/ then go to "Dashboard - Appearance - Theme Options", choose a font (and style) and "Save Options".
The code to save the fonts can be found here https://plugins.trac.wordpress.org/browser/template-editor/trunk/template-editor.php#L1396
injecting additional Google Fonts to the Full Site Editor global styles by adding elements to
['settings']['typography']['fontFamilies']
of the wp_global_styles post type in the database
Hmmm, not sure if this is supported/expected/documented way of adding more fonts? :)
Haha :) Sure, it's not documented ... but the way that script-loader.php
loads the fonts from those in the wp_global_styles
post and then merges them with those from theme.json
suggests that it is expected (even if array_unique()
isn't used correctly, hence the error).
you can only use array_unique with an array of string values
Right,
array_unique()
cannot be used for multi-dimensional arrays. There's an example of a function in the PHP manual that may be helpful here: https://www.php.net/manual/en/function.array-unique.php#116302.
Yes, I've used something similar in the past ... are you saying you'd like me to amend the patch accordingly rather than just adding SORT_REGULAR
which prevents the PHP notification?
Oliver
#12
in reply to:
↑ 11
@
2 years ago
Replying to domainsupport:
the easiest way to demonstrate that is to install Template Editor plugin
Thanks, this is really helpful.
the way that
script-loader.php
loads the fonts from those in thewp_global_styles
post and then merges them with those fromtheme.json
suggests that it is expected
I tried to reproduce this without using the Template Editor plugin, but seems it's not possible. Assuming there are very few cases, if any, where this bug is triggered. That's not surprising as _wp_theme_json_webfonts_handler()
is clearly marked as a temporary solution: "A future public Webfonts API will replace this stopgap code.".
At the same time the use of array_unique()
is still a bug and should probably be fixed, even in this temporary code.
are you saying you'd like me to amend the patch accordingly rather than just adding
SORT_REGULAR
which prevents the PHP notification?
The intention here is to make sure no duplicate font families remain in the settings after merging them with the fonts from theme.json. As far as I see fontFamily
is the identification, so probably best to ensure there are no duplicates of it even if name
and slug
are not the same. Perhaps the function that looks in the arrays and compares sub-array elements would work best.
On the other hand this is a temporary, stopgap code that is going to be removed soon (I hope), so maybe adding SORT_REGULAR
would be enough. Going to defer to @hellofromTonya here for a decision.
#13
follow-up:
↓ 16
@
2 years ago
No wonder it was so difficult to find a way to inject fonts into the Global Styles interface! I've just read this on https://make.wordpress.org/core/2022/04/22/status-of-webfonts-api-for-wordpress-6-0-inclusion/ ...
This led to the decision to pursue a pared down version that only supports declaring fonts within a theme.json file, that is only available to theme authors, rather than to the wider extender base (i.e., plugin authors). To ensure this approach, the core of the API was put into a private theme.json handler preventing plugins from registering or enqueuing, bypassing the current performance and robustness concerns.
So ... my bad for finding this loop-hole way to do precisely what is quite clearly not meant to be done but like you say, it is a bug nonetheless :)
If it were up to me I would vote for custom multidimensional array unique function unless the Webfonts API is imminent ... ?
This ticket was mentioned in Slack in #core by mukeshpanchal27. View the logs.
2 years ago
#15
@
2 years ago
This ticket was discussed during the recent bug scrub. It looks like it's unlikely that work will be done on this during the 6.2 cycle.
@azaozz @audrasjb Is this still possible to land in 6.2, or should it be moved to Future Release
for now?
#16
in reply to:
↑ 13
@
2 years ago
- Keywords has-patch needs-testing removed
- Milestone 6.2 deleted
- Resolution set to wontfix
- Status changed from new to closed
Replying to domainsupport:
No wonder it was so difficult to find a way to inject fonts into the Global Styles interface! I've just read this on https://make.wordpress.org/core/2022/04/22/status-of-webfonts-api-for-wordpress-6-0-inclusion/ ...
This led to the decision to pursue a pared down version that only supports declaring fonts within a theme.json file, that is only available to theme authors, rather than to the wider extender base (i.e., plugin authors). To ensure this approach, the core of the API was put into a private theme.json handler preventing plugins from registering or enqueuing, bypassing the current performance and robustness concerns.
So ... my bad for finding this loop-hole way to do precisely what is quite clearly not meant to be done but like you say, it is a bug nonetheless :)
If it were up to me I would vote for custom multidimensional array unique function unless the Webfonts API is imminent ... ?
I'm hesitant to change the current stopgap code in Core. Why?
- It's temporary.
- Injecting fonts is doing it wrong at this time, as the stopgap does not support nor was it designed for any fonts other than those in the active theme's
theme.json
file.
Registering fonts is supported in the Fonts API being tested and refined in Gutenberg. This API will be introduced into Core in 6.3 which will remove this stopgap code.
@domainsupport I invite you test and help with the Fonts API https://github.com/WordPress/gutenberg/issues/41479.
I'm closing this ticket as wontfix
since the injecting fonts is not supported nor intended.
#17
@
2 years ago
Thank you @hellofromTonya, it is regrettable that a bug in core that's relatively simple to fix will remain for at least the next 6 months but I take onboard your reasoning and will gladly test the Fonts API in due course which sounds fantastic and something that's been required for years!
Oliver
#18
follow-up:
↓ 19
@
19 months ago
Hi @hellofromTonya, bearing in mind the status of the Fonts API and the stopgap code is still currently being used ... can this bug please be re-appraised and possibly fixed?
I would be happy to create a patch that uses a function to replace array_unique()
for this multi-dimensional array?
Oliver
#19
in reply to:
↑ 18
@
19 months ago
Replying to domainsupport:
Hi @hellofromTonya, bearing in mind the status of the Fonts API and the stopgap code is still currently being used ... can this bug please be re-appraised and possibly fixed?
I would be happy to create a patch that uses a function to replace
array_unique()
for this multi-dimensional array?
Hello @domainsupport,
I originally closed the ticket because within Core at this time, injecting fonts is doing it wrong:
Injecting fonts is doing it wrong at this time, as the stopgap does not support nor was it designed for any fonts other than those in the active theme's
theme.json
file.
I'm closing this ticket as
wontfix
since the injecting fonts is not supported nor intended.
The direction of fonts has changed since last we spoke. Give me a moment to share.
Planned in 6.4 is a new font management system which will include:
- Font Library: the UI/UX for managing fonts including previewing, uploading, activating, and deactivating. Google Fonts (served locally in
wp-content/uploads/fonts/
) is bundled into it. - Font Face: the server-side generator and printer of the
@font-face
styles for the fonts the user activates in the Font Library.
This new system is being developed and tested in the Gutenberg repo. I invite you to follow along and contribute:
- Font Library - Stage 1: Google and Local fonts
- Font Library - Stage 2: Fonts Library extensions which will be an API to allow plugins / themes to add font collections for user consideration within the Font Library.
Patch for /wp-includes/script-loader.php