Font face resolver: print font faces from font families defined in all theme.json origins #59376

matiasbenedetto · 2024-02-26T17:01:35Z

What?

Port this PR from WordPress core repo: WordPress/wordpress-develop#6161

Font face resolver: If a font family name is repeated across theme.json origins (default, theme, custom), only the font faces from one origin are rendered, so the site lacks the other font faces added.

This problem was reported originally here: #58764

Why?

To print the missing font faces when a font family is defined in more than one place.
Fixes: #58764

How ?

The parse_settings method of the WP_Font_Face_Resolver class uses an associative array to list all the font families that must be printed on the page. This PR replaces it with an indexed array because the associative array prevents having more than one font family with the same name, causing the bug that this PR fixes and seems unnecessary.

Testing instruction

Use a theme with at least one font family defined with some font faces.
Using the font library, install a font family with the same name as the existing in the theme.
Navigate the site's public frontend and check that all the font faces were printed to the page in the <style id="wp-fonts-local"></style> tag.

…an one theme.json data origin.

github-actions · 2024-02-26T17:01:49Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: matiasbenedetto <mmaattiiaass@git.wordpress.org>
Co-authored-by: youknowriad <youknowriad@git.wordpress.org>
Co-authored-by: arthur791004 <arthur791004@git.wordpress.org>
Co-authored-by: annezazu <annezazu@git.wordpress.org>
Co-authored-by: getdave <get_dave@git.wordpress.org>
Co-authored-by: okmttdhr <okat@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

github-actions · 2024-02-26T17:02:50Z

This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress.

If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged.

If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack.

Thank you! ❤️

View changed files

❔ lib/compat/wordpress-6.5/fonts/font-face/class-wp-font-face-resolver.php
❔ lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php
❔ lib/compat/wordpress-6.4/fonts/fonts.php
❔ lib/load.php
❔ lib/compat/wordpress-6.5/fonts/font-face/class-wp-font-face.php

youknowriad · 2024-02-27T13:31:50Z

lib/compat/wordpress-6.4/fonts/font-face/class-wp-font-face-resolver.php

-					}
-
-					$fonts[ $font_family_name ] = static::convert_font_face_properties( $definition['fontFace'], $font_family_name );
+					$fonts[] = static::convert_font_face_properties( $definition['fontFace'], $font_family_name );


This should move to 6.5 folder now I think.

Do you think that because of this particular change or because of something else?

If you mean because of this particular change, this is my rationale:

I understand that the wordpress-6.4 folder holds the code needed to make the code work with the latest Gutenberg version running on WordPress < 6.4, right? That's still true, so I'm unsure why this should be moved.

I understand that the wordpress-6.4 folder holds the code needed to make the code work with the latest Gutenberg version running on WordPress < 6.4, right?

no, wordpress-6.4 is the code that was backported during 6.4, so to make Gutenberg work with 6.3 and lower.

wordpress-6.4 is the code that was backported during 6.4, so to make Gutenberg work with 6.3 and lower.

I think we said the same but with different words :)

Given this PR is only a bugfix, and these classes were merged in 6.4, and only one bugfix will be different in 6.5, should we still move all files to the WordPress-6.5 folder?

yes, because the 6.4 folder will get removed once the minimum supported version for Gutenberg is going to be 6.4 but that particular fix will still be needed.

I think if we don't figure out a way to override the 6.4 use-case properly, then there's no match point in "fixing" this bug in Gutenberg. Maybe we should just close this PR.

figure out a way to override the 6.4 use-case properly

The use 6.4 use-case would be printing all the font faces when running wp 6.4 + the latest GB plugin, but I don't understand the quote. Could you explain or add more context, please?

I don't understand your question :P

I'm saying that if we can't override the class properly on Gutenberg, why change it at all, the 6.4 code should match the code that was merged in 6.4 and we can't move it to 6.5 and update it because it will have no effect anyway.

Oh, I see, thanks for explaining. In that case, the bug will be still there when running 6.4 + Gutenberg. Is that ok? If that's OK we can close the PR.

It's ok for me I think.

matiasbenedetto · 2024-03-01T15:05:56Z

Closing this PR following @youknowriad 's rationale in this comment: #59376 (comment)

if we can't override the class properly on Gutenberg, why change it at all, the 6.4 code should match the code that was merged in 6.4 and we can't move it to 6.5 and update it because it will have no effect anyway.

print the missing font faces when a font family is defined in more th…

b6ea657

…an one theme.json data origin.

matiasbenedetto requested a review from spacedmonkey as a code owner February 26, 2024 17:01

matiasbenedetto marked this pull request as draft February 26, 2024 17:01

matiasbenedetto added [Type] Bug An existing feature does not function as intended [Feature] Typography Font and typography-related issues and PRs labels Feb 26, 2024

matiasbenedetto mentioned this pull request Feb 27, 2024

Font Weight Conflicts Between Built-in and Installed Fonts #58764

Closed

matiasbenedetto marked this pull request as ready for review February 27, 2024 12:59

youknowriad reviewed Feb 27, 2024

View reviewed changes

arthur791004 mentioned this pull request Feb 27, 2024

Google Fonts: Update the implementation of the print_font_faces (2nd try) Automattic/jetpack#35986

Merged

3 tasks

matiasbenedetto added 2 commits March 1, 2024 09:31

Merge branch 'trunk' into port/print-faces-from-all-origins

b873f5a

move files to from compat/wordpress-6.4 to compat/wordpress-6.5

2ec7d76

matiasbenedetto mentioned this pull request Mar 1, 2024

Font face resolver: print font faces from font families defined in all theme.json origins #59321

Closed

matiasbenedetto closed this Mar 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Font face resolver: print font faces from font families defined in all theme.json origins #59376

Font face resolver: print font faces from font families defined in all theme.json origins #59376

matiasbenedetto commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

youknowriad Feb 27, 2024

matiasbenedetto Feb 27, 2024

youknowriad Feb 27, 2024

matiasbenedetto Feb 27, 2024 •

edited

Loading

youknowriad Feb 27, 2024

youknowriad Mar 1, 2024

matiasbenedetto Mar 1, 2024

youknowriad Mar 1, 2024

matiasbenedetto Mar 1, 2024

youknowriad Mar 1, 2024

matiasbenedetto commented Mar 1, 2024

Font face resolver: print font faces from font families defined in all theme.json origins #59376

Font face resolver: print font faces from font families defined in all theme.json origins #59376

Conversation

matiasbenedetto commented Feb 26, 2024 • edited Loading

What?

Why?

How ?

Testing instruction

github-actions bot commented Feb 26, 2024 • edited Loading

github-actions bot commented Feb 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matiasbenedetto Feb 27, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matiasbenedetto commented Mar 1, 2024

matiasbenedetto commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

github-actions bot commented Feb 26, 2024 •

edited

Loading

matiasbenedetto Feb 27, 2024 •

edited

Loading