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.

Sign up for GitHub

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

Jump to bottom

Font Library: add global configuration variables for font directory #57044

Closed
wants to merge 4 commits into from
Closed

Font Library: add global configuration variables for font directory #57044

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c31497a
add global configuration variables for font directory
madhusudhand Dec 14, 2023
d6f0a0e
add multi-site based directory path for fonts
madhusudhand Jan 8, 2024
1976369
Merge branch 'trunk' into font-lib/add-config-variable
matiasbenedetto Jan 9, 2024
b48887a
add docblock for get_multi_site_font_sub_dir
matiasbenedetto Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 22 additions & 9 deletions lib/experimental/fonts/font-library/class-wp-font-library.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,21 @@ public static function get_font_collection( $id ) {
return new WP_Error( 'font_collection_not_found', 'Font collection not found.' );
}

/**
* Gets a multi site sub-dir.
*
* @since 6.5.0
*
* @return string multi site sub-dir path.
*/
private static function get_multi_site_font_sub_dir() {
$font_sub_dir = '';
if ( is_multisite() && ! ( is_main_network() && is_main_site() ) ) {
$font_sub_dir = '/sites/' . get_current_blog_id();
}
return $font_sub_dir;
}

/**
* Gets the upload directory for fonts.
*
Expand All @@ -148,7 +163,9 @@ public static function get_font_collection( $id ) {
* @return string Path of the upload directory for fonts.
*/
public static function get_fonts_dir() {
return path_join( WP_CONTENT_DIR, 'fonts' );
$base_font_dir = ( defined( 'WP_FONT_DIR' ) && ! empty( WP_FONT_DIR ) ) ? WP_FONT_DIR : path_join( WP_CONTENT_DIR, 'fonts' );
$font_sub_dir = self::get_multi_site_font_sub_dir();
return rtrim( $base_font_dir, '/' ) . $font_sub_dir;
}

/**
Expand All @@ -161,19 +178,15 @@ public static function get_fonts_dir() {
*
* @type string $path Path to the directory.
* @type string $url URL for the directory.
* @type string $subdir Sub-directory of the directory.
* @type string $basedir Base directory.
* @type string $baseurl Base URL.
* }
* @return array Modified upload directory.
*/
public static function set_upload_dir( $defaults ) {
$defaults['basedir'] = WP_CONTENT_DIR;
$defaults['baseurl'] = content_url();
$defaults['subdir'] = '/fonts';
Comment on lines -171 to -173
Copy link
Contributor

@creativecoder creativecoder Dec 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is anything lost by not setting basedir, baseurl, and subdir here? It seems like unexpected things could happen if some of the properties for upload_dir are changed and some are not, but I haven't used this filter before.

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on the definition of wp_upload_dir, when path is not set, basedir + subdir are used, and when url is not set, baseurl + subdir are used. I think we are not loosing anything here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Alternatively, we could switch to basedir + subdir where basedir becomes the path set by the variable WP_FONT_DIR and subdir will be the site_id (in case of multi site) and empty for simple sites.
Please refer this

$defaults['path'] = self::get_fonts_dir();
$defaults['url'] = $defaults['baseurl'] . '/fonts';
$font_url = ( defined( 'WP_FONT_URL' ) && ! empty( WP_FONT_URL ) ) ? set_url_scheme( WP_FONT_URL ) : content_url( 'fonts' );
$font_sub_dir = self::get_multi_site_font_sub_dir();

$defaults['path'] = self::get_fonts_dir();
$defaults['url'] = untrailingslashit( $font_url ) . $font_sub_dir;
return $defaults;
}

Expand Down
14 changes: 4 additions & 10 deletions phpunit/tests/fonts/font-library/wpFontLibrary/setUploadDir.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,12 @@ class Tests_Fonts_WpFontLibrary_SetUploadDir extends WP_Font_Library_UnitTestCas

public function test_should_set_fonts_upload_dir() {
$defaults = array(
'subdir' => '/abc',
'basedir' => '/any/path',
'baseurl' => 'http://example.com/an/arbitrary/url',
'path' => '/any/path/abc',
'url' => 'http://example.com/an/arbitrary/url/abc',
'path' => '/any/path/abc',
'url' => 'http://example.com/an/arbitrary/url/abc',
);
$expected = array(
'subdir' => '/fonts',
'basedir' => WP_CONTENT_DIR,
'baseurl' => content_url(),
'path' => path_join( WP_CONTENT_DIR, 'fonts' ),
'url' => content_url() . '/fonts',
'path' => path_join( WP_CONTENT_DIR, 'fonts' ),
'url' => content_url( 'fonts' ),
);
$this->assertSame( $expected, WP_Font_Library::set_upload_dir( $defaults ) );
}
Expand Down
Loading