-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Font Library: Font Family post type retained when unable to install any font faces #59486
Comments
I can't replicate this on Gutenberg trunk, although I believe it should have been fixed by #59451, as that PR checks if a new font family has been added and will uninstall/remove it if no fonts are successfully uploaded for the font family. Can you see this issue with Gutenberg trunk? |
...worth noting that this will be synced to WP Core as part of WP 6.5 RC 1 which is scheduled for tomorrow. |
It looks like that's also going to be included with WP 6.5 RC1. |
Awesome. Removing this from 6.5 :) |
Verified: if all fonts fail to upload the font family is not available. However, I'm still seeing the Normal font as available if there are childless font family posts in the database
Screen shot, note that the system font is underlined in Firefox dev tools indicating the active font. Should I log that as a separate issue or do you want to work on a fix off this one? |
With #59436 we are ensuring that users using the font library UI don't end up with font families with no font faces when that was not the intention. Using a low-level method, as the one in your example, we can not assume the user's intention, and I don't think we should remove the font family parent of the deleted font faces. If the user is using those low-level commands to manipulate the fonts installed, it's better to let them remove the font family manually using a similar command. |
This is what I am saying, the font family still appears in the UI if there are no child font faces. This is why the dev tools are showing the default sans-serif font is in use in the screen shot The use of a WP CLI command in the reproduction steps is a developer convenience for the purpose of reproduction. As the font-face REST API endpoint doesn't remove childless font-families, the same can occur removing font-faces using the recommended technique. |
A font family without a font face is still a legit font family, it would just be a system font. The one in question isn't helpful, sure, but not having font faces associated with it wouldn't be a reason to exclude it from API responses or to delete it. It's tricky and the resolution is unclear. |
The video from the issue's description is no longer accurate regarding the behavior of the font library after #59436 was merged. However, if you installed a font family and all the font faces failed to install, it will still be listed in the font library if it was installed in before #59436 was merged. This is because a font family without font faces installed (system fonts) are valid font families, so they should be listed. After #59436 was merged is all the font faces failed to be installed, the font family will be deleted right after it was created, and the font faces creation failed. Here's a screencast following the steps detailed in this issue's description, where you can see that: 2024-03-06.10-49-58.mp4The example from this comment #59486 (comment) doesn't seem to be valid because the low-level data manipulation where the extender's intention should not be assumed, so the font family should not be removed and it should be listed because font families without font faces associated are valid fonts (system font stacks). In my opinion, this issue should be closed as already fixed. |
As I said, the low level manipulation was a developer convenience for reproducing. It allows me to avoid instructions requiring an Application password, etc. When deleting font-faces, it's possible to determine whether the font-familiy is a web-font or a local font from two sources of data:
If these items on the font-face are present then keeping around the font-family is to show fonts in the typography sidebar that are unavailable to the user. Instead of getting the expected font they get the system default font. If the library is showing a preview of the correctly formated font via the SVG then it's more confusing for the user. |
@peterwilsoncc I'm not sure what the issue is currently.The issue's original video is outdated. Could you please add a new screencast featuring what you consider the issue to be? |
I've read back through this discussion to try and get a handle on what the point of concern is. My understanding is as follows:
Argument against this being a bug
Argument in support of this being a valid bug
I'm going to continue investigating this. In the meantime I would welcome input from Editor release leads as to whether they would consider this to be a bug. cc @fabiankaegy @annezazu @youknowriad. |
Coming back to this, I noticed the font families are listed in the typography sidebar even when the font-family post type is removed:
|
Long form UI steps
This should apply to the earlier notes too. I think what will probably need to happen is this:
|
I've opened #59974 for above comments as it seems to be an interaction between global styles and the font library. I still think there's room for improvement on this ticket though and safe to conclude the font-family post type can be removed once all the child font-face post types are removed. I tested using System fonts with the font library using matiasbenedetto/modern-fonts-stacks-for-wp-font-library and system fonts only create a font-family post type, not any font-face post types. If the font face exists and the |
Description
When attempting to upload or install fonts via the Font Manager, the font family post type is retained even if there are no successful font faces uploaded.
This results in a Font Family listing with no variants. However, once can open the font family and by default the normal font weight will be displayed allowing the user to activate it and select it in the side bar.
If no fonts are successfully uploaded, the font-family post ought to be removed.
Step-by-step reproduction instructions
WP_REST_Font_Faces_Controller::handle_font_file_upload()
to simply return aWP_Error
objectScreenshots, screen recording, code snippet
Environment info
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: