Font Faces endpoint: prevent creating font faces with duplicate settings #57903

creativecoder · 2024-01-17T03:50:19Z

What?

Return an error when trying to create a Font Face with settings that match an existing one.

According to the css spec, these properties are used to match a @font-face declaration when loading fonts, and so are used here to determine duplication: fontFamily, fontStyle, fontWeight, fontStretch, and unicodeRange.

Why?

Part of #55278

How?

When a new wp_font_face is created, a slug is generated from the font face settings and stored in the post_title field, which is then used to check for duplicates.

Testing Instructions

Create a font family using the POST wp/v2/font-families/<id>/font-faces
Try to create another font family with the same settings, including fontFamily, fontStyle, fontWeight, fontStretch, and unicodeRange, you should get an error.

github-actions · 2024-01-17T03:51:37Z

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! ❤️

View changed files

❔ phpunit/tests/fonts/font-library/wpFontFamilyUtils/getFontFaceSlug.php
❔ lib/experimental/fonts/font-library/class-wp-font-family-utils.php
❔ lib/experimental/fonts/font-library/class-wp-rest-font-faces-controller.php
❔ phpunit/tests/fonts/font-library/wpFontFamilyUtils/formatFontFamily.php
❔ phpunit/tests/fonts/font-library/wpRestFontFacesController.php
❔ phpunit/tests/fonts/font-library/wpRestFontFamiliesController.php

mikachan

Test report:

Created a font face using the POST wp/v2/font-families/<id>/font-faces
Tried to create another font face with the same settings, including fontFamily, fontStyle, fontWeight, fontStretch, and unicodeRange, and got this error: A font face matching those settings already exists.

Works for me, and I like that we're using the CSS spec to inform how we match font faces ✅

mikachan

Actually, it looks like this test failure is relevant: https://github.com/WordPress/gutenberg/actions/runs/7550796701/job/20557005264?pr=57903

Looks like we're seeing double quotes where we should see single quotes.

creativecoder · 2024-01-17T14:28:41Z

Actually, it looks like this test failure is relevant:

Thanks for catching that! I switched WP_Font_Family_Utils::format_font_family to use " rather than ', since " is what I see in the css standards docs and other examples, like MDN.

mikachan · 2024-01-17T14:47:43Z

Thanks for catching that! I switched WP_Font_Family_Utils::format_font_family to use " rather than ', since " is what I see in the css standards docs and other examples, like MDN.

Gotcha, sounds good!

mikachan · 2024-01-17T15:12:27Z

lib/experimental/fonts/font-library/class-wp-font-family-utils.php

+	 * Generates a slug from font face properties.
+	 *
+	 * Used for comparison with other font faces in the same family.
+	 *


Maybe we could link to the CSS spec in the comments, so it's easy for folks to find if they want to see the spec details, etc.

Great idea! I've added a @link in the function's docblock.

…ngs (#57903)

* Font Library: add wp_font_face post type and scaffold font face REST API controller (#57656) * Font Library: create font faces through the REST API (#57702) * Refactor Font Family Controller (#57785) * Font Family and Font Face REST API endpoints: better data handling and errors (#57843) * Font Families REST API endpoint: ensure unique font family slugs (#57861) * Font Library: delete child font faces and font assets when deleting parent (#57867) Co-authored-by: Sarah Norris <[email protected]> * Font Library: refactor client side install functions to work with revised API (#57844) * Add batchInstallFontFaces function and related plumbing. * Fix resolver name. * Add embedding and rebuild theme.json settings for fontFamily. * Handle responses directly, add to collection before activating. Remove unused test. * Remove getIntersectingFontFaces. * Check for existing font family before installing. * Reference src, not uploadedFile key. Co-authored-by: Matias Benedetto <[email protected]> * Check for existing font family using GET /font-families?slug=. * Filter already installed font faces (determined by matching fontWeight AND fontStyle) --------- Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Jason Crist <[email protected]> * Cleanup/font library view error handling (#57926) * Add batchInstallFontFaces function and related plumbing. * Fix resolver name. * Add embedding and rebuild theme.json settings for fontFamily. * Handle responses directly, add to collection before activating. Remove unused test. * Remove getIntersectingFontFaces. * Check for existing font family before installing. * Reference src, not uploadedFile key. Co-authored-by: Matias Benedetto <[email protected]> * Check for existing font family using GET /font-families?slug=. * Filter already installed font faces (determined by matching fontWeight AND fontStyle) * moved response processing into the resolver for fetchGetFontFamilyBySlug * Moved response processing for font family installation to the resolver * Refactored font face installation process to handle errors more cleanly * Cleanup error handling for font library view * Add i18n function to error messages * Add TODO comment for uninstall notice --------- Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Sarah Norris <[email protected]> * Fix unique key prop warning when opening modal * Add key props to FontsGrid children * Font Faces endpoint: prevent creating font faces with duplicate settings (#57903) * Font Library: Update uninstall/delete on client side (#57932) * Fix delete endpoint * Update fetchUninstallFontFamily to match new format * Update uninstallFont * Add uninstall notice back in * Tidy up comments * Re-word uninstall notices * Add spacing to error message * Refactored how font family values were processed so they would retain their id, which is now used to delete a font family without fetching data via slug * Rename uninstallFont to uninstallFontFamily * Throw uninstall errors rather than returning them --------- Co-authored-by: Jason Crist <[email protected]> * Add slug/id back to FontCollection * Change tabsFromCollections inline with Font Collections PR * Use child.key for key prop in FontsGrid * Update packages/edit-site/src/components/global-styles/font-library-modal/local-fonts.js Co-authored-by: Jonny Harris <[email protected]> * Font Library: address JS feedback in #57688 (#57961) * Wrap error messages in sprintf * Use await rather than then * Add variables for API URLs * Update packages/edit-site/src/components/global-styles/font-library-modal/resolvers.js Co-authored-by: Jeff Ong <[email protected]> --------- Co-authored-by: Jeff Ong <[email protected]> * Font Library REST API endpoints: address initial feedback from feature branch (#57946) * Font Library: font collection refactor to use the new schema (#57884) * google fonts collection data provisional url * rename controller methods * fix get_items parameters * fix endpoint return * rafactor font collection class * fix tests for the refactored class * refactor font collections rest controller * update font collection tests * update the frontend to use the new endpoint data schema * format php * adding linter line ignore rul * replacing throwing an exception by calling doing_it_wrong * add translation marks Co-authored-by: Jeff Ong <[email protected]> * user ternary operator * correct translation formatting and comments * renaming function * renaming tests * improve url matching Co-authored-by: Grant Kinney <[email protected]> * return error without rest_ensure_response * fix contradictory if condition --------- Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Grant Kinney <[email protected]> * Remove old WP_REST_Autosave_Fonts_Controller class --------- Co-authored-by: Grant Kinney <[email protected]> Co-authored-by: Grant Kinney <[email protected]> Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jonny Harris <[email protected]>

* Font Library: add wp_font_face post type and scaffold font face REST API controller (#57656) * Font Library: create font faces through the REST API (#57702) * Refactor Font Family Controller (#57785) * Font Family and Font Face REST API endpoints: better data handling and errors (#57843) * Font Families REST API endpoint: ensure unique font family slugs (#57861) * Font Library: delete child font faces and font assets when deleting parent (#57867) * Font Library: refactor client side install functions to work with revised API (#57844) * Cleanup/font library view error handling (#57926) * Font Faces endpoint: prevent creating font faces with duplicate settings (#57903) * Font Library: Update uninstall/delete on client side (#57932) * Font Library: address JS feedback in #57688 (#57961) * Font Library REST API endpoints: address initial feedback from feature branch (#57946) * Font Library: font collection refactor to use the new schema (#57884) * Fix font asset download when font faces are installed (#58021) * Font Families and Faces: disable autosaves using empty class (#58018) * Adds migration for legacy font family content (#58032) --------- Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Sarah Norris <[email protected]> Co-authored-by: Jonny Harris <[email protected]>

* Font Library: add wp_font_face post type and scaffold font face REST API controller (#57656) * Font Library: create font faces through the REST API (#57702) * Refactor Font Family Controller (#57785) * Font Family and Font Face REST API endpoints: better data handling and errors (#57843) * Font Families REST API endpoint: ensure unique font family slugs (#57861) * Font Library: delete child font faces and font assets when deleting parent (#57867) Co-authored-by: Sarah Norris <[email protected]> * Font Library: refactor client side install functions to work with revised API (#57844) * Add batchInstallFontFaces function and related plumbing. * Fix resolver name. * Add embedding and rebuild theme.json settings for fontFamily. * Handle responses directly, add to collection before activating. Remove unused test. * Remove getIntersectingFontFaces. * Check for existing font family before installing. * Reference src, not uploadedFile key. Co-authored-by: Matias Benedetto <[email protected]> * Check for existing font family using GET /font-families?slug=. * Filter already installed font faces (determined by matching fontWeight AND fontStyle) --------- Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Jason Crist <[email protected]> * Cleanup/font library view error handling (#57926) * Add batchInstallFontFaces function and related plumbing. * Fix resolver name. * Add embedding and rebuild theme.json settings for fontFamily. * Handle responses directly, add to collection before activating. Remove unused test. * Remove getIntersectingFontFaces. * Check for existing font family before installing. * Reference src, not uploadedFile key. Co-authored-by: Matias Benedetto <[email protected]> * Check for existing font family using GET /font-families?slug=. * Filter already installed font faces (determined by matching fontWeight AND fontStyle) * moved response processing into the resolver for fetchGetFontFamilyBySlug * Moved response processing for font family installation to the resolver * Refactored font face installation process to handle errors more cleanly * Cleanup error handling for font library view * Add i18n function to error messages * Add TODO comment for uninstall notice --------- Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Sarah Norris <[email protected]> * Font Faces endpoint: prevent creating font faces with duplicate settings (#57903) * Font Library: Update uninstall/delete on client side (#57932) * Fix delete endpoint * Update fetchUninstallFontFamily to match new format * Update uninstallFont * Add uninstall notice back in * Tidy up comments * Re-word uninstall notices * Add spacing to error message * Refactored how font family values were processed so they would retain their id, which is now used to delete a font family without fetching data via slug * Rename uninstallFont to uninstallFontFamily * Throw uninstall errors rather than returning them --------- Co-authored-by: Jason Crist <[email protected]> * Update packages/edit-site/src/components/global-styles/font-library-modal/local-fonts.js Co-authored-by: Jonny Harris <[email protected]> * Font Library: address JS feedback in #57688 (#57961) * Wrap error messages in sprintf * Use await rather than then * Add variables for API URLs * Update packages/edit-site/src/components/global-styles/font-library-modal/resolvers.js Co-authored-by: Jeff Ong <[email protected]> --------- Co-authored-by: Jeff Ong <[email protected]> * Add missing dep in font-demo * Move notice to top of local-fonts panel * Add container around spinner * Move notice to TabPanelLayout * Remove spacer from LocalFonts * Move notice and setNotice state to context.js * Move spacer outside of notice container * Rename LocalFonts to UploadFonts * Make notices dismissible * Reset notices onRemove * Reset notice when new tab is selected * Reset notice on each action * Fix notice re-render on fetchFontCollection * Move notices below font collection title and description --------- Co-authored-by: Grant Kinney <[email protected]> Co-authored-by: Grant Kinney <[email protected]> Co-authored-by: Jeff Ong <[email protected]> Co-authored-by: Matias Benedetto <[email protected]> Co-authored-by: Jason Crist <[email protected]> Co-authored-by: Jonny Harris <[email protected]>

creativecoder added 2 commits January 16, 2024 21:17

Font Faces endpoint: prevent creating font faces with duplicate settings

4706663

Include unicode range in font face slug

085219e

creativecoder added REST API Interaction Related to REST API [Type] Experimental Experimental feature or API. [Feature] Typography Font and typography-related issues and PRs labels Jan 17, 2024

creativecoder requested review from pbking, matiasbenedetto, mikachan and jffng January 17, 2024 03:50

creativecoder self-assigned this Jan 17, 2024

mikachan approved these changes Jan 17, 2024

View reviewed changes

mikachan reviewed Jan 17, 2024

View reviewed changes

creativecoder added 2 commits January 17, 2024 10:18

Improve slug implementation, fix tests

068c5a6

Adds tests for WP_Font_Family_Utils::get_font_face_slug

bac0df7

creativecoder marked this pull request as ready for review January 17, 2024 17:23

creativecoder requested a review from spacedmonkey as a code owner January 17, 2024 17:23

creativecoder added 3 commits January 17, 2024 11:33

Fix test race condition

99fa7ab

Fixing tests again

18f0863

Update comment

9fafdee

creativecoder merged commit 8a5efd4 into try/font-library-refactor Jan 17, 2024
55 checks passed

creativecoder deleted the update/font-face-unique-slug branch January 17, 2024 22:37

creativecoder added a commit that referenced this pull request Jan 22, 2024

Font Faces endpoint: prevent creating font faces with duplicate setti…

3e37968

…ngs (#57903)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Font Faces endpoint: prevent creating font faces with duplicate settings #57903

Font Faces endpoint: prevent creating font faces with duplicate settings #57903

creativecoder commented Jan 17, 2024

github-actions bot commented Jan 17, 2024 •

edited

Loading

mikachan left a comment

mikachan left a comment

creativecoder commented Jan 17, 2024

mikachan commented Jan 17, 2024

mikachan Jan 17, 2024

creativecoder Jan 17, 2024

Font Faces endpoint: prevent creating font faces with duplicate settings #57903

Font Faces endpoint: prevent creating font faces with duplicate settings #57903

Conversation

creativecoder commented Jan 17, 2024

What?

Why?

How?

Testing Instructions

github-actions bot commented Jan 17, 2024 • edited Loading

mikachan left a comment

Choose a reason for hiding this comment

mikachan left a comment

Choose a reason for hiding this comment

creativecoder commented Jan 17, 2024

mikachan commented Jan 17, 2024

mikachan Jan 17, 2024

Choose a reason for hiding this comment

creativecoder Jan 17, 2024

Choose a reason for hiding this comment

github-actions bot commented Jan 17, 2024 •

edited

Loading