Directory

All registered fonts being enqueued; Jetpack fonts module leads to 24,000 lines of CSS · Issue #48263 · WordPress/gutenberg · GitHub
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.

Sign up for GitHub

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

Jump to bottom

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

Closed
mreishus opened this issue Feb 20, 2023 · 6 comments
Closed

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

mreishus opened this issue Feb 20, 2023 · 6 comments
Labels
[Type] Bug An existing feature does not function as intended [Type] Performance Related to performance efforts

Comments

@mreishus
Copy link
Contributor

mreishus commented Feb 20, 2023 •
edited

Description

When Jetpack registers fonts to make them generally available, Gutenberg is enqueuing them even if they aren't used, causing unneeded font css to render to the front-end of sites.

End result: There is no distinction between fonts that are only registered, and fonts that are registered and enqueued. All registered fonts are enqueued even if not used, causing extra font CSS to be rendered on the front-end.

I believe this bit of code is related:

			// BEGIN OF EXPERIMENTAL CODE. Not to backport to core.
			static::$theme = gutenberg_add_registered_fonts_to_theme_json( static::$theme );
			// END OF EXPERIMENTAL CODE.

On WPCOM, I see the bug begin to happen when WPCOM started to use Gutenberg 15.1.1.

Step-by-step reproduction instructions

Gutenberg + wp-env only

This can be reproduced by using this test plugin which imitates Jetpack's Google Fonts module. Tested using commit a9db335 2023-02-21 19:16 +0100.

## Resetting Gutenberg Repo + Destroying wp-env
wp-env destroy
git reset --hard
git pull
npm run build

## Setting up example Jetpack fonts plugin
mkdir /tmp/webfonts-jetpack-test
git clone git@github.com:mreishus/webfonts-jetpack-test.git /tmp/webfonts-jetpack-test
cd /tmp/webfonts-jetpack-test
composer install
cd -

## Adding the new plugin to be loaded and starting wp-env
sed -i '3s/.*/       "plugins": [ ".", "\/tmp\/webfonts-jetpack-test" ],/' .wp-env.json
wp-env start

The .wp-env.json file should now point to the new plugin:

diff --git a/.wp-env.json b/.wp-env.json
index aa8dfaf3c0..788e754f70 100644
--- a/.wp-env.json
+++ b/.wp-env.json
@@ -1,6 +1,6 @@
 {
        "core": "WordPress/WordPress",
-       "plugins": [ "." ],
+       "plugins": [ ".", "/tmp/webfonts-jetpack-test" ],
        "themes": [ "./test/emptytheme" ],
        "env": {
                "tests": {

If all went well, the wp-admin plugins page should show the new plugin:

2023-02-21_17-17

Now visit http://localhost:8888/ and view source.

  • Actually see: There will be over 20,000 lines of font CSS added.
  • Expected to see: Only fonts used will be enqueued on the front-end, not all registered fonts.

WPCOM + Jetpack + Gutenberg

  • Visit https://wordpress.com/start/ and create a new free site
  • Skip past as many of the signup prompts as possible
  • Change the site's theme to twentytwentytwo
  • Visit the frontend of the site and view source. (Example: https://testsitemmrfont1.wordpress.com/ )
  • See: 24,000 lines of font css

Screenshots, screen recording, code snippet

2023-02-21_17-10

Environment info

wp-env off commit a9db335 2023-02-21 19:16 +0100

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

No
@kathrynwp kathrynwp added [Type] Enhancement A suggestion for improvement. [Type] Performance Related to performance efforts labels Feb 21, 2023
@mreishus mreishus changed the title All registered fonts being enqueued in some circumstances All registered fonts being enqueued; Jetpack fonts module leads to 24,000 lines of CSS Feb 21, 2023
@mreishus
Copy link
Contributor Author

I've added instructions to reproduce the issue with only gutenberg, wp-env, and a test plugin.

@mreishus mreishus mentioned this issue Feb 21, 2023
Merged
@hellofromtonya hellofromtonya added [Type] Bug An existing feature does not function as intended [Feature] Webfonts and removed [Type] Enhancement A suggestion for improvement. labels Feb 22, 2023
@hellofromtonya
Copy link
Contributor

This issue appears to be from #47290. cc @aristath @pbking

@ironprogrammer
Copy link
Contributor

ironprogrammer commented Feb 22, 2023 •
edited

Reproduction Report

This is a confirmation of this issue using a local nginx install.

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 12.6.3
  • Browser: Safari 16.3
  • Server: nginx/1.23.3
  • PHP: 7.4.33
  • WordPress: 6.2-beta3-55400-src trunk
  • Theme: twentytwentythree v1.0
  • Active Plugins:
    • gutenberg v15.2.0 trunk
    • webfonts-jetpack-test v1.0.0-mmr (provided here)

Actual Results

  • ✅ With the test plugin active, but no custom fonts used in global styles or per-post, all fonts from the plugin are emitted on FE of site (the <style id="wp-fonts-jetpack-google-fonts"> tag is 23,973 lines long).
  • ✅ When an individual custom font is assigned to global styles or per-post, all fonts from the plugin are emitted on FE.

Additional Information

The test plugin also triggers deprecation notices for wp_webfonts and wp_enqueue_webfont.

@ironprogrammer ironprogrammer mentioned this issue Feb 23, 2023
Merged
@ironprogrammer
Copy link
Contributor

I've created #48341 to address this issue. It's getting late 😪, so if I've missed something, LMK and I can check it in the morning.

@hellofromtonya
Copy link
Contributor

Thank you @ironprogrammer for investigating 🥇 PR #48341 does indeed resolve the issue.

This issue appears to be from #47290. cc @aristath @pbking

Root cause follow-up:
PR #47290 is not the root cause of the issue.

The root cause is missing that the original (now deprecated) origin also needed to be considered in gutenberg_register_fonts_from_theme_json() as part of the theme.json / global styles font orchestration logic. When the API was renamed, the origin from the deprecated logic also needed consideration.

Why? To ensure plugins using the deprecated API's functionality are also skipped in gutenberg_register_fonts_from_theme_json().

@hellofromtonya
Copy link
Contributor

#48341 fixes this issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Type] Bug An existing feature does not function as intended [Type] Performance Related to performance efforts
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ironprogrammer @mreishus @hellofromtonya @kathrynwp