Webfonts API: Output only used fonts #39099

zaguiini · 2022-02-25T20:09:19Z

What

In #37140, we finally introduced the Webfonts API. The initial implementation has a pitfall: it includes every registered font, even if it's not being used, to the output.

This PR addresses exactly that: it introduces a mechanism to remove webfonts that are not used by the providers when emitting inline CSS that's sent to the front-end.

Why

A single @font-face declaration could add up to 2kb to every request. Imagine someone registering 100 fonts to a provider so one single font could be picked up in FSE. Without this filtering mechanism, we'd add a lot of useless information per request. It can -- and probably will -- become a problem very quickly.

How

I'm looking up the contents of the current template for every request and saving its used webfonts as an option. This happens for every template, lazily.

Fonts used in the global styles settings are also cached. We're caching because this is an expensive operation.

The cache is invalidated on every update of:

template;
template part;
global styles...

And also when the current theme changes.

The content (post/page content) is also processed, per request, to include webfonts used by the blocks inside the current post or page. There is no cache for this type of content, but there might be if someone requests it.

It also includes a filter called gutenberg_used_webfonts so one can override the decision of the mechanism, either by adding or removing fonts as needed. This is a concern raised by @justintadlock regarding a few blocks that do not have the font-family attribute yet. It's a workaround until all blocks get the possibility to change the font family inside the block editor instead of working around it by using additional CSS classes.

Testing

You should check only the used fonts are included as inline CSS when the webfont is picked as:

an element, block, or typography setting in global styles;
the font family on any block that supports it in any active template;
the font family on any block that supports it in any template part and that template part is used by any active template;
the font family on any block that supports it in any post or page in the block editor.

TODO

Use WP Cache instead of option to save stuff?

lib/compat/wordpress-6.0/class-wp-webfonts.php

zaguiini · 2022-03-02T21:36:11Z

@justintadlock I'm going the filter route in this PR.

I'm thinking of creating an issue regarding exposing the fontFamily attribute to all blocks. It's out of the scope of this PR, that's why I thought of adding the filter here as a temporary solution until 3rd-party block maintainers edit their plugins to take fontFamily attribute into consideration instead of classes. Any concerns/suggestions?

justintadlock · 2022-03-03T21:04:21Z

I'm going the filter route in this PR.

I'm thinking of creating an issue regarding exposing the fontFamily attribute to all blocks. It's out of the scope of this PR, that's why I thought of adding the filter here as a temporary solution until 3rd-party block maintainers edit their plugins to take fontFamily attribute into consideration instead of classes. Any concerns/suggestions?

@zaguiini I think that should work.

There are probably some other cases with supporting non-block content, such as plain HTML elements from classic posts. But, again, out of scope. WP really needs to allow theme devs to style all HTML elements via theme.json, too.

zaguiini · 2022-03-07T22:43:38Z

lib/compat/wordpress-6.0/class-wp-webfonts.php

+			return $response;
+		}
+
+		$this->invalidate_globally_used_webfonts_cache();


This is unnecessary. Solely updating the option as I'm doing in the next line is enough.

jeyip · 2022-03-08T18:47:05Z

lib/compat/wordpress-6.0/class-wp-webfonts.php

+	public function register_current_template_filter() {
+		$templates = get_block_templates( array(), 'wp_template' );
+
+		foreach ( $templates as $template ) {
+			add_filter(
+				$template->slug . '_template',
+				function() use ( $template ) {
+					$this->register_used_webfonts_by_template( $template );
+				}
+			);
+		}
+	}


@creativecoder Do you happen to know if there's a single hook that returns the current template instead of what we're doing here?

The $template_html global is available in wp_head, and contains all the HTML of the page if that helps (in block themes)

Does it contain the slug too?

Does it contain the slug too?

No, $template_html is set in the template-canvas.php file when building the page, and only contains the HTML for the page:

gutenberg/lib/compat/wordpress-5.9/template-canvas.php

Line 12 in f2e5e95

$template_html = gutenberg_get_the_template_html();

Got it, so it won't really work for our use case since we need the slug too.

lib/compat/wordpress-6.0/class-wp-webfonts.php

… used in the front-end

… all the webfonts used in the front-end

…e part update

creativecoder · 2022-03-10T04:28:40Z

This is a really important step forward for the Webfonts API, thank you for tackling it, @zaguiini !

While this change largely works for removing unused font-face declarations, a couple things worry me about the approach. The first is that the initial Webfont API we have now can be used in many different (and possibly unexpected) ways that might feel broken if we filter out fonts by default (unless they are used in a very specific way). For example, any css files included in a theme, or custom css from a plugin that registers a specific font would not be detected here, the font-face rule removed, and the font not loaded.

It seems like it would be better if the Webfonts API had an explicit way to differentiate between

Fonts that are registered so they show up in the font-family settings for block and global styles
Fonts that are used by those settings or are declared necessary by a theme/plugin that causes the @font-face rules to be printed out

My second concern is the complexity of caching what fonts are used for what templates when they are saved. This seems like it might be fragile if something unexpected happens between saving and outputting the templates, and that it would be better to check what fonts are used as the page is loaded, if at all possible.

Since I anticipate continuing to iterate on this approach, I created #39332 as a hub for discussing and tracking work on this area.

zaguiini · 2022-03-11T15:05:11Z

We're closing this PR since it's not going to be merged because of the implementation. However, as pointed above, there's an issue for it now: #39332.

zaguiini force-pushed the try/cache-used-webfonts branch from 58ed84d to afe1b56 Compare February 25, 2022 20:09

Base automatically changed from add/webfonts-simplified-api to trunk February 28, 2022 12:55

zaguiini force-pushed the try/cache-used-webfonts branch 2 times, most recently from 5582859 to 64c912f Compare February 28, 2022 22:17

zaguiini mentioned this pull request Mar 1, 2022

Filter out unused webfonts from the front-end #39036

Closed

jeyip reviewed Mar 1, 2022

View reviewed changes

lib/compat/wordpress-6.0/class-wp-webfonts.php Outdated Show resolved Hide resolved

jeyip reviewed Mar 1, 2022

View reviewed changes

lib/compat/wordpress-6.0/class-wp-webfonts.php Outdated Show resolved Hide resolved

jeyip reviewed Mar 1, 2022

View reviewed changes

lib/compat/wordpress-6.0/class-wp-webfonts.php Outdated Show resolved Hide resolved

zaguiini force-pushed the try/cache-used-webfonts branch from 8825187 to 6a5d8ae Compare March 2, 2022 21:34

zaguiini self-assigned this Mar 2, 2022

zaguiini force-pushed the try/cache-used-webfonts branch 3 times, most recently from b3ce575 to 23a8205 Compare March 3, 2022 16:39

zaguiini marked this pull request as ready for review March 3, 2022 16:39

zaguiini requested review from TimothyBJacobs and spacedmonkey as code owners March 3, 2022 16:39

mtias added the [Type] Performance Related to performance efforts label Mar 3, 2022

zaguiini force-pushed the try/cache-used-webfonts branch from 23a8205 to a0e8182 Compare March 3, 2022 17:01

zaguiini requested review from aristath, jeyip, mattwiebe, Addison-Stavlo, creativecoder, vindl and mtias March 3, 2022 17:03

zaguiini changed the title ~~Cache used webfonts instead of querying on every page load~~ Webfonts API: Filter out unused fonts from the output Mar 3, 2022

zaguiini changed the title ~~Webfonts API: Filter out unused fonts from the output~~ Webfonts API: Output only used fonts Mar 3, 2022

zaguiini commented Mar 7, 2022

View reviewed changes

jeyip reviewed Mar 8, 2022

View reviewed changes

lib/compat/wordpress-6.0/class-wp-webfonts.php Outdated Show resolved Hide resolved

jeyip reviewed Mar 8, 2022

View reviewed changes

lib/compat/wordpress-6.0/class-wp-webfonts.php Outdated Show resolved Hide resolved

zaguiini added 8 commits March 8, 2022 16:44

Filter out webfonts that are not being used by the front-end

fa883aa

Include webfonts used in global styles in the set of all the webfonts…

65364d8

… used in the front-end

Update webfonts used in global styles cache after save

cf9f5a6

Update webfonts used in global styles cache after theme switch

280c3f0

Include and cache webfonts used in the current template in the set of…

6c410d0

… all the webfonts used in the front-end

Invalidate webfonts used in templates cache after template or templat…

78dffcb

…e part update

Invalidate webfonts used in templates cache on theme switch

3b62835

Collect webfonts used in the post/page content

f045382

zaguiini force-pushed the try/cache-used-webfonts branch from a0e8182 to 3fcd821 Compare March 8, 2022 23:02

Regenerate used webfonts meta for post and page on save

62d677b

zaguiini force-pushed the try/cache-used-webfonts branch from 3fcd821 to 62d677b Compare March 8, 2022 23:08

creativecoder mentioned this pull request Mar 10, 2022

Webfonts: remove unused font-face declarations #39332

Closed

zaguiini closed this Mar 11, 2022

zaguiini deleted the try/cache-used-webfonts branch March 22, 2022 15:43

Directory

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Webfonts API: Output only used fonts #39099

Webfonts API: Output only used fonts #39099

zaguiini commented Feb 25, 2022 •

edited

zaguiini commented Mar 2, 2022 •

edited

justintadlock commented Mar 3, 2022

zaguiini Mar 7, 2022

jeyip Mar 8, 2022

aristath Mar 9, 2022

zaguiini Mar 9, 2022

aristath Mar 9, 2022

zaguiini Mar 9, 2022

creativecoder commented Mar 10, 2022

zaguiini commented Mar 11, 2022 •

edited

Directory

Webfonts API: Output only used fonts #39099

Webfonts API: Output only used fonts #39099

Conversation

zaguiini commented Feb 25, 2022 • edited

What

Why

How

Testing

TODO

zaguiini commented Mar 2, 2022 • edited

justintadlock commented Mar 3, 2022

zaguiini Mar 7, 2022

Choose a reason for hiding this comment

jeyip Mar 8, 2022

Choose a reason for hiding this comment

aristath Mar 9, 2022

Choose a reason for hiding this comment

zaguiini Mar 9, 2022

Choose a reason for hiding this comment

aristath Mar 9, 2022

Choose a reason for hiding this comment

zaguiini Mar 9, 2022

Choose a reason for hiding this comment

creativecoder commented Mar 10, 2022

zaguiini commented Mar 11, 2022 • edited

zaguiini commented Feb 25, 2022 •

edited

zaguiini commented Mar 2, 2022 •

edited

zaguiini commented Mar 11, 2022 •

edited