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

Deprecate and make Fonts API non-functional. #52485

Merged
merged 3 commits into from
Jul 19, 2023

Conversation

hellofromtonya
Copy link
Contributor

@hellofromtonya hellofromtonya commented Jul 10, 2023

Closes 51819.

What?

Creates a BC Layer in the new Font Face.

Deprecates all of the Fonts API's public-facing files, functions, classes, and methods.

Makes the Fonts API non-functional, i.e. as Font Face replaces it.

Why?

Font Face replaces Fonts API.

For sites using the Fonts API, this PR:

  1. Avoids those sites experiencing a fatal parsing error.
  2. Makes the Fonts API non-functional to avoid conflicts with the new font management approach.

How?

  1. Copies all of the Fonts API's files into a new fonts/bc-layer directory.
  2. Removes all private methods.
  3. Removes all functionality from remaining functions and methods.
  4. Deprecates all of the files, classes, functions, and methods.
  5. Loads the files into memory (to avoid fatal parsing errors on sites using it).

Testing Instructions

Set up

  1. Enable the loading of the Font Face files:
    a. Open lib/load.php.
    b. Scroll down to where the Font Face files are loaded.
    c. Add true || in the if:
if ( true || class_exists( 'WP_Fonts_Library' ) || class_exists( 'WP_Fonts_Library_Controller' ) ) {
  1. Plugin using the Fonts API:
                 if ( ! function_exists( 'wp_register_font_provider' ) || ! function_exists( 'wp_register_fonts' ) ) {
                        return;
                }

Reproduce the fatal error

Before applying the changes from this PR (or comment out the BC Layer files loading in lib/load.php, refresh the admin page on your test site.

Expected: Fatal error such as:

Fatal error: Uncaught Error: Call to undefined function wp_register_font_provider() in ../wp-content/plugins/webfonts-jetpack-test/webfonts.php:79

the path to the webfonts.php will be different in your test site.

This fatal error happens because the tester plugin is using the Fonts API but it is not loaded into memory.

Test this PR

  1. Apply the changes from this PR to your local testing site.
  2. Refresh the admin page.
    Expected: No fatal error. There will be deprecation notices as the tester plugin is using the Fonts API (which is now deprecated).
  3. Check that none of the Google Fonts from the tester plugin are in the font pickers:
    a. Go to Site Editor > Styles > Typography > Headings.
    b. Select "FONT".
    Expected: Google Fonts such as Arvo, Merriweather, Open Sans, etc. are not in the list. Rather, only the theme's fonts in the list.

Using TT3, the FONT picker should contain:

  • Default
  • DM Sans
  • IBM Plex Mono
  • Inter
  • System Font
  • Source Serif Pro
    Screen Shot 2023-07-11 at 3 48 19 PM

Note: To see all of the Google Fonts the tester plugin registers, remove the true so that the Font Face libs are not loaded.

@hellofromtonya hellofromtonya self-assigned this Jul 10, 2023
@hellofromtonya hellofromtonya added [Feature] Fonts API [Feature] Typography Font and typography-related issues and PRs labels Jul 10, 2023
@WordPress WordPress deleted a comment from github-actions bot Jul 11, 2023
With Font Face, the Fonts API is no longer needed. However for sites using Fonts API, the public facing functions, classes, and methods still need to be available in memory to avoid fatal errors.

This commit:
1. Adds a bc-layer into the Font Face.
2. Copies each of the Fonts API files into it.
3. Deprecates each file and public function, class, and method.
4. Makes each function and method non-functional.
@hellofromtonya hellofromtonya changed the title Deprecate Fonts API. Make non-functional. Deprecate and make Fonts API non-functional. Jul 11, 2023
@WordPress WordPress deleted a comment from github-actions bot Jul 11, 2023
@hellofromtonya hellofromtonya marked this pull request as ready for review July 11, 2023 20:55
@hellofromtonya hellofromtonya requested review from anton-vlasenko and aristath and removed request for spacedmonkey July 11, 2023 20:55
@anton-vlasenko
Copy link
Contributor

anton-vlasenko commented Jul 13, 2023

Test Report

PR tested: #52485

Environment

  • WordPress: 6.3-beta3-56130-src
  • PHP: 8.0.27
  • Server: Apache/2.4.55 (Unix) PHP/8.0.27
  • Database: mysqli (Server: 5.7.38 / Client: mysqlnd 8.0.27)
  • Browser: Safari 16.5 (macOS)
  • Theme: Twenty Twenty-Three 1.1
  • MU-Plugins: None activated
  • Plugins:
    • Fonts API Test Plugin 1.0.0-ironprogrammer
    • Gutenberg 16.2.0-rc.1

Steps to Test

Follow the steps listed in the PR's description above.

Expected Results

  1. Fatal error
Uncaught Error: Class 'WP_Fonts_Resolver' not found

occurs before applying the changes from this PR.
2. The font picker should contain the expected fonts.

Actual Results

  1. ❌ Fatal error Uncaught Error: Class 'WP_Fonts_Resolver' not found doesn't occur whether using the trunk branch or commenting out the BC layer files loading in load.php.
  2. ✅ The font picker contains the expected fonts.

Supplemental Artifacts

Screenshot 2023-07-14 at 01 07 30

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code LGTM.
However, I'm not sure why the fatal error doesn't occur when the BC layer files are not loaded. Please see my test report here: #52485 (comment).

Copy link
Member

@aristath aristath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM 👍

@hellofromtonya
Copy link
Contributor Author

The code LGTM.
However, I'm not sure why the fatal error doesn't occur when the BC layer files are not loaded. Please see my test report here: #52485 (comment).

Oh silly me 🤦‍♀️ Thanks @anton-vlasenko for flagging this. I missed a step in the Set up process and the fatal error is a different one.

I've updated the testing instructions. The tester plugin already guards against the functions not existing. By commenting out those lines of code, a fatal error will happen when it attempts to invoke wp_register_font_provider() when it does not exist in memory.

@anton-vlasenko
Copy link
Contributor

I've updated the testing instructions. The tester plugin already guards against the functions not existing. By commenting out those lines of code, a fatal error will happen when it attempts to invoke wp_register_font_provider() when it does not exist in memory.

Thank you, @hellofromtonya. I'm able to reproduce the fatal error now.

Copy link
Contributor

@anton-vlasenko anton-vlasenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✅.

@hellofromtonya
Copy link
Contributor Author

Thank you @aristath and @anton-vlasenko 🎉 Merging now.

@hellofromtonya hellofromtonya merged commit 36f2288 into trunk Jul 19, 2023
48 checks passed
@hellofromtonya hellofromtonya deleted the try/deprecate-fonts-api branch July 19, 2023 14:06
@github-actions github-actions bot added this to the Gutenberg 16.3 milestone Jul 19, 2023
@mburridge mburridge added the [Type] Bug An existing feature does not function as intended label Jul 19, 2023
@mikachan mikachan added the Needs PHP backport Needs PHP backport to Core label Sep 5, 2023
@hellofromtonya hellofromtonya removed the Needs PHP backport Needs PHP backport to Core label Sep 6, 2023
@hellofromtonya
Copy link
Contributor Author

Removed Needs PHP backport label as this PR will not be backported to Core. The Fonts API is deprecated within the plugin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Fonts API] Deprecate and remove all functionality (in favor of Font Face)
5 participants