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

Local font asset files not loading in post Editor when metaboxes are enabled (i.e. Editor is not iframed) #51209

Closed
ndiego opened this issue Jun 2, 2023 · 12 comments · Fixed by #51770 or #52343
Assignees
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release

Comments

@ndiego
Copy link
Member

ndiego commented Jun 2, 2023

Description

#51178 fixed a bug in 15.9 that prevented fonts included in themes to be loaded in the Editor, both post and Site Editor. This was due to a change in #50875.

While #51178 fixed the issue when the Editor is iframed, if a plugin registers metaboxes, the Editor is no longer iframed. As a result, the fonts are no longer loaded.

cc @hellofromtonya @ellatrix

Step-by-step reproduction instructions

  1. Using TT3, switch to the Block out style variation, which uses a custom heading font making it easy to see the difference.
  2. Install and activate Yoast, or another plugin that registers a metabox in the post Editor.
  3. Create a new page/post and add a few headings. See that the custom IBM Plex Mono font is not actually getting applied.
  4. Deactivate Yoast and see that the font is now applied.

Screenshots, screen recording, code snippet

With Yoast Active Without Yoast Active
with metaboxes without metaboxes

Environment info

  • WordPress 6.2
  • Gutenberg 15.9.1

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

@ndiego ndiego added [Type] Bug An existing feature does not function as intended [Feature] Fonts API [Type] Regression Related to a regression in the latest release labels Jun 2, 2023
@hellofromtonya hellofromtonya added the [Status] In Progress Tracking issues with work in progress label Jun 22, 2023
@hellofromtonya
Copy link
Contributor

This issue will be resolved with Font Face, which is replacing the Fonts API.

@hellofromtonya
Copy link
Contributor

I'm closing this issue.

Why? No further work will happen on the Fonts API.

Why? It is being replaced by Font Face and will be removed when Font Library is merged.

@hellofromtonya
Copy link
Contributor

I'm not seeing where this issue was directly introduced within the Fonts API itself. @ndiego reported the issue still exists with the new Font Face generator and printer. Since it exists in both the Fonts API and new Font Face, I suspect the root cause is not within either.

I'm reopening this issue to capture the discussion, investigation, and separate resolving PR.

@hellofromtonya hellofromtonya reopened this Jul 5, 2023
@ndiego
Copy link
Member Author

ndiego commented Jul 5, 2023

Thanks @hellofromtonya. I just know that this is going to impact a LOT of people, given the popularity of tools like Yoast and ACF (both of which use metaboxes). My guess is something related to the relatively recent work on iframing the Post Editor is the culprit. Any ideas @ellatrix?

@hellofromtonya
Copy link
Contributor

More context to help:

@ndiego noted here

fonts registed in theme.json do not load when metaboxes are present in Gutenberg 16.1 or WordPress 6.3 Beta 2 (i.e. the Editor is iframed)....

In WordPress 6.2.2, without Gutenberg active, if you enable the Yoast plugin, the fonts in TT3 will load as expected. Yoast includes a metabox and the post Editor in 6.2.2 is not iframed.

Now if you update to 6.3 Beta 2, the fonts are not loaded when a metabox is present (the Editor is no longer iframed).

Nick notes the metabox regression was introduced during WordPress 6.3 beta cycle.

Neither the Fonts API nor the new Font Face have been merged into WordPress 6.3. Rather, WordPress Core is still using the stopgap code from WP 6.0.0 (i.e. _wp_theme_json_webfonts_handler()).

Something else is the culprit / root cause, some other backport to Core introduced during the 6.3 cycle is causing this regression.

Note: If the fix for the root cause also requires a change to be made in the @font-face handling, it'll need to be done in _wp_theme_json_webfonts_handler() within WordPress Core.

What's the next step?

Identify the PR(s) merged into Gutenberg or the backport in WordPress Core that introduced the regression. This step will take some testing and investigation.

@anton-vlasenko @ellatrix

@hellofromtonya hellofromtonya added [Feature] Typography Font and typography-related issues and PRs and removed [Feature] Fonts API labels Jul 5, 2023
@hellofromtonya hellofromtonya changed the title [Fonts API] Local font asset files not loading in post Editor when metaboxes are enabled (i.e. Editor is not iframed) Local font asset files not loading in post Editor when metaboxes are enabled (i.e. Editor is not iframed) Jul 5, 2023
@anton-vlasenko
Copy link
Contributor

anton-vlasenko commented Jul 5, 2023

Applying the approach described in #51770 (comment) should fix this issue. I've just tested that it works.

@anton-vlasenko
Copy link
Contributor

Test report

  1. WordPress 6.3-beta3-56130-src, no meta boxes: font faces are loaded;
    Screenshot 2023-07-05 at 18 36 44

  2. WordPress 6.3-beta3-56130-src, meta boxes enabled: font faces are loaded;
    Screenshot 2023-07-05 at 18 36 54

  3. WordPress 6.3-beta3-56130-src, Gutenberg trunk (16.1.0), no meta boxes: font faces are loaded;
    Screenshot 2023-07-05 at 18 37 08

  4. WordPress 6.3-beta3-56130-src, Gutenberg trunk (16.1.0), meta boxes enabled: font faces are NOT loaded;
    Screenshot 2023-07-05 at 18 37 21

@hellofromtonya
Copy link
Contributor

hellofromtonya commented Jul 5, 2023

Test Report for WordPress Core only

Env

  • WordPress: current trunk (i.e. 6.3-beta3-56130-src)
  • PHP: 7.4.1
  • Browser: Firefox and Chrome
  • Theme: TT3
  • Plugins: Yoast SEO

Note: the Gutenberg plugin is deactivated in this test report.

Set up in Site Editor

  1. Go to the Site Editor > Styles > Typography > Headings.
  2. In "Font", select "IBM Plex Mono".
  3. In "Appearance", select "Bold Italic.
  4. Click/select the "Save" button twice.

Site Editor > Styles > Typography > Headings UI

Here is what the font looks like on Google Fonts:
Screen Shot 2023-07-05 at 12 54 01 PM

Test Results

Scenario 1: No metaboxes

IBM Plex Mono font:

  • file is being requested by the browser ✅
  • visually looks correct ✅

Test Results: no metaboxes - Yoast SEO deactivated

Scenario 2: With metaboxes (from Yoast SEO plugin)

IBM Plex Mono font:

  • file is being requested by the browser ✅
  • visually looks correct ✅

Results: with metaboxes - Yoast SEO activated

Summary

WordPress Core's current trunk via 6.3-beta3-56130-src works as expected with no regression ✅

  • The font files are being requested from the server ✅
  • Fonts visually look correct ✅

@ndiego
Copy link
Member Author

ndiego commented Jul 5, 2023

I can confirm with 6.3-beta3-56140. Activating Gutenberg causes the issue to come back. But looks like we are good in Core 🙌

@hellofromtonya
Copy link
Contributor

hellofromtonya commented Jul 5, 2023

Test Report 2: With Gutenberg

Next, this test report repeats my above tests but with the Gutenberg plugin activated.

Test Report for WordPress Core only

Env

  • localhost: Local
  • WordPress: Beta 3
  • PHP: 7.4.1
  • Browser: Firefox and Chrome
  • Theme: TT3
  • Plugins: Gutenberg 16.1.1 and Yoast SEO 20.10

Set up in Site Editor

  1. Go to the Site Editor > Styles > Typography > Headings.
  2. In "Font", select "IBM Plex Mono".
  3. In "Appearance", select "Bold Italic.
  4. Click/select the "Save" button twice.

Site Editor > Styles > Typography > Headings UI

Here is what the font looks like on Google Fonts:
Screen Shot 2023-07-05 at 12 54 01 PM

Test Results

Scenario 1: No metaboxes

IBM Plex Mono font:

  • file is being requested by the browser ✅
  • visually looks correct ✅

headings look correct and font files are requested by the browser

Scenario 2: With metaboxes (from Yoast SEO plugin)

IBM Plex Mono font:

  • file is not being requested by the browser ❌
  • visually does not look correct ❌

no font files requested in browser

headings do not look correct

Summary

With metaboxes (such as from Yoast SEO plugin):

  • WordPress Core 6.3 Beta 3 works correctly ✅
  • Gutenberg does not ❌

The regression is in Gutenberg, but not in WordPress Core 6.3 Beta 3.

@hellofromtonya
Copy link
Contributor

Why are the @font-face styles are not being generated?

tl;dr
Printing is limited to the iframe for block themes.

I was wrong. The regression is in the Fonts API.

The change that introduced the regression in Gutenberg:

The hook to print them into the main doc's <head> was guarded to only work for classic themes:

if ( ! wp_theme_has_theme_json() ) {
add_action( 'admin_print_styles', 'wp_print_fonts', 50 );
}

This was done for performance to avoid printing the @font-face styles more than once, i.e. in the main doc and in the iframe. But consideration was not given to when metabox is present, causing the editor no longer be in an iframe.

Commenting out the if guard wrapping around the add_action() resolves the regression in the Fonts API.

The new Font Face implementation replacing the Fonts API also resolves the regression.

As the Font Face is not yet merged into Gutenberg, I'll create a quick fix PR to remove the guarding in wp_fonts(). This way, the regression is resolved and not dependent upon the new Font Face.

@hellofromtonya
Copy link
Contributor

PR #52343 should resolve this regression in Gutenberg. @ndiego @anton-vlasenko can you test the PR please to confirm it does resolve it for you?

@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
[Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended [Type] Regression Related to a regression in the latest release
Projects
No open projects
4 participants