Update format used by font resolver #50499

oandregal · 2023-05-10T08:30:45Z

Follow-up to #50297 and #50366

What?

Fixes two issues:

one issue by which the font resolver is unable to locate the proper fonts due to a later change introduced at Fix/wp get global styles for custom props returns internal variable #50366
updates how the fonts API determine what's a user-provided font, as it was incorrectly using the internal format. See [Fonts API] Automatically enqueue user-selected fonts #50297 (comment)

Why?

#50366 updated gutenberg_get_global_styles so that the custom format var:preset|... is never returned to consumers. Just before it was merged, a PR for the font resolver landed at #50297 that makes use of that format. Hence, when 50366 landed, it broke the font resolver.

Investigating this issue uncovered a bug in how the user-provided fonts are detected. See #50297 (comment)

How?

Update the font resolver to use the regular format.
Update the font resolver to query for user data explicitly, instead of trying to infer it based on whether the font is using var:preset|... format that any origin can use.

Testing Instructions

Make sure automated test pass.

See manual test instructions at #50297

oandregal · 2023-05-10T08:37:57Z

Current status of the PR: it returns all the expected results defined by the tests plus the system-font (which it should not according to the tests).

oandregal · 2023-05-10T08:58:39Z

Current status of the PR: it returns all the expected results defined by the tests plus the system-font (which it should not according to the tests).

This issue uncovered a bug in the way the fonts API enqueues user-provided fonts. See https://github.com/WordPress/gutenberg/pull/50297/files#r1189573512

It relied on the font using the var:preset|... format to decide whether it was user-provided or not. Anyone can use the var:... format, the tests just happened not to.

github-actions · 2023-05-10T09:08:53Z

Flaky tests detected in 0211bb9.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4935454675
📝 Reported issues:

[Flaky Test] should be able to create an hierarchical post without title support #36939 in /test/e2e/specs/editor/plugins/custom-post-types.spec.js

spacedmonkey · 2023-05-10T09:38:12Z

lib/experimental/fonts-api/class-wp-fonts-resolver.php

-		$global_styles       = wp_get_global_styles();
-		$user_selected_fonts = static::get_user_selected_fonts( $global_styles );
+		$user_selected_fonts = array();
+		$user_global_styles  = WP_Theme_JSON_Resolver_Gutenberg::get_user_data()->get_raw_data();


Would this avoid the cache we introduced?

This has the same performance impact than using wp_get_global_styles. That function has not a cache of their own: both approaches rely on the resolver cache.

hellofromtonya · 2023-05-10T11:43:54Z

user-provided font

What does this mean?

The Resolver's intent is to detect and then automatically enqueued fonts that a user "selects" for global usage via the Site Editor > Styles > Typography UI. All of the theme defined fonts (via theme.json file) are already registered and enqueued for global usage. But fonts registered via plugin (i.e. not in the theme's theme.json file) that are selected for global usage by a user need to be detected for auto enqueuing.

I assume user "provided" is different than user "selected".

@oandregal can you share more about the intent of this PR and the distinction between user "provided" and user "selected"?

And do you foresee system fonts as having property definitions that require a @font-face to be generated and printed by the Fonts API?

hellofromtonya

I'm blocking this PR until further discussion happens about its intent and what issues it resolves. Why? The PR is taking a different approach to identify all user "provided" fonts vs user "selected" fonts that are not in a theme's theme.json file.

oandregal · 2023-05-10T11:56:46Z

There are few things here:

PHP unit tests are failing
There is a bug in the existing resolver code [Fonts API] Automatically enqueue user-selected fonts #50297 (comment)
There's confusion in the font resolver about how to work with theme.json data.

I'm interested in shipping a fix for 1 ASAP as it's blocking development. If necessary, I'm open to revert #50366 but that is not the issue. 2 is also fixed by this PR.

3 requires a larger conversation and I need to look at the fonts code to understand how it is organized and ways forward. The TLDR is this: wp_get_global_styles or any other theme.json API doesn't give you data for a "single" origin (for example, only the data for "user"). It actually merges all the data together. The fonts resolver API PR circumvented this by checking for the var:preset|... format, expecting it only to come from the user data. That format can come from any other origin (default, theme, etc.). I created a test case to reproduce the bug at #50501

Does this help?

hellofromtonya · 2023-05-10T12:01:41Z

More discussion is needed around how to identify which fonts a user selected in the Site Editor > Global Styles > Typography UI. As PR #50297 is currently causing failed tests that is blocking development on other PRs, I'll revert that PR, reopen its enhancement issue, and then reopen a PR with the same code for further discussion.

oandregal · 2023-05-10T12:43:38Z

Closing this in favor of #50512

@oandregal

From PR #50499. Props @oandregal.

@oandregal

From PR #50499. Props @oandregal.

@oandregal

* Adds user-select fonts enqueuer. * Auto queue before printing * Always set $handles to false when empty When invoked as a hooked callback, it receives an empty string. Empty is the same as false. * Use WP_Theme_JSON_Resolver_Gutenberg::get_user_data(). From PR #50499. Props @oandregal. * Adds print tests. Moves datasets to trait.

Update format used by font resolver

e8f94f7

oandregal self-assigned this May 10, 2023

oandregal added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress labels May 10, 2023

Use gutenberg_get_global_styles instead of wp_get_global_styles

a523235

oandregal requested review from aristath, hellofromtonya and anton-vlasenko May 10, 2023 08:35

oandregal mentioned this pull request May 10, 2023

[Fonts API] Automatically enqueue user-selected fonts #50297

Merged

Get user global styles

0211bb9

oandregal marked this pull request as ready for review May 10, 2023 09:36

oandregal requested a review from spacedmonkey as a code owner May 10, 2023 09:36

spacedmonkey reviewed May 10, 2023

View reviewed changes

hellofromtonya requested changes May 10, 2023

View reviewed changes

oandregal closed this May 10, 2023

oandregal deleted the update/font-resolver-use-regular-css-format branch May 10, 2023 12:43

hellofromtonya mentioned this pull request May 10, 2023

[Fonts API] Automatically enqueue all user-selected fonts picked in Global Styles. #40363

Closed

hellofromtonya added a commit that referenced this pull request May 10, 2023

Use WP_Theme_JSON_Resolver_Gutenberg::get_user_data().

fe2bd0d

From PR #50499. Props @oandregal.

hellofromtonya mentioned this pull request May 10, 2023

[Fonts API] Automatically enqueue user-selected global fonts #50529

Merged

3 tasks

priethor removed the [Status] In Progress Tracking issues with work in progress label May 11, 2023

anton-vlasenko pushed a commit that referenced this pull request May 12, 2023

Use WP_Theme_JSON_Resolver_Gutenberg::get_user_data().

0f2e835

From PR #50499. Props @oandregal.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update format used by font resolver #50499

Update format used by font resolver #50499

oandregal commented May 10, 2023 •

edited

oandregal commented May 10, 2023 •

edited

oandregal commented May 10, 2023

github-actions bot commented May 10, 2023 •

edited

spacedmonkey May 10, 2023

oandregal May 10, 2023

hellofromtonya commented May 10, 2023

hellofromtonya left a comment

oandregal commented May 10, 2023 •

edited

hellofromtonya commented May 10, 2023

oandregal commented May 10, 2023

Update format used by font resolver #50499

Update format used by font resolver #50499

Conversation

oandregal commented May 10, 2023 • edited

What?

Why?

How?

Testing Instructions

oandregal commented May 10, 2023 • edited

oandregal commented May 10, 2023

github-actions bot commented May 10, 2023 • edited

spacedmonkey May 10, 2023

Choose a reason for hiding this comment

oandregal May 10, 2023

Choose a reason for hiding this comment

hellofromtonya commented May 10, 2023

hellofromtonya left a comment

Choose a reason for hiding this comment

oandregal commented May 10, 2023 • edited

hellofromtonya commented May 10, 2023

oandregal commented May 10, 2023

oandregal commented May 10, 2023 •

edited

oandregal commented May 10, 2023 •

edited

github-actions bot commented May 10, 2023 •

edited

oandregal commented May 10, 2023 •

edited