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

[Fonts API] All registered fonts are being rendered outside of the Site Editor #49645

Closed
hellofromtonya opened this issue Apr 6, 2023 · 4 comments · Fixed by #49646
Closed
Assignees
Labels
[Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Bug An existing feature does not function as intended

Comments

@hellofromtonya
Copy link
Contributor

hellofromtonya commented Apr 6, 2023

Description

In lib/compat/wordpress-6.2/gutenberg_resolve_assets_override(), all of the registered fonts are being rendered when not in the Site Editor. This can cause a performance issue when a plugin(s) registers additional fonts for user selection.

The way it's supposed to work is this:

  • When in the Site Editor, all registered fonts are inserted into the iframe, allowing users to preview typography selections in the Styles UI for possible selection.
  • When not in the Site Editor, only enqueued fonts should be inserted into the iframe. These enqueued fonts are all of the theme defined fonts and any user-selected fonts from plugins.

Step-by-step reproduction instructions

  1. On your test site, activate the TT3 theme and Gutenberg plugin.
  2. Install and activate this test plugin.
  3. Open a post by going to Posts > and selecting or adding a post.
  4. In your browser, open Dev Tools to the Inspector tab.
  5. Search for wp-fonts-jetpack-google-fonts.

The expected behavior:

  • <style id="wp-fonts-jetpack-google-fonts"> should not exist.
  • <style id="wp-fonts-local"> should exist (these are the theme's defined fonts).

Screenshots, screen recording, code snippet

With no user selected fonts, <style id="wp-fonts-jetpack-google-fonts"> should not exist in a Post's iframed editor HTML.

Notice in the following screenshot, it does exist in the Post's iframed editor HTML 🐞 ❌
pr50558-before

Environment info

  • WordPress: 6.2
  • Gutenberg: 15.5 and trunk

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

@hellofromtonya hellofromtonya self-assigned this Apr 6, 2023
@hellofromtonya hellofromtonya added [Type] Bug An existing feature does not function as intended [Feature] Webfonts labels Apr 6, 2023
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 6, 2023
@hellofromtonya
Copy link
Contributor Author

Reopening as #49646 was reverted for now, i.e. until other dependencies are committed.

@hellofromtonya hellofromtonya added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label Apr 27, 2023
@hellofromtonya
Copy link
Contributor Author

I've marked this issue as blocked. It's currently blocked by #40363. Why?

See this explanation: #49646 (comment)

It needs to be bundled with the #40363 feature enhancement which (when implemented) will automatically enqueue all user selected fonts, selected from the Site Editor's Global Typography UI. Why? One of the most popular Google Fonts implementation has a bug in it where its fonts will not enqueue. The bug was hidden until this PR fixed the Fonts API for the iframe assets. Delaying the shipment of this PR's fix avoids breaking sites.

@hellofromtonya hellofromtonya removed the [Status] Blocked Used to indicate that a current effort isn't able to move forward label May 22, 2023
@hellofromtonya hellofromtonya added this to the Gutenberg 15.9 milestone May 23, 2023
@hellofromtonya hellofromtonya added the [Status] Blocked Used to indicate that a current effort isn't able to move forward label May 24, 2023
@hellofromtonya
Copy link
Contributor Author

A fix is ready for merging. However, IMO the bugfix should wait until #40362 is resolved. I explain the reasoning here.

@hellofromtonya
Copy link
Contributor Author

Update:

This issue is no longer valid for the Fonts API once the Font Library is merged.

Why?

As shared in Ongoing Roadmap update #41479 (comment), the Fonts API will no longer be expose "register" or "enqueue" functionality. Instead, it will pull the fonts from theme.json merged data.

I'll close this issue.

@priethor priethor removed the [Status] In Progress Tracking issues with work in progress label Jul 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Status] Blocked Used to indicate that a current effort isn't able to move forward [Type] Bug An existing feature does not function as intended
Projects
None yet
2 participants