WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.5k stars 4.2k forks source link

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

Closed ndiego closed 1 year ago

ndiego commented 1 year ago

Description

https://github.com/WordPress/gutenberg/pull/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 https://github.com/WordPress/gutenberg/pull/50875.

While https://github.com/WordPress/gutenberg/pull/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

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 commented 1 year ago

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

hellofromtonya commented 1 year ago

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 commented 1 year ago

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.

ndiego commented 1 year ago

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 commented 1 year ago

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

anton-vlasenko commented 1 year ago

Applying the approach described in https://github.com/WordPress/gutenberg/pull/51770#issuecomment-1620867701 should fix this issue. I've just tested that it works.

anton-vlasenko commented 1 year ago

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 commented 1 year ago

Test Report for WordPress Core only

Env

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:

Test Results: no metaboxes - Yoast SEO deactivated

Scenario 2: With metaboxes (from Yoast SEO plugin)

IBM Plex Mono font:

Results: with metaboxes - Yoast SEO activated

Summary

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

ndiego commented 1 year ago

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

hellofromtonya commented 1 year ago

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

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:

headings look correct and font files are requested by the browser

Scenario 2: With metaboxes (from Yoast SEO plugin)

IBM Plex Mono font:

no font files requested in browser

headings do not look correct

Summary

With metaboxes (such as from Yoast SEO plugin):

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

hellofromtonya commented 1 year ago

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:

https://github.com/WordPress/gutenberg/blob/68fa80a0a8cb2c242f0ed280d3552efd9f04aacc/lib/experimental/fonts-api/fonts-api.php#L34-L36

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 commented 1 year ago

PR https://github.com/WordPress/gutenberg/pull/52343 should resolve this regression in Gutenberg. @ndiego @anton-vlasenko can you test the PR please to confirm it does resolve it for you?