-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add a databse persisted fallback for the default font folder path #6252
Changes from all commits
6e1e8e0
0fb2cec
7a46783
b85cdf3
b8e35cc
4f4e63c
e655888
8e23ae4
25d3c80
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,22 +92,20 @@ function wp_unregister_font_collection( string $slug ) { | |
} | ||
|
||
/** | ||
* Returns an array containing the current fonts upload directory's path and URL. | ||
* Returns the default font directory path and URL. | ||
* | ||
* @since 6.5.0 | ||
* | ||
* @return array $defaults { | ||
* Array of information about the upload directory. | ||
* | ||
* @type string $path Base directory and subdirectory or full path to the fonts upload directory. | ||
* @type string $url Base URL and subdirectory or absolute URL to the fonts upload directory. | ||
* @type string $subdir Subdirectory | ||
* @type string $basedir Path without subdir. | ||
* @type string $baseurl URL path without subdir. | ||
* @type string|false $error False or error message. | ||
* } | ||
* @param bool $refresh_cache Optional. Whether to refresh the cache and generate a new default font directory path and URL. Default is false. | ||
* @return array Default font directory path and URL. | ||
*/ | ||
function wp_get_font_dir() { | ||
function wp_default_font_dir( $refresh_cache = false ) { | ||
$defaults = get_option( 'font_dir' ); | ||
|
||
if ( ! empty( $defaults ) && ! $refresh_cache ) { | ||
return $defaults; | ||
} | ||
|
||
$site_path = ''; | ||
if ( is_multisite() && ! ( is_main_network() && is_main_site() ) ) { | ||
$site_path = '/sites/' . get_current_blog_id(); | ||
|
@@ -122,6 +120,40 @@ function wp_get_font_dir() { | |
'error' => false, | ||
); | ||
|
||
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'; | ||
} | ||
|
||
Comment on lines
+123
to
+133
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Notes from #6259 (review) apply here too. |
||
update_option( 'font_dir', $defaults ); | ||
return $defaults; | ||
} | ||
|
||
/** | ||
* Returns an array containing the current fonts upload directory's path and URL. | ||
* | ||
* @since 6.5.0 | ||
* | ||
* @return array $defaults { | ||
* Array of information about the upload directory. | ||
* | ||
* @type string $path Base directory and subdirectory or full path to the fonts upload directory. | ||
* @type string $url Base URL and subdirectory or absolute URL to the fonts upload directory. | ||
* @type string $subdir Subdirectory | ||
* @type string $basedir Path without subdir. | ||
* @type string $baseurl URL path without subdir. | ||
* @type string|false $error False or error message. | ||
* } | ||
*/ | ||
function wp_get_font_dir() { | ||
$defaults = wp_default_font_dir( true ); | ||
|
||
/** | ||
* Filters the fonts directory data. | ||
* | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
<?php | ||
/** | ||
* Test wp_default_font_dir(). | ||
* | ||
* @package WordPress | ||
* @subpackage Font Library | ||
* | ||
* @group fonts | ||
* @group font-library | ||
* | ||
* @covers ::wp_default_font_dir | ||
*/ | ||
class Tests_Fonts_WpDefaultFontDir extends WP_UnitTestCase { | ||
|
||
public function test_default_font_dir() { | ||
$font_dir = wp_default_font_dir(); | ||
|
||
$this->assertSame( | ||
array( | ||
'path' => WP_CONTENT_DIR . '/fonts', | ||
'url' => content_url( 'fonts' ), | ||
'subdir' => '', | ||
'basedir' => WP_CONTENT_DIR . '/fonts', | ||
'baseurl' => content_url( 'fonts' ), | ||
'error' => false, | ||
), | ||
$font_dir, | ||
'The font directory should be a dir inside wp-content' | ||
); | ||
} | ||
|
||
public function test_uploads_font_dir() { | ||
$font_dir = wp_default_font_dir( true ); | ||
chmod( $font_dir['path'], 0000 ); | ||
Comment on lines
+33
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't found a way to restrict the writing permissions in the context of unit tests. This There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
|
||
$upload_dir = wp_upload_dir(); | ||
|
||
$this->assertSame( | ||
array( | ||
'path' => path_join( $upload_dir['basedir'], 'fonts' ), | ||
'url' => $upload_dir['baseurl'] . '/fonts', | ||
'subdir' => '', | ||
'basedir' => path_join( $upload_dir['basedir'], 'fonts' ), | ||
'baseurl' => $upload_dir['baseurl'] . '/fonts', | ||
'error' => false, | ||
), | ||
$font_dir, | ||
'The font directory should be a subdir in the uploads directory.' | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure if there are significant benefits in making this persistent. Related: WordPress/gutenberg#59699 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An non-persisted alternative PR to this one: #6259
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
It's already complicated enough without allowing extenders to change the path via either an option or a filter.