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 Address Outstanding Feedback from WP 6.4 review #58675

Merged
merged 8 commits into from
Feb 5, 2024
Merged

Font Library Address Outstanding Feedback from WP 6.4 review #58675

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
20cbc75
Translate error string
getdave Feb 5, 2024
4f846fa
Improve get_allowed_font_mime_types comment
creativecoder Feb 5, 2024
82c04d5
Use str_contains rather than strpos
creativecoder Feb 5, 2024
e187b03
Move period outside double quotes
getdave Feb 5, 2024
ab62473
Correct error message
creativecoder Feb 5, 2024
f6f513f
Use sprintf for parameter names in translated strings
creativecoder Feb 5, 2024
59e6fde
Capitalize HTTP
creativecoder Feb 5, 2024
28bbc36
Updates tests
creativecoder Feb 5, 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
14 changes: 7 additions & 7 deletions lib/compat/wordpress-6.5/fonts/class-wp-font-collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,10 +48,10 @@ final class WP_Font_Collection {
*
* @since 6.5.0
*
* @param string $slug Font collection slug.
* @param array|string $data_or_file Font collection data array or a path/URL to a JSON file
* containing the font collection.
* See {@see wp_register_font_collection()} for the supported fields.
* @param string $slug Font collection slug.
* @param array|string $data_or_file Font collection data array or a path/URL to a JSON file
* containing the font collection.
* See {@see wp_register_font_collection()} for the supported fields.
*/
public function __construct( $slug, $data_or_file ) {
$this->slug = sanitize_title( $slug );
Expand Down Expand Up @@ -162,7 +162,7 @@ private function load_from_url( $url ) {

$data = json_decode( wp_remote_retrieve_body( $response ), true );
if ( empty( $data ) ) {
return new WP_Error( 'font_collection_decode_error', __( 'Error decoding the font collection data from the REST response JSON.', 'gutenberg' ) );
return new WP_Error( 'font_collection_decode_error', __( 'Error decoding the font collection data from the HTTP response JSON.', 'gutenberg' ) );
}

// Make sure the data is valid before storing it in a transient.
Expand Down Expand Up @@ -193,8 +193,8 @@ private function sanitize_and_validate_data( $data ) {
foreach ( $required_properties as $property ) {
if ( empty( $data[ $property ] ) ) {
$message = sprintf(
// translators: 1: Font collection slug, 2: Missing property name.
__( 'Font collection "%1$s" has missing or empty property: "%2$s."', 'gutenberg' ),
// translators: 1: Font collection slug, 2: Missing property name, e.g. "font_families".
__( 'Font collection "%1$s" has missing or empty property: "%2$s".', 'gutenberg' ),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feedback from https://wordpress.slack.com/archives/C02QB2JS7/p1707143390995019

$this->slug,
$property
);
Expand Down
2 changes: 1 addition & 1 deletion lib/compat/wordpress-6.5/fonts/class-wp-font-library.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ public function get_font_collection( $slug ) {
if ( array_key_exists( $slug, $this->collections ) ) {
return $this->collections[ $slug ];
}
return new WP_Error( 'font_collection_not_found', 'Font collection not found.' );
return new WP_Error( 'font_collection_not_found', __( 'Font collection not found.', 'gutenberg' ) );
}

/**
Expand Down
12 changes: 8 additions & 4 deletions lib/compat/wordpress-6.5/fonts/class-wp-font-utils.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public static function sanitize_font_family( $font_family ) {
$wrapped_font_families = array_map(
function ( $family ) {
$trimmed = trim( $family );
if ( ! empty( $trimmed ) && false !== strpos( $trimmed, ' ' ) && false === strpos( $trimmed, "'" ) && false === strpos( $trimmed, '"' ) ) {
if ( ! empty( $trimmed ) && str_contains( $trimmed, ' ' ) && ! str_contains( $trimmed, "'" ) && ! str_contains( $trimmed, '"' ) ) {
return '"' . $trimmed . '"';
}
return $trimmed;
Expand Down Expand Up @@ -215,13 +215,17 @@ private static function apply_sanitizer( $value, $sanitizer ) {
}

/**
* Provide the expected mime-type value for font files per-PHP release. Due to differences in the values returned these values differ between PHP versions.
* Returns the expected mime-type values for font files, depending on PHP version.
*
* This is necessary until a collection of valid mime-types per-file extension can be provided to 'upload_mimes' filter.
* This is needed because font mime types vary by PHP version, so checking the PHP version
* is necessary until a list of valid mime-types for each file extension can be provided to
* the 'upload_mimes' filter.
*
* @since 6.5.0
*
* @return Array A collection of mime types keyed by file extension.
* @access private
getdave marked this conversation as resolved.
Show resolved Hide resolved
*
* @return array A collection of mime types keyed by file extension.
*/
public static function get_allowed_font_mime_types() {
$php_7_ttf_mime_type = PHP_VERSION_ID >= 70300 ? 'application/font-sfnt' : 'application/x-font-ttf';
Expand Down
2 changes: 1 addition & 1 deletion lib/compat/wordpress-6.5/fonts/class-wp-rest-font-collections-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ public function get_items_permissions_check( $request ) { // phpcs:ignore Variab

return new WP_Error(
'rest_cannot_read',
__( 'Sorry, you are not allowed to use the Font Library on this site.', 'gutenberg' ),
__( 'Sorry, you are not allowed to access font collections.', 'gutenberg' ),
array(
'status' => rest_authorization_required_code(),
)
Expand Down
19 changes: 10 additions & 9 deletions lib/compat/wordpress-6.5/fonts/class-wp-rest-font-faces-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,8 @@ public function validate_create_font_face_settings( $value, $request ) {
if ( isset( $settings[ $key ] ) && ! $settings[ $key ] ) {
return new WP_Error(
'rest_invalid_param',
/* translators: %s: Font family setting key. */
sprintf( __( 'font_face_setting[%s] cannot be empty.', 'gutenberg' ), $key ),
/* translators: %s: Name of the missing font face settings parameter, e.g. "font_face_settings[src]". */
sprintf( __( '%s cannot be empty.', 'gutenberg' ), "font_face_setting[ $key ]" ),
array( 'status' => 400 )
);
}
Expand All @@ -196,7 +196,8 @@ public function validate_create_font_face_settings( $value, $request ) {
if ( empty( $src ) ) {
return new WP_Error(
'rest_invalid_param',
__( 'font_face_settings[src] values must be non-empty strings.', 'gutenberg' ),
/* translators: %s: Font face source parameter name: "font_face_settings[src]". */
sprintf( __( '%s values must be non-empty strings.', 'gutenberg' ), 'font_face_settings[src]' ),
array( 'status' => 400 )
);
}
Expand All @@ -205,8 +206,8 @@ public function validate_create_font_face_settings( $value, $request ) {
if ( false === wp_http_validate_url( $src ) && ! isset( $files[ $src ] ) ) {
return new WP_Error(
'rest_invalid_param',
/* translators: %s: src value in the font face settings. */
sprintf( __( 'font_face_settings[src] value "%s" must be a valid URL or file reference.', 'gutenberg' ), $src ),
/* translators: 1: Font face source parameter name: "font_face_settings[src]", 2: The invalid src value. */
sprintf( __( '%1$s value "%2$s" must be a valid URL or file reference.', 'gutenberg' ), 'font_face_settings[src]', $src ),
array( 'status' => 400 )
);
}
Expand All @@ -217,8 +218,8 @@ public function validate_create_font_face_settings( $value, $request ) {
if ( ! in_array( $file, $srcs, true ) ) {
return new WP_Error(
'rest_invalid_param',
// translators: %s: File key (e.g. `file-0`) in the request data.
sprintf( __( 'File %1$s must be used in font_face_settings[src].', 'gutenberg' ), $file ),
/* translators: 1: File key (e.g. "file-0") in the request data, 2: Font face source parameter name: "font_face_settings[src]". */
sprintf( __( 'File %1$s must be used in %2$s.', 'gutenberg' ), $file, 'font_face_settings[src]' ),
array( 'status' => 400 )
);
}
Expand Down Expand Up @@ -291,7 +292,7 @@ public function get_item( $request ) {
return new WP_Error(
'rest_font_face_parent_id_mismatch',
/* translators: %d: A post id. */
sprintf( __( 'The font face does not belong to the specified font family with id of "%d"', 'gutenberg' ), $font_family->ID ),
sprintf( __( 'The font face does not belong to the specified font family with id of "%d".', 'gutenberg' ), $font_family->ID ),
array( 'status' => 404 )
);
}
Expand Down Expand Up @@ -405,7 +406,7 @@ public function delete_item( $request ) {
return new WP_Error(
'rest_font_face_parent_id_mismatch',
/* translators: %d: A post id. */
sprintf( __( 'The font face does not belong to the specified font family with id of "%d"', 'gutenberg' ), $font_family->ID ),
sprintf( __( 'The font face does not belong to the specified font family with id of "%d".', 'gutenberg' ), $font_family->ID ),
array( 'status' => 404 )
);
}
Expand Down
10 changes: 6 additions & 4 deletions lib/compat/wordpress-6.5/fonts/class-wp-rest-font-families-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ public function validate_font_family_settings( $value, $request ) {
if ( null === $settings ) {
return new WP_Error(
'rest_invalid_param',
__( 'font_family_settings parameter must be a valid JSON string.', 'gutenberg' ),
/* translators: %s: Parameter name: "font_family_settings". */
sprintf( __( '%s parameter must be a valid JSON string.', 'gutenberg' ), 'font_family_settings' ),
array( 'status' => 400 )
);
}
Expand All @@ -102,7 +103,8 @@ public function validate_font_family_settings( $value, $request ) {
if ( isset( $settings['slug'] ) ) {
return new WP_Error(
'rest_invalid_param',
__( 'font_family_settings[slug] cannot be updated.', 'gutenberg' ),
/* translators: %s: Name of parameter being updated: font_family_settings[slug]". */
sprintf( __( '%s cannot be updated.', 'gutenberg' ), 'font_family_settings[slug]' ),
array( 'status' => 400 )
);
}
Expand All @@ -121,8 +123,8 @@ public function validate_font_family_settings( $value, $request ) {
if ( isset( $settings[ $key ] ) && ! $settings[ $key ] ) {
return new WP_Error(
'rest_invalid_param',
/* translators: %s: Font family setting key. */
sprintf( __( 'font_family_settings[%s] cannot be empty.', 'gutenberg' ), $key ),
/* translators: %s: Name of the empty font family setting parameter, e.g. "font_family_settings[slug]". */
sprintf( __( '%s cannot be empty.', 'gutenberg' ), "font_family_settings[ $key ]" ),
array( 'status' => 400 )
);
}
Expand Down
4 changes: 2 additions & 2 deletions phpunit/tests/fonts/font-library/wpRestFontFacesController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ public function test_get_item_invalid_parent_id() {
$response = rest_get_server()->dispatch( $request );
$this->assertErrorResponse( 'rest_font_face_parent_id_mismatch', $response, 404 );

$expected_message = 'The font face does not belong to the specified font family with id of "' . self::$other_font_family_id . '"';
$expected_message = 'The font face does not belong to the specified font family with id of "' . self::$other_font_family_id . '".';
$this->assertSame( $expected_message, $response->as_error()->get_error_messages()[0], 'The message must contain the correct parent ID.' );
}

Expand Down Expand Up @@ -928,7 +928,7 @@ public function test_delete_item_invalid_parent_id() {
$response = rest_get_server()->dispatch( $request );
$this->assertErrorResponse( 'rest_font_face_parent_id_mismatch', $response, 404, 'The response should return an error for "rest_font_face_parent_id_mismatch" with 404 status.' );

$expected_message = 'The font face does not belong to the specified font family with id of "' . self::$other_font_family_id . '"';
$expected_message = 'The font face does not belong to the specified font family with id of "' . self::$other_font_family_id . '".';
$this->assertSame( $expected_message, $response->as_error()->get_error_messages()[0], 'The message must contain the correct parent ID.' );
}

Expand Down
Loading