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] does not correctly handle variable fonts files in .woff2 format #41158

Closed
alexandrebuffet opened this issue May 19, 2022 · 10 comments
Assignees
Labels
[Status] Blocked Used to indicate that a current effort isn't able to move forward [Status] Needs More Info Follow-up required in order to be actionable.
Projects

Comments

@alexandrebuffet
Copy link
Contributor

Description

When I register custom font families with variable font files in .woff2 format into my theme.json, the browser doesn't seem to understand the font format and the "variable" properties doesn't work.

Tested up with WordPress 5.9.3 + Gutenberg 13.2.1 and a webfont registration via theme.json in a custom Block Theme (as explained in #37140 PR).

Maybe I'm wrong, but it seems to be the format declared in the @font-face that is incorrect. I believe that a variable font file in .woff2 format should be declared with format('woff2' supports variations) hint.

Currently, it's the Webfonts Provider that deduces the format. Wouldn't it be better to leave the possibility to declare the "format" inside src array (via PHP or theme.json) for each source file?

Step-by-step reproduction instructions

  1. Register a new font family with local .woff2 variable webfont file as source
  2. Apply font family to Heading block
  3. Try to change font weight, doesn't work

Screenshots, screen recording, code snippet

No response

Environment info

  • WordPress 5.9.3, Gutenberg 13.2.1 with Custom Block Theme

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

@SchecDev
Copy link

SchecDev commented Jun 4, 2022

I stumbled upon an issue with the new webfonts registration via theme.json and searching open issues I found this one. Which is at least very similar to my issues (I think it's actually the same, but due to another cause), so I decided to post here instead of opening a new issue.

Switching to WP 6.0 my Twenty Twenty-Two based theme (the original Twenty Twenty-Two is affected as well!) the headings - which use the Source Serif Pro (web-)font - switched font-weight and italics in headings doesn't work anymore (see screenshots).

The cause is that the webfont handler _wp_theme_json_webfont_handler adds a local($font_family) in $fn_compile_src to the src attribute of the generated CSS inline style. And on my system (Win 10 btw.) there happen's to be the Source Serif Pro font installed (although in ttf version). Which now get's precedence over the webfont, and as ttf doesn't respond to font-weight and/or italic settings anymore. Instead the name of the font used would need to change to "Source Serif Pro Light", "Source Serif Pro Bold Italic", etc.

System_fonts

By removing the local part of src in the CSS using the browser's debugging tools I could restore the originally defined styling:

Webfont_local_1_both
Webfont_local_2_only_italic
Webfont_local_3_none

So the result in my case is the same as above, a font being used not responding to font-weight settings (or italic style). But the cause is not an incorrect handling of the .woff2 format which works fine in my environments without supports variation hint. The problem is the provided ´.woff2´ isn't used st all but gets replaced by the client-locally installed ttf version. If this issue and the behaviour I've described are one and the same depends on the question if you had the same scenario, i. e. having the fonts used locally installed on the client system you were using, @alexandrebuffet.

Anyway, I consider this behaviour a bug, it just renders font-weight/italic settings useless, the headings of Twenty Twenty-Two theme now look different on my PC and my tablet (obviously unwanted).

From a development point of view the webfont handler should probably create several font-faces with the appropriate font names in local. Or provide an option to specify this name explicitely on the font face level, or to suppress the addition of the local part at all.

Currently it's not even possible to avoid or fix this behaviour in a "clean" way. The only "solutions" I found are:

  • Not using the theme.json to define webfonts (you can still do this in php as before 6.0)
  • Changing the font family name (but not the slug, so definitions using var(--wp--preset...) still work) in something that for sure doesn't exist as local font thus forcing the usage of the woff2 webfont.
    webfont_theme json
@CodeAdminDe
Copy link

I can confirm the issue described by @SchecDev, it also occurs in WP 6.0.1 in the block editor. Is there an advantage to using local()?

By the way... I would like to help with WordPress development.... who would I need to contact to discuss the behavior so we can fix it?

@nigel-maher
Copy link

nigel-maher commented Aug 26, 2022

In addition to the local version of the font taking precedence, the setup of the local font file(s) can come into play here too.

My theme.json used "Inter" (Inter-VariableFont_slnt,wght.ttf) which worked fine while I had that very same file installed on my desktop, as expected there were no visual differences in the browser when it detected and called the locally installed (singular) file.

However, I had to remove the variable version of Inter and install the individual/static .ttf files instead, so that desktop publishing software would load the font. It was then I noticed the issues described in this thread.

If I had left the variable version installed on my machine, I might not have picked up the problem without close inspection, because visually at least, everything looked fine.

Like @SchecDev I got around the issue of different font file versions by prefixing "fontFamily":

{
    "fontFamily": "Prefix Inter",
    "slug": "inter",
    "name": "Inter",
    "fontFace": [
        {
          "fontFamily": "Prefix Inter",
          "fontWeight": "100 900",
          "fontStyle": "normal",
          "fontStretch": "normal",
          "src": ["file:./assets/fonts/inter/Inter-VariableFont_slnt,wght.ttf"]
        }
    ]
},

That was a clean a solution as I could come up with for now, as it still shows the font family as "Inter" in the editor.

@jeflopodev
Copy link

jeflopodev commented Jan 27, 2023

I tried what @SchecDev & @nigel-maher suggested on wp 6.1.1 and Gutenberg 15.0.1 as a workaround. The prefix trick temporarily fixes the problem. Thx btw

@hellofromtonya hellofromtonya changed the title Webfonts API does not correctly handle variable fonts files in .woff2 format Apr 6, 2023
@hellofromtonya
Copy link
Contributor

hellofromtonya commented Apr 6, 2023

The cause is that the webfont handler _wp_theme_json_webfont_handler adds a local($font_family) in $fn_compile_src to the src attribute of the generated CSS inline style.

By removing the local part of src in the CSS using the browser's debugging tools I could restore the originally defined styling:

The @font-face local() was removed from:

@hellofromtonya
Copy link
Contributor

Maybe I'm wrong, but it seems to be the format declared in the @font-face that is incorrect. I believe that a variable font file in .woff2 format should be declared with format('woff2' supports variations) hint.

@aristath what do you think?

@aristath
Copy link
Member

aristath commented Apr 7, 2023

Reading the docs for font-formats on MDN, I see that the proper way to write this would be format(woff2) tech(variations).
The problem with that is that it's not supported by all browsers yet (see table on caniuse for the tech keyword), so we'll need to use the old syntax which is format("woff2-variations").

There is no fast, performant & reliable way to automatically detect if a font-file contains variations or not, so perhaps we could add a tech item when defining the fontFace in theme.json?
This way we could manually define the tech of the font, and say it's a variable font. By adding a tech item we'd also be able to support any of the other available technologies supported by fonts (see table on MDN for font technologies).
Since there is the browser-support issue with using the tech keyword, we could do the following:

  • If tech is defined and set to variations, we change the src format from format("woff2") to format("woff2-variations").
  • If tech is defined and set to a value other than variations, we append it to the src like for example format("woff2") tech(incremental).
  • If in the future all browsers properly support the tech keyword, it will be easy to switch to the new format simply by removing the exception/condition for variations and instead fall-back to the more generic implementation for tech that we already implemented for all other values except variations.

I created a PR on #49653 that does just that, let me know what you think 👍

@hellofromtonya hellofromtonya moved this from Backlog to In progress in Fonts API Apr 25, 2023
@ironprogrammer
Copy link
Contributor

Reproduction Report

This report focuses on .woff2 variable font support originally reported in this issue. It does not cover issues related to OS-installed font name collisions that have also been mentioned in this ticket, which should ideally be filed separately. (Though I'd like to test these in macOS as well.)

The Fonts API has undergone extensive changes since this ticket was opened. I was unable to reproduce, but am I missing anything?

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 13.4
  • Browser: Safari 16.5, Google Chrome 114.0.5735.90, Mozilla Firefox 113.0.2
  • Server: nginx/1.25.0
  • PHP: 7.4.33
  • WordPress: 6.3-alpha-55505-src
  • Theme: twentytwentythree v1.1
  • Active Plugins:
    • gutenberg v15.9.0

Steps to Test

  1. Download the Latin .woff2 file for Oswald via the following URL: https://fonts.googleapis.com/css2?family=Oswald:wght@200..700&display=swap (the .woff2 URL to download can be found in the resulting CSS). Rename the file to Oswald-VariableFont_wght.woff2.
  2. In the TT3 theme folder, move the file into a new folder: assets/fonts/oswald/.
  3. In the theme's theme.json file, register the font by adding the following to settings -> typography -> fontFamilies (I inserted it above DM Sans):
{
	"fontFace": [
		{
			"fontFamily": "Oswald",
			"fontStretch": "normal",
			"fontStyle": "normal",
			"fontWeight": "200 700",
			"src": [
				"file:./assets/fonts/oswald/Oswald-VariableFont_wght.woff2"
			]
		}
	],
	"fontFamily": "\"Oswald\", sans-serif",
	"name": "Oswald",
	"slug": "oswald"
},
  1. In the Site Editor, open Styles > Typography > Headings, and confirm that Oswald appears in the list.
  2. Assign Oswald to Headings. Adjust the Appearance (font weight). Observe results.

Actual Results

  • ❌ Unable to reproduce issue. Font weight was adjustable.

Additional Notes

  • In addition to testing the .woff2 file, I also swapped in the .ttf version (downloaded directly), which performed the same when set in theme.json.
  • While not demonstrated, I also tested manually changing font-weight for the heading element to confirm that the expected 200-700 weight range was supported by the file.

Supplemental Artifacts

gb-41158-variable-font-woff2

@hellofromtonya hellofromtonya moved this from In progress to In Discussion in Fonts API Jun 5, 2023
@hellofromtonya hellofromtonya added [Status] In discussion Used to indicate that an issue is in the process of being discussed [Status] Needs More Info Follow-up required in order to be actionable. and removed [Status] In Progress Tracking issues with work in progress labels Jun 5, 2023
@hellofromtonya
Copy link
Contributor

Thank you @ironprogrammer for your test report. I agree this API is vastly different from the implementation when this issue was first opened.

@alexandrebuffet can you retest please with Gutenberg 15.9.1 or newer? Is this still an issue for you?

Reading the docs for font-formats on MDN, I see that the proper way to write this would be format(woff2) tech(variations).
The problem with that is that it's not supported by all browsers yet (see table on caniuse for the tech keyword), so we'll need to use the old syntax which is format("woff2-variations").

As @aristath noted, it is not yet "supported by all browsers".

Given that the new format is not yet supported by all browsers and the issue is not reproducible, I'm inclined to think this issue may not be actionable at this time. It seems like a close for "maybe later" in the future when all browsers support the new approach.

I'll close this issue for now. However, if the issue still persists, please re-open and share the step-by-step instructions on how to reproduce the problem.

Fonts API automation moved this from In Discussion to Done Jun 5, 2023
@hellofromtonya hellofromtonya added [Status] Blocked Used to indicate that a current effort isn't able to move forward and removed [Status] In discussion Used to indicate that an issue is in the process of being discussed labels Jun 5, 2023
@hellofromtonya
Copy link
Contributor

Note: I added the [Status] Blocked label. Why?

  1. Not reproducible.
  2. All browsers do not yet support the format approach.
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 [Status] Needs More Info Follow-up required in order to be actionable.
10 participants