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

wp_font_face post type is a near duplication of the attachment post type. #59387

Closed
peterwilsoncc opened this issue Feb 26, 2024 · 6 comments
Closed
Labels
[Feature] Font Library Needs Decision Needs a decision to be actionable or relevant [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended

Comments

@peterwilsoncc
Copy link
Contributor

Description

The Font Library feature introduces two new post types: wp_font_family for the general font name (eg, Open Sans, Noto Sans, etc) and wp_font_face for the storage in the individual font files (eg, Open Sans Regular, Bold, etc).

The font-face typically includes:

  • an uploaded file (either from the file system or uploaded from google fonts)
  • meta data containing the file path
  • post content data containing URLs and font data

Each of these components is either directly or computationally available via the existing attachment post type.

Adding a near duplicate post type causes a number of issues (some of which have been hit already):

  1. upload permissions need to be reimplemented
  2. third party S3 and other bucket offloaders need to be updated
  3. export/import of attached files needs to be reimplemented
  4. duplicate actions and filters need to be created

The duplication of hooks is of major concern to me as core contributors will need to rapidly iterate 20 years of allowances for theme and plugin developers to allow them to customize file systems for attachments.

While there are plans to allow font files to be hosted on CDNs, I think the fundamental issues with near duplication of existing code remains and will continue in to the future.

I think the use of a new post type needs to be reconsidered due to the maintenance burdon it will create for contributors: both in the near term playing catch up with hooks and the future as changes need to be duplicated for each post type.

Step-by-step reproduction instructions

N/A

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

@peterwilsoncc peterwilsoncc added [Type] Bug An existing feature does not function as intended [Feature] Font Library labels Feb 26, 2024
@youknowriad
Copy link
Contributor

I believe this has been considered initially and the decision was made to not go with attachments. Maybe @matiasbenedetto recall the context here.

By looking at the code, here are some of the main differences:

I agree that ultimately "attachments" can be seen as a file abstraction but the differences above seem to be meaningful. It feels like if the decision in #55280 is to stick with. wp-content/fonts then it's actually logical to have a dedicated CPT for font faces.

@hellofromtonya
Copy link
Contributor

I'd like to share a discussion @matiasbenedetto @youknowriad and I had last June.

Riad asked whether "attachments" fit into the Font Library. I didn't see font files or the Font Library as a use-case for these reasons:

Attachments are a special post type that holds information about a file uploaded through the WordPress media upload system, such as its description and name, which can display through several post type – attachment template files.

Media consists of the images, video, recordings, and files that you upload and use in your blog. Media is typically uploaded and inserted into the content when writing a Post or writing a Page.
* Each font is not attached to a "post" but rather entities within a post, such as the template or a block or even globally.

@hellofromtonya
Copy link
Contributor

hellofromtonya commented Feb 27, 2024

I agree that ultimately "attachments" can be seen as a file abstraction but the differences above seem to be meaningful.

Yes, I agree with @youknowriad: there is an argument to be made for "attachments..as a file abstraction". However, as Riad and I both shared, there are distinct differences between the font files and media attachments, as well as Media Library and Font Library.

Fast-forward to today's design. A near duplication is cause for pause (rhyme unintended 😉 ). It suggests attachments could have been potentially used as the base abstraction and then the distinct font needs built on top of it.

I'm wondering about these bigger questions as we consider this discussion and what should happen next:

  • Do font files fit well into attachments and the attachment template hierarchy?
  • Should font files be distinct from media, i.e. like the Font Library is distinct from the Media Library? Or should they be considered another type of media?
  • Is there any impact to the Media Library to bundle font files within media?
  • Do the risks and concerns raised outweigh the distinct needs/reasons for not using it?

Please note, I fully support @peterwilsoncc raising these concerns, support this discussion, and support design decisions being revisited. This feature has not yet shipped. We can't fast travel back to last summer to reconsider its architectural needs and design. Rather, the best we can do today is to revisit and discuss to determine if it is the right design for the feature, contributors, extenders, and the long-term (especially within the reality of big picture burden).

@creativecoder
Copy link
Contributor

Some differences between attachment and wp_font_face that are worth noting, as they would need to be reconciled if attachments were used as a base for font files in some way:

  • The src property of wp_font_face can have multiple font files that are different formats of the same font (woff, woff2, ttf, and otf), whereas attachments have one image file each + generated previews (afiak).
  • A wp_font_face post can have a combination of properties that make it unique: there should not be more than one font face with the same font family, font weight, font style, font stretch, and unicode range, as browsers would treat them as duplicates.
  • The core of wp_font_face is a JSON blob that is the theme.json definition for the font face. A url to one or more font files is part of that definition, but the file can be hosted at any url, by default.

@hellofromtonya hellofromtonya added [Priority] High Used to indicate top priority items that need quick attention Needs Decision Needs a decision to be actionable or relevant labels Feb 27, 2024
@aaronjorbin
Copy link
Member

I think @youknowriad's analysis is spot on. Attachments and Fonts feel different enough that reusing the same post type feels like a mistake. To me, this more signifies we need better ways to have shared commonalities between post types. Perhaps we should explore (in a separate issue) abstract post types or something like PHP traits for post types?

@peterwilsoncc
Copy link
Contributor Author

Thanks for chiming in folks.

I agree, it appears there are enough differences to justify the near duplicate. @creativecoder raises a great point about multiple source files that I hadn’t considered.

I’m happy if this is closed as unplanned

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Font Library Needs Decision Needs a decision to be actionable or relevant [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
Status: Done
Development

No branches or pull requests

5 participants