Skip to content

Commit

Permalink
Restore font family in variation required
Browse files Browse the repository at this point in the history 
Restores `font-family` in variation requirement
when registering a variation. The restore includes
the notice being thrown when not defined.
  • Loading branch information
@hellofromtonya @aristath
hellofromtonya authored and aristath committed Oct 3, 2022
1 parent b36017b commit 2497b63
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 16 deletions.
14 changes: 8 additions & 6 deletions lib/experimental/class-wp-webfonts.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function add_variation( $font_family_handle, array $variation, $variation
}
}

$variation = $this->validate_variation( $font_family_handle, $variation );
$variation = $this->validate_variation( $variation );

// Variation validation failed.
if ( ! $variation ) {
Expand Down Expand Up @@ -318,14 +318,16 @@ private function add_dependency( $font_family_handle, $variation_handle ) {
*
* @since X.X.X
*
* @param string $font_family The font family for this variation.
* @param array $variation An array of variation properties to add.
* @param array $variation Variation properties to add.
* @return false|array Validated variation on success. Else, false.
*/
private function validate_variation( $font_family, $variation ) {
private function validate_variation( $variation ) {
$variation = wp_parse_args( $variation, $this->variation_property_defaults );
if ( empty( $variation['font-family'] ) ) {
$variation['font-family'] = $font_family;

// Check the font-family.
if ( empty( $variation['font-family'] ) || ! is_string( $variation['font-family'] ) ) {
trigger_error( 'Webfont font-family must be a non-empty string.' );
return false;
}

// Local fonts need a "src".
Expand Down
114 changes: 110 additions & 4 deletions phpunit/webfonts/wp-webfonts-tests-dataset.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,17 @@ public function data_valid_variation() {
'expected' => 'lato-400-normal',
'font_family_handle' => 'lato',
'variation' => array(
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
'font-family' => 'Lato',
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
),
),

'font-weight and font-style not defined; with variation handle' => array(
'expected' => 'my-custom-handle',
'font_family_handle' => 'lato',
'variation' => array(
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
'font-family' => 'Lato',
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
),
'variation_handle' => 'my-custom-handle',
),
Expand Down Expand Up @@ -103,11 +105,12 @@ public function data_valid_variation() {
}

/**
* Data provider for testing registration of variations where the font family is not defined.
* Data provider for testing registration of variations where the font family handle
* is not defined.
*
* @return array
*/
public function data_font_family_not_define_in_variation() {
public function data_font_family_handle_undefined() {
return array(
'empty string font family handle' => array(
'font_family_handle' => '',
Expand Down Expand Up @@ -142,6 +145,56 @@ public function data_font_family_not_define_in_variation() {
);
}

/**
* Data provider for testing registration of variations where the font family is not defined
* in the variation.
*
* @return array
*/
public function data_font_family_undefined_in_variation() {
$message = 'Webfont font-family must be a non-empty string.';
return array(
'font-family: undefined' => array(
'font_family_handle' => 'lato',
'variation' => array(
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
),
'expected_message' => $message,
),
'font-family: null' => array(
'font_family_handle' => 'Lato',
'variation' => array(
'provider' => 'local',
'font-family' => null,
'font-weight' => '200',
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
),
'expected_message' => $message,
),
'font-family: non string' => array(
'font_family_handle' => 'Source Serif Pro',
'variation' => array(
'provider' => 'local',
'font-family' => 10,
'font-style' => 'normal',
'font-weight' => '200 900',
'font-stretch' => 'normal',
'src' => 'https://example.com/assets/fonts/source-serif-pro/SourceSerif4Variable-Roman.ttf.woff2',
'font-display' => 'fallback',
),
'expected_message' => $message,
),
'font-family: empty string' => array(
'font_family_handle' => 'lato',
'variation' => array(
'font-family' => '',
'src' => 'https://example.com/assets/fonts/lato/lato.ttf.woff2',
),
'expected_message' => $message,
),
);
}

/**
* Data provider for testing when the variation's handle can't be determine from the given input.
*
Expand All @@ -152,6 +205,7 @@ public function data_unable_determine_variation_handle() {
'integer values' => array(
'font_family_handle' => 'lato',
'variation' => array(
'font-family' => 'Lato',
'font-weight' => 400,
'font-style' => 0,
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
Expand All @@ -161,6 +215,7 @@ public function data_unable_determine_variation_handle() {
'with empty string font-weight and font-style' => array(
'font_family_handle' => 'merriweather',
'variation' => array(
'font-family' => 'Merriweather',
'font-weight' => '',
'font-style' => '',
'src' => 'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -170,6 +225,7 @@ public function data_unable_determine_variation_handle() {
'integer font-weight, empty string font-style' => array(
'font_family' => 'merriweather',
'variation' => array(
'font-family' => 'Merriweather',
'font-weight' => 400,
'font-style' => '',
'src' => 'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -179,6 +235,7 @@ public function data_unable_determine_variation_handle() {
'empty string font-weight, integer font-style' => array(
'font_family' => 'lato',
'variation' => array(
'font-family' => 'Merriweather',
'font-weight' => '',
'font-style' => 400,
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
Expand All @@ -195,6 +252,29 @@ public function data_unable_determine_variation_handle() {
*/
public function data_invalid_variation() {
return array(
'font-family: undefined' => array(
'expected' => 'Webfont font-family must be a non-empty string.',
'font_family_handle' => 'lato',
'variation' => array(
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
),
),
'font-family: null' => array(
'expected' => 'Webfont font-family must be a non-empty string.',
'font_family_handle' => 'lato',
'variation' => array(
'font-family' => null,
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
),
),
'font-family: empty string' => array(
'expected' => 'Webfont font-family must be a non-empty string.',
'font_family_handle' => 'lato',
'variation' => array(
'font-family' => '',
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
),
),
'src: undefined' => array(
'expected' => 'Webfont src must be a non-empty string or an array of strings.',
'font_family_handle' => 'lato',
Expand Down Expand Up @@ -229,6 +309,7 @@ public function data_invalid_variation() {
'font_family_handle' => 'lato',
'variation' => array(
'provider' => 'local',
'font-family' => 'Lato',
'font-weight' => '200',
'src' => array(),
),
Expand All @@ -238,6 +319,7 @@ public function data_invalid_variation() {
'font_family_handle' => 'lato',
'variation' => array(
'provider' => 'local',
'font-family' => 'Lato',
'font-weight' => '200',
'src' => array( '' ),
),
Expand All @@ -247,6 +329,7 @@ public function data_invalid_variation() {
'font_family_handle' => 'lato',
'variation' => array(
'provider' => 'local',
'font-family' => 'Lato',
'font-weight' => '200',
'src' => array( null ),
),
Expand All @@ -256,6 +339,7 @@ public function data_invalid_variation() {
'font_family_handle' => 'lato',
'variation' => array(
'provider' => 'local',
'font-family' => 'Lato',
'font-weight' => '200',
'src' => array(
'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -269,6 +353,7 @@ public function data_invalid_variation() {
'font_family_handle' => 'lato',
'variation' => array(
'provider' => 'local',
'font-family' => 'Lato',
'font-weight' => '200',
'src' => array(
'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -282,13 +367,15 @@ public function data_invalid_variation() {
'font_family_handle' => 'merriweather',
'variation' => array(
'provider' => 'doesnotexit',
'font-family' => 'Merriweather',
'font-weight' => '200 900',
),
),
'font-weight: null' => array(
'expected' => 'Webfont font-weight must be a properly formatted string or integer.',
'font_family_handle' => 'merriweather',
'variation' => array(
'font-family' => 'Merriweather',
'font-weight' => null,
'font-style' => 'normal',
'src' => 'https://example.com/assets/fonts/lato.ttf.woff2',
Expand Down Expand Up @@ -317,6 +404,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
'inputs' => array(
'merriweather' => array(
'merriweather-200-900-normal' => array(
'font-family' => 'Merriweather',
'font-weight' => '200 900',
'font-stretch' => 'normal',
'src' => 'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -332,6 +420,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
'Source Serif Pro' => array(
'Source Serif Pro-300-normal' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'normal',
'font-weight' => '300',
'font-stretch' => 'normal',
Expand All @@ -340,6 +429,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
),
'Source Serif Pro-900-italic' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'italic',
'font-weight' => '900',
'font-stretch' => 'normal',
Expand Down Expand Up @@ -367,6 +457,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
'inputs' => array(
'merriweather' => array(
'merriweather-200-900-normal' => array(
'font-family' => 'Merriweather',
'font-weight' => '200 900',
'font-stretch' => 'normal',
'src' => 'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -375,6 +466,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
'Source Serif Pro' => array(
'Source Serif Pro-300-normal' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'normal',
'font-weight' => '300',
'font-stretch' => 'normal',
Expand All @@ -383,6 +475,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
),
'Source Serif Pro-900-italic' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'italic',
'font-weight' => '900',
'font-stretch' => 'normal',
Expand All @@ -392,6 +485,7 @@ public function data_one_to_many_font_families_and_zero_to_many_variations() {
),
'my-font' => array(
'my-font-300-italic' => array(
'font-family' => 'My Font',
'font-weight' => '300',
'src' => 'https://example.com/assets/fonts/my-font.ttf.woff2',
),
Expand Down Expand Up @@ -826,6 +920,7 @@ protected function get_data_registry() {
'lato' => array(),
'merriweather' => array(
'merriweather-200-900-normal' => array(
'font-family' => 'Merriweather',
'font-weight' => '200 900',
'font-stretch' => 'normal',
'src' => 'https://example.com/assets/fonts/merriweather.ttf.woff2',
Expand All @@ -834,6 +929,7 @@ protected function get_data_registry() {
'Source Serif Pro' => array(
'Source Serif Pro-300-normal' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'normal',
'font-weight' => '300',
'font-stretch' => 'normal',
Expand All @@ -842,6 +938,7 @@ protected function get_data_registry() {
),
'Source Serif Pro-900-italic' => array(
'provider' => 'local',
'font-family' => 'Source Serif Pro',
'font-style' => 'italic',
'font-weight' => '900',
'font-stretch' => 'normal',
Expand All @@ -851,15 +948,18 @@ protected function get_data_registry() {
),
'my-font' => array(
'my-font-300-normal' => array(
'font-family' => 'My Font',
'font-weight' => '300',
'src' => 'https://example.com/assets/fonts/my-font.ttf.woff2',
),
'my-font-300-italic' => array(
'font-family' => 'My Font',
'font-weight' => '300',
'font-style' => 'italic',
'src' => 'https://example.com/assets/fonts/my-font.ttf.woff2',
),
'my-font-900-normal' => array(
'font-family' => 'My Font',
'font-weight' => '900',
'src' => 'https://example.com/assets/fonts/my-font.ttf.woff2',
),
Expand Down Expand Up @@ -1004,18 +1104,21 @@ protected function get_registered_mock_fonts() {
'font1' => array(
'font1-300-normal' => array(
'provider' => 'mock',
'font-family' => 'Font 1',
'font-weight' => '300',
'font-style' => 'normal',
'font-display' => 'fallback',
),
'font1-300-italic' => array(
'provider' => 'mock',
'font-family' => 'Font 1',
'font-weight' => '300',
'font-style' => 'italic',
'font-display' => 'fallback',
),
'font1-900-normal' => array(
'provider' => 'mock',
'font-family' => 'Font 1',
'font-weight' => '900',
'font-style' => 'normal',
'font-display' => 'fallback',
Expand All @@ -1024,12 +1127,14 @@ protected function get_registered_mock_fonts() {
'font2' => array(
'font2-200-900-normal' => array(
'provider' => 'mock',
'font-family' => 'Font 2',
'font-weight' => '200 900',
'font-style' => 'normal',
'font-display' => 'fallback',
),
'font2-200-900-italic' => array(
'provider' => 'mock',
'font-family' => 'Font 2',
'font-weight' => '200 900',
'font-style' => 'italic',
'font-display' => 'fallback',
Expand All @@ -1038,6 +1143,7 @@ protected function get_registered_mock_fonts() {
'font3' => array(
'font3-bold-normal' => array(
'provider' => 'mock',
'font-family' => 'Font 3',
'font-weight' => 'bold',
'font-style' => 'normal',
'font-display' => 'fallback',
Expand Down
Loading

0 comments on commit 2497b63

Please sign in to comment.