Add a databse persisted fallback for the default font folder path #6252

matiasbenedetto · 2024-03-11T20:35:24Z

What?

Add a fallback for the default font directory persisted in the database as a site option.
There's an alternative PR without database persistence: #6259

Why?

For installations that don’t support modification of the wp-content directory, the Font Library will use wp-content/uploads/fonts as a fallback location, ensuring we stay true to our project philosophy of designing for the majority while still making the feature available to anyone out of the box without extra steps from the user.

Decision from https://make.wordpress.org/core/2024/03/07/unblocking-wp6-5-font-library-and-synced-pattern-overrides/
Discussion: WordPress/gutenberg#59699

How?

Add the wp_default_font_dir function.
If /wp-content/fonts is writable this function should return:

array(
    'path' => '/var/www/src/wp-content/fonts',
    'url' => 'http://localhost:8889/wp-content/fonts',
    'subdir' => ''
    'basedir' => '/var/www/src/wp-content/fonts',
    'baseurl' => 'http://localhost:8889/wp-content/fonts'
    'error' => false
)

If /wp-content/fonts is not writable this function should return:

array(
    'path' => '/var/www/src/wp-content/uploads/fonts',
    'url' => 'http://localhost:8889/wp-content/uploads/fonts',
    'subdir' => ''
    'basedir' => '/var/www/src/wp-content/uploads/fonts',
    'baseurl' => 'http://localhost:8889/wp-content/uploads/fonts'
    'error' => false
)

The output of this function is persisted in the database as a site_option. To refresh the value persisted the wp_default_font_dir function should be called like this wp_default_font_dir( true ).

Trac ticket: https://core.trac.wordpress.org/ticket/60751#ticket

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

github-actions · 2024-03-11T20:35:41Z

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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props mmaattiiaass, swissspidy.

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

matiasbenedetto · 2024-03-11T20:39:57Z

tests/phpunit/tests/fonts/wpDefaultFontDir.php

+		$font_dir = wp_default_font_dir( true );
+		chmod( $font_dir['path'], 0000 );


I haven't found a way to restrict the writing permissions in the context of unit tests. This chmod line isn't working. It is there just to show how the functionality could be tested.

No idea how to test this either. If the option is changed to record the fallback rather than the directory structure then you migh be able to filter it using the pre_option_{$option} hook.

github-actions · 2024-03-11T20:54:34Z

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

The Plugin and Theme Directories cannot be accessed within Playground.
All changes will be lost when closing a tab with a Playground instance.
All changes will be lost when refreshing the page.
A fresh instance is created each time the link below is clicked.
Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

src/wp-includes/fonts.php

matiasbenedetto · 2024-03-12T14:14:51Z

src/wp-includes/fonts.php

 */
-function wp_get_font_dir() {
+function wp_default_font_dir( $refresh_cache = false ) {
+	$defaults = get_option( 'font_dir' );


I'm not 100% sure if there are significant benefits in making this persistent. Related: WordPress/gutenberg#59699 (comment)

An non-persisted alternative PR to this one: #6259

Rather than store the entire font directory in the option, I'd suggest storing whether it falls back to the secondary location.

This will prevent future contributors from having to manage three scenarios:

default

fallback

option set

It's already complicated enough without allowing extenders to change the path via either an option or a filter.

peterwilsoncc

Some notes for this option too.

peterwilsoncc · 2024-03-12T23:53:19Z

src/wp-includes/fonts.php

 */
-function wp_get_font_dir() {
+function wp_default_font_dir( $refresh_cache = false ) {
+	$defaults = get_option( 'font_dir' );


Rather than store the entire font directory in the option, I'd suggest storing whether it falls back to the secondary location.

This will prevent future contributors from having to manage three scenarios:

default

fallback

option set

It's already complicated enough without allowing extenders to change the path via either an option or a filter.

peterwilsoncc · 2024-03-12T23:54:15Z

src/wp-includes/fonts.php

+	wp_mkdir_p( $defaults['path'] );
+
+	if ( ! wp_is_writable( $defaults['path'] ) ) {
+		$defaults            = wp_upload_dir();
+		$defaults['path']    = path_join( $defaults['basedir'], 'fonts' );
+		$defaults['url']     = $defaults['baseurl'] . '/fonts';
+		$defaults['subdir']  = '';
+		$defaults['basedir'] = path_join( $defaults['basedir'], 'fonts' );
+		$defaults['baseurl'] = $defaults['baseurl'] . '/fonts';
+	}
+


Notes from #6259 (review) apply here too.

peterwilsoncc · 2024-03-12T23:59:23Z

tests/phpunit/tests/fonts/wpDefaultFontDir.php

+		$font_dir = wp_default_font_dir( true );
+		chmod( $font_dir['path'], 0000 );


No idea how to test this either. If the option is changed to record the fallback rather than the directory structure then you migh be able to filter it using the pre_option_{$option} hook.

peterwilsoncc · 2024-03-21T23:02:40Z

Closing this PR off per https://make.wordpress.org/core/2024/03/21/font-library-update-storage-of-font-files/. Thanks everyone for your work on this, I appreciate it.

Add fallback for the default font folder path

6e1e8e0

matiasbenedetto added 2 commits March 11, 2024 17:36

improve dir is_writable check

0fb2cec

format php

7a46783

matiasbenedetto commented Mar 11, 2024

View reviewed changes

matiasbenedetto mentioned this pull request Mar 11, 2024

Architectural discussion allowing for fonts folder fallbacks. WordPress/gutenberg#59699

Closed

swissspidy requested review from desrosj, peterwilsoncc and swissspidy March 12, 2024 07:37

swissspidy reviewed Mar 12, 2024

View reviewed changes

src/wp-includes/fonts.php Outdated Show resolved Hide resolved

src/wp-includes/fonts.php Outdated Show resolved Hide resolved

src/wp-includes/fonts.php Outdated Show resolved Hide resolved

matiasbenedetto added 5 commits March 12, 2024 10:56

remove unwanted early return

b85cdf3

re-order actual and expected

b8e35cc

remove not needed assignement

4f4e63c

do not suppres errors

e655888

format php

8e23ae4

matiasbenedetto commented Mar 12, 2024

View reviewed changes

use the same format as in the wp-content/fonts default path

25d3c80

matiasbenedetto mentioned this pull request Mar 12, 2024

Add fallback for the default font folder path #6259

Closed

matiasbenedetto changed the title ~~Add fallback for the default font folder path~~ Add a databse persisted fallback for the default font folder path Mar 12, 2024

peterwilsoncc reviewed Mar 12, 2024

View reviewed changes

peterwilsoncc closed this Mar 21, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a databse persisted fallback for the default font folder path #6252

Add a databse persisted fallback for the default font folder path #6252

matiasbenedetto commented Mar 11, 2024 •

edited

Loading

github-actions bot commented Mar 11, 2024 •

edited

Loading

matiasbenedetto Mar 11, 2024

peterwilsoncc Mar 12, 2024

github-actions bot commented Mar 11, 2024

matiasbenedetto Mar 12, 2024

matiasbenedetto Mar 12, 2024

peterwilsoncc Mar 12, 2024

peterwilsoncc left a comment

peterwilsoncc Mar 12, 2024

peterwilsoncc Mar 12, 2024

peterwilsoncc Mar 12, 2024

peterwilsoncc commented Mar 21, 2024

		$font_dir = wp_default_font_dir( true );
		chmod( $font_dir['path'], 0000 );

Add a databse persisted fallback for the default font folder path #6252

Add a databse persisted fallback for the default font folder path #6252

Conversation

matiasbenedetto commented Mar 11, 2024 • edited Loading

What?

Why?

How?

github-actions bot commented Mar 11, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Mar 11, 2024

Test using WordPress Playground

Some things to be aware of

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peterwilsoncc left a 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

peterwilsoncc commented Mar 21, 2024

matiasbenedetto commented Mar 11, 2024 •

edited

Loading

github-actions bot commented Mar 11, 2024 •

edited

Loading