Font Library: Merge the font faces instead of overriding when parsing settings
What?
This PR is to combine the font faces provided by ALL origins instead of overriding.
Why?
Referring to https://github.com/WordPress/gutenberg/issues/58764#issuecomment-1947778291, the installed font variants override the font variants provided by the theme. For example, if a theme provides a Cardo font with 400, and 500 font variants and people install a new Cardo font with a 600 font variant, then only the Cardo font with a 600 font variant will take effect. As a result, we have to merge the font faces across different origins to prevent this issue.
How?
Instead of overriding, now we check whether the new font face exists or not. If it exists, we will merge the new one into the existing font face. If not, we just add it to the array.
Testing Instructions
- Make this change take effect by commenting out the core files
- Run
wp-env run cli bash - Open the
wp-settings.php - Comment out the following lines
// require ABSPATH . WPINC . '/fonts/class-wp-font-face-resolver.php'; // require ABSPATH . WPINC . '/fonts/class-wp-font-face.php'; // require ABSPATH . WPINC . '/fonts.php';
- Run
- Open a site with TT4
- Go to the Editor
- Create the following paragraph:
- Cardo Normal. Set the Font Family to Cardo, and the Appearance to “Regular”
- Cardo Bold. Set the Font Family to Cardo, and the Appearance to “Bold”
- Cardo Normal Italic. Set the Font Family to Cardo, and the Appearance to “Regular Italic”
- Install the Cardo font with 400 font variant from the Install Fonts tab of the Font Library Modal
- Save changes and refresh the page
- Make sure the rendered font of those paragraphs is Cardo-Regular
Testing Instructions for Keyboard
Screenshots or screencast
Here is the video to reproduce the issue:
https://github.com/WordPress/gutenberg/assets/13596067/11114225-a017-407c-b75b-874ce78257cc
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.
If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
Co-authored-by: arthur791004 <[email protected]>
Co-authored-by: okmttdhr <[email protected]>
Co-authored-by: matiasbenedetto <[email protected]>
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.
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! :heart:
View changed files
:grey_question: lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php
Apart from that, I think it would be useful to implement these changes in the WordPress core repo first to be able to run the unit tests for this functionality.
There's an alternative approach implementing what i mentioned here: https://github.com/WordPress/gutenberg/pull/59119#pullrequestreview-1896329058 I'd appreciate if you can take a look.
Reviewed https://github.com/WordPress/wordpress-develop/pull/6161#pullrequestreview-1897356295 and it looks good 🙂
Do we still need this PR? I'm unsure whether we have a mechanism to sync changes from Core.
Do we still need this PR?
Nope no longer needed.
I'm unsure whether we have a mechanism to sync changes from Core.
In this case, we need to manually copy these changes from the core to the Gutenberg repo when this is merged.
Okay, thanks!
Closing it in favor of https://github.com/WordPress/wordpress-develop/pull/6161 and https://github.com/WordPress/gutenberg/pull/59321