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.

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

Backport r57868 from WordPress-Develop. #60141

Merged
merged 2 commits into from Apr 2, 2024
Merged

Conversation

peterwilsoncc
Copy link
Contributor

@peterwilsoncc peterwilsoncc commented Mar 22, 2024

What?

Backports r57868 / WordPress/wordpress-develop@ca8d78e to the plugin's 6.5 compatibility shims.

Fixes #60140.

The WordPress-Develop commit message is:

Editor: Prevent font folder naive filtering causing infinite loops.

This modifies the font directory API to more closely reflect the upload directory API to help account for naive filtering when uploading fonts.

This moves the protection of infinite loops to the new function `_wp_filter_font_directory()` to allow developers extending and maintaining the font library to apply the filter without the need for a closure.

These changes also ensure both the `upload_dir` and `font_dir` filter are applied consistently when both creating and deleting fonts faces. Prior to this commit the `upload_dir` filter was only fired when creating fonts faces via the REST API.

Applying the font directory filter to the `upload_dir` filter is now done by adding the `_wp_filter_font_directory` function rather than `wp_get_font_dir()`. Developers who have previously modified the font upload directory using the `font_dir` filter will NOT need to upload their code.

Extenders wishing to upload files to the font directory can do so via the code:

{{{#!php
<?php
add_filter( 'upload_dir', '_wp_filter_font_directory' );
// Your code to upload or sideload a font file.
remove_filter( 'upload_dir', '_wp_filter_font_directory' );
}}}

Introduces:

* `wp_font_dir()`: Attempt to create and retrieve the font upload directory. The equivalent to `wp_upload_dir()`.
* `_wp_filter_font_directory()`: To run on the `upload_dir` filter, this sets the default destination of the fonts directory and fires the `font_dir` filter. 

`wp_get_font_dir()` has been modified to be a lightweight getter for the font directory. It returns the location without attempting to create it. The equivalent to `wp_get_upload_dir()`.

Follow up to [57740].

Props peterwilsoncc, mukesh27, mikachan, costdev, mmaattiiaass, swissspidy, youknowriad, dd32, grantmkin.
Fixes https://core.trac.wordpress.org/ticket/60652.

Why?

Consistency with WordPress Core in future version of the plugins.

How?

Copy paste mainly, please review the changes to make sure I haven't messed anything up

Testing Instructions

  1. Compare the copy-paste to make sure I haven't missed anything.
  2. Install WordPress 6.4.x
  3. Activate Gutenberg
  4. Ensure it doesn't trigger a fatal error
  5. Ensure uploading a font from your computer works
  6. Ensure installing a font from Google works
  7. Install WordPress 6.5 (trunk)
  8. Ensure it doesn't trigger a fatal error
  9. Ensure uploading a font from your computer works
  10. Ensure installing a font from Google works
  11. Install the Fonts To Uploads plugin
  12. Install another font via Google
  13. Ensure the filter continues to work as expected and fonts are uploaded to the new destination

Testing Instructions for Keyboard

N/A

Screenshots or screencast

N/A

@peterwilsoncc peterwilsoncc added the Backport from WordPress Core Pull request that needs to be backported to the a Gutenberg release from WordPress Core label Mar 22, 2024
Copy link

github-actions bot commented Mar 22, 2024

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: peterwilsoncc <peterwilsoncc@git.wordpress.org>
Co-authored-by: creativecoder <grantmkin@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@creativecoder creativecoder left a comment

Choose a reason for hiding this comment

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

  • ✅ Manually verified that changes match wordpress-develop commit
  • ✅ Tested installing and using fonts
  • ✅ Testing filtering the fonts directory using the font_dir filter

@peterwilsoncc
Copy link
Contributor Author

@creativecoder Thanks Grant, can you see why the pull request automation action is failing. It consistently shows "Error: main: Task assignFixedIssues failed with error: HttpError: Not Found" but I am unsure what that is trying to do.

@creativecoder
Copy link
Contributor

@peterwilsoncc There was this line in the PR description at the end of the commit message from the original trac commit

Fixes #60652.

I think the automation was trying to find that Gutenberg issue (which doesn't exist, because it's a trac ticket number) and assign it to you. I changed it to "Fixes https://core.trac.wordpress.org/ticket/60652.", but needed to update the branch to get the job to pick up on the PR description edit. Hopefully should be good to go now 🤞🏼 .

@peterwilsoncc peterwilsoncc merged commit 24e550f into trunk Apr 2, 2024
56 checks passed
@peterwilsoncc peterwilsoncc deleted the do/wp-dev-r57868-backport branch April 2, 2024 05:32
@github-actions github-actions bot added this to the Gutenberg 18.1 milestone Apr 2, 2024
cbravobernal pushed a commit to garridinsi/gutenberg that referenced this pull request Apr 9, 2024
Prevent font folder naive filtering causing infinite loops.

This modifies the font directory API to more closely reflect the upload directory API to help account for naive filtering when uploading fonts.

This moves the protection of infinite loops to the new function `_wp_filter_font_directory()` to allow developers extending and maintaining the font library to apply the filter without the need for a closure.

These changes also ensure both the `upload_dir` and `font_dir` filter are applied consistently when both creating and deleting fonts faces. Prior to this commit the `upload_dir` filter was only fired when creating fonts faces via the REST API.

Applying the font directory filter to the `upload_dir` filter is now done by adding the `_wp_filter_font_directory` function rather than `wp_get_font_dir()`. Developers who have previously modified the font upload directory using the `font_dir` filter will NOT need to upload their code.

Co-authored-by: Grant Kinney <hi@grant.mk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport from WordPress Core Pull request that needs to be backported to the a Gutenberg release from WordPress Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Backport wp-dev commit r57868 to the plugin's 6.5 compat folder.
2 participants