Skip redundant registration for Fonts API-registered fonts #48341

ironprogrammer · 2023-02-23T06:02:27Z

What?

This update synchronizes the origin name checked for fonts registered through the Fonts API. Those fonts are already registered, so can be skipped here.

Why?

Addresses #48263, where all fonts registered through a plugin are rendered on the front end, regardless of whether they are in use by global, template, or per-page style settings.

How?

Updates the origin name check to gutenberg_wp_fonts_api, to match where it's being set originally.

Testing Instructions

If you have previously set custom fonts for global styles, templates/patterns through the editor, or on a per-post basis, please reset the typography to their defaults for the active template, otherwise test results may vary.

On your test site, activate the TT3 theme, as well as the Gutenberg plugin (from this branch).
Install and activate this test plugin.
View the site frontend, and view source or inspect the elements/markup.
👀 Near the end of the <head> element, there should be a <style id="wp-fonts-local"> tag, but there should NOT be a <style id="wp-fonts-jetpack-google-fonts"> tag.
In WP admin, create a new post, and add a heading.
Open the Settings sidebar, and under Typography enable Font Family. The added Font dropdown should list numerous fonts registered by the test plugin.
Set the heading's Font to one of the non-default font options (e.g. Arvo), and Publish or Update the post.
View the post on the site frontend, and view source or inspect the elements/markup.
👀 Near the end of the <head> element, there should now be a <style id="wp-fonts-jetpack-google-fonts"> tag, and the @font-face declarations should match the custom font selected above. For instance, for Arvo there should be 4 declarations.
In WP admin, navigate to Appearance > Editor, and click anywhere on the right side to go into edit mode.
Open the Styles panel, and select Typography.
Under Elements select Headings, and set the Font to something other than a default TT3 font or the one selected above (e.g. Space Mono), then Save the custom styles.
View the previous post on the site frontend, and view source or inspect the elements/markup.
👀 The <style id="wp-fonts-jetpack-google-fonts"> tag should include @font-face declarations for both the custom font selected on the post, as well as the font set in the global styles above. For instance, for Arvo there should be 4 declarations, and Space Mono should have 12.

Expected Results

✅ With the test plugin activated, but no custom fonts assigned, unassigned fonts should not be output on the site frontend (i.e. no <style id="wp-fonts-jetpack-google-fonts"> tag).
✅ With a custom font(s) assigned in a post, the @font-face(s) should render on the site frontend in the <style id="wp-fonts-jetpack-google-fonts"> tag.
✅ With a custom font(s) assigned globally, the @font-face(s) should render on the site frontend's <style id="wp-fonts-jetpack-google-fonts"> tag, in addition to other custom fonts per template or page.

Please note that logged deprecation notices for wp_webfonts and wp_enqueue_webfont are expected.

Testing Instructions for Keyboard

n/a

Screenshots or screencast

n/a

Additional Note

The test plugin provided for #48263 includes registration of fonts already present in TT3, and results in those fonts being doubly-rendered in the front end (first in #wp-fonts-local, and then #wp-fonts-jetpack-google-fonts). The updated test plugin for this PR mitigates that issue by removing those fonts, as that bug should be tracked down separately.

When fonts are converted to theme.json format, origin is set to `gutenberg_wp_fonts_api`.

lib/experimental/fonts-api/register-fonts-from-theme-json.php

hellofromtonya · 2023-02-23T14:01:50Z

lib/experimental/fonts-api/deprecations/class-wp-web-fonts.php

@@ -690,7 +690,7 @@ public function to_theme_json( $font_family_handle ) {
 			}

 			$variation_obj        = $this->registered[ $variation_handle ];
-			$variation_properties = array( 'origin' => 'gutenberg_wp_webfonts_api' );
+			$variation_properties = array( 'origin' => 'gutenberg_wp_fonts_api' );


Is there risk of this impacting extenders who may be iterating through the registered fonts?

A search of the wporg plugin directory shows only Gutenberg using it https://wpdirectory.net/search/01GSZ95M6F3772YGWKZXG9A15P.

I suspect the risk to users is minimally low.

If anything, this feature is in the experimental folder so external developers should be aware that things might break 😅

Is it worth it to include a doing_it_wrong call to announce that things changed, @hellofromtonya?

If anything, this feature is in the experimental folder so external developers should be aware that things might break 😅
True, though the API now includes a temporary BC Layer which is intended to keep sites running while giving time for extenders to upgrade to the new API.

Is it worth it to include a doing_it_wrong call to announce that things changed, @hellofromtonya?

Good question @zaguiini! The origin is set internally within the API itself. So I think it's okay to not add a doing_it_wrong here. But it does me think about: could it be set externally? If yes and set wrong, what happens? Hmm that's interesting as it may cause unexpected things. Thinking a new issue is needed to discuss and explore.

hellofromtonya

tl;dr
This PR resolves the issue and is good for merge 👍

Reasoning:

Before applying the PR, can reproduce the issue ✅
After applying the PR, resolves the issue ✅
The changes to the BC Layer have minimal risk of breakages: the 'origin' is an internal flag and there are no instances of its usage in wporg plugin directory.

Needs tests to avoid this kind of regression, but thinking these tests can be added separately as more thought is needed for the coverage paths.

lib/experimental/fonts-api/register-fonts-from-theme-json.php

pbking · 2023-02-23T16:07:19Z

I can confirm the change fixes the problem and doesn't negatively impact Jetpack's or Blockbase's font stuff.

hellofromtonya · 2023-02-23T16:12:04Z

@DaisyOlsen this patch is needed in 15.2.1 release (see #48263).

* Update font origin to match updated API When fonts are converted to theme.json format, origin is set to `gutenberg_wp_fonts_api`. * Update font origin set in deprecated WP_Web_Fonts class --------- Co-authored-by: Tonya Mork <tonya.mork@automattic.com>

ironprogrammer added 2 commits February 22, 2023 17:50

Update font origin to match updated API

a7ea1ab

When fonts are converted to theme.json format, origin is set to `gutenberg_wp_fonts_api`.

Update font origin set in deprecated WP_Web_Fonts class

9f11341

ironprogrammer requested a review from spacedmonkey as a code owner February 23, 2023 06:02

ironprogrammer mentioned this pull request Feb 23, 2023

All registered fonts being enqueued; Jetpack fonts module leads to 24,000 lines of CSS #48263

Closed

ironprogrammer changed the title ~~Skip redundant font registration for Fonts API-registered fonts~~ Skip redundant registration for Fonts API-registered fonts Feb 23, 2023

hellofromtonya self-requested a review February 23, 2023 13:13

hellofromtonya reviewed Feb 23, 2023

View reviewed changes

lib/experimental/fonts-api/register-fonts-from-theme-json.php Outdated Show resolved Hide resolved

hellofromtonya reviewed Feb 23, 2023

View reviewed changes

hellofromtonya approved these changes Feb 23, 2023

View reviewed changes

hellofromtonya added [Type] Bug An existing feature does not function as intended Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release labels Feb 23, 2023

hellofromtonya added this to the Gutenberg 15.2 milestone Feb 23, 2023

Change webfont to font in inline comment.

5948b05

hellofromtonya reviewed Feb 23, 2023

View reviewed changes

lib/experimental/fonts-api/register-fonts-from-theme-json.php Show resolved Hide resolved

hellofromtonya self-requested a review February 23, 2023 15:44

hellofromtonya merged commit 1a93475 into WordPress:trunk Feb 23, 2023

DaisyOlsen removed the Backport to Gutenberg Minor Release Pull request that needs to be backported to a Gutenberg minor release label Feb 27, 2023

hellofromtonya added the [Feature] Fonts API label Apr 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Skip redundant registration for Fonts API-registered fonts #48341

Skip redundant registration for Fonts API-registered fonts #48341

ironprogrammer commented Feb 23, 2023 •

edited

Loading

hellofromtonya Feb 23, 2023

zaguiini Feb 23, 2023

hellofromtonya Feb 23, 2023

hellofromtonya left a comment

pbking commented Feb 23, 2023

hellofromtonya commented Feb 23, 2023

Skip redundant registration for Fonts API-registered fonts #48341

Skip redundant registration for Fonts API-registered fonts #48341

Conversation

ironprogrammer commented Feb 23, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Expected Results

Testing Instructions for Keyboard

Screenshots or screencast

Additional Note

hellofromtonya Feb 23, 2023

Choose a reason for hiding this comment

zaguiini Feb 23, 2023

Choose a reason for hiding this comment

hellofromtonya Feb 23, 2023

Choose a reason for hiding this comment

hellofromtonya left a comment

Choose a reason for hiding this comment

pbking commented Feb 23, 2023

hellofromtonya commented Feb 23, 2023

ironprogrammer commented Feb 23, 2023 •

edited

Loading