Improve page experience for fonts in core themes

includes/sanitizers/class-amp-style-sanitizer.php

@@ -18,6 +18,7 @@
 use AmpProject\Dom\Element;
 use AmpProject\Exception\FailedToGetFromRemoteUrl;
 use AmpProject\RemoteGetRequest;
+use AmpProject\RequestDestination;
 use Sabberworm\CSS\RuleSet\DeclarationBlock;
 use Sabberworm\CSS\CSSList\CSSList;
 use Sabberworm\CSS\Property\Selector;
@@ -29,6 +30,7 @@
 use Sabberworm\CSS\OutputFormat;
 use Sabberworm\CSS\Property\Import;
 use Sabberworm\CSS\CSSList\AtRuleBlockList;
+use Sabberworm\CSS\Value\CSSFunction;
 use Sabberworm\CSS\Value\RuleValueList;
 use Sabberworm\CSS\Value\URL;
 use Sabberworm\CSS\Value\Value;
@@ -139,6 +141,7 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
 	 *      @type bool     $skip_tree_shaking              Whether tree shaking should be skipped.
 	 *      @type bool     $allow_excessive_css            Whether to allow CSS to exceed the allowed max bytes (without raising validation errors).
 	 *      @type bool     $transform_important_qualifiers Whether !important rules should be transformed. This also necessarily transform inline style attributes.
+	 *      @type string[] $font_face_display_overrides    Array of the font family names and the font-display value they should each have.
 	 * }
 	 */
 	protected $args;
@@ -166,6 +169,11 @@ class AMP_Style_Sanitizer extends AMP_Base_Sanitizer {
 		'skip_tree_shaking'              => false,
 		'allow_excessive_css'            => false,
 		'transform_important_qualifiers' => true,
+		'font_face_display_overrides'    => [
+			'NonBreakingSpaceOverride' => 'optional',
+			'Inter var'                => 'optional',
+			'Genericons'               => 'block',
+		],
 	];
 
 	/**
@@ -1655,6 +1663,7 @@ private function get_parsed_stylesheet( $stylesheet, $options = [] ) {
 					'parsed_cache_variant',
 					'dynamic_element_selectors',
 					'transform_important_qualifiers',
+					'font_face_display_overrides',
 				]
 			),
 			[
@@ -2522,8 +2531,9 @@ private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
 		$font_family   = null;
 		$font_basename = null;
 		$properties    = $ruleset->getRules( 'font-family' );
-		if ( isset( $properties[0] ) ) {
-			$font_family = trim( $properties[0]->getValue(), '"\'' );
+		$property      = end( $properties );
+		if ( $property instanceof Rule ) {
+			$font_family = trim( $property->getValue(), '"\'' );
 
 			// Remove all non-word characters from the font family to serve as the filename.
 			$font_basename = preg_replace( '/[^A-Za-z0-9_\-]/', '', $font_family ); // Same as sanitize_key() minus case changes.
@@ -2540,8 +2550,11 @@ private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
 			$stylesheet_base_url = trailingslashit( $stylesheet_base_url );
 		}
 
+		// Obtain the font file path (if any) and the first font src type.
+		$font_file      = '';
+		$first_src_type = '';
+
 		// Attempt to transform data: URLs in src properties to be external file URLs.
-		$converted_count = 0;
 		foreach ( $src_properties as $src_property ) {
 			$value = $src_property->getValue();
 			if ( ! ( $value instanceof RuleValueList ) ) {
@@ -2599,24 +2612,50 @@ private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
 			 * @var URL[]    $source_data_url_objects
 			 */
 			$source_data_url_objects = [];
-			foreach ( $sources as $i => $source ) {
-				if ( $source[0] instanceof URL ) {
-					$value = $source[0]->getURL()->getString();
-					if ( 'data:' === substr( $value, 0, 5 ) ) {
-						$source_data_url_objects[ $i ] = $source[0];
-					} else {
-						$source_file_urls[ $i ] = $value;
+			foreach ( $sources as $source ) {
+				if ( count( $source ) !== 2 ) {
+					continue;
+				}
+				list( $url, $format ) = $source;
+				if (
+					! $url instanceof URL
+					||
+					! $format instanceof CSSFunction
+					||
+					$format->getName() !== 'format'
+					||
+					count( $format->getArguments() ) !== 1
+				) {
+					continue;
+				}
+
+				list( $format_value ) = $format->getArguments();
+				$format_value         = trim( $format_value, '"\'' );
+
+				$value = $url->getURL()->getString();
+				if ( 'data:' === substr( $value, 0, 5 ) ) {
+					$source_data_url_objects[ $format_value ] = $source[0];
+					if ( empty( $first_src_type ) ) {
+						$first_src_type = 'inline';
+					}
+				} else {
+					$source_file_urls[] = $value;
+					if ( empty( $first_src_type ) ) {
+						$first_src_type = 'file';
+						$font_file      = $value;
 					}
 				}
 			}
 
 			// Convert data: URLs into regular URLs, assuming there will be a file present (e.g. woff fonts in core themes).
-			foreach ( $source_data_url_objects as $i => $data_url ) {
+			foreach ( $source_data_url_objects as $format => $data_url ) {
 				$mime_type = strtok( substr( $data_url->getURL()->getString(), 5 ), ';' );
-				if ( ! $mime_type ) {
-					continue;
+				if ( $mime_type ) {
+					$extension = preg_replace( ':.+/(.+-)?:', '', $mime_type );
+				} else {
+					$extension = $format;
 				}
-				$extension = preg_replace( ':.+/(.+-)?:', '', $mime_type );
+				$extension = sanitize_key( $extension );
 
 				$guessed_urls = [];
 
@@ -2648,7 +2687,10 @@ private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
 					$path = $this->get_validated_url_file_path( $guessed_url, [ 'woff', 'woff2', 'ttf', 'otf', 'svg' ] );
 					if ( ! is_wp_error( $path ) ) {
 						$data_url->getURL()->setString( $guessed_url );
-						$converted_count++;
+						if ( 'inline' === $first_src_type ) {
+							$first_src_type = 'file';
+							$font_file      = $guessed_url;
+						}
 						continue 2;
 					}
 				}
@@ -2661,23 +2703,41 @@ private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
 					'genericons.woff',
 				];
 				if ( in_array( $font_filename, $bundled_fonts, true ) ) {
-					$data_url->getURL()->setString( plugin_dir_url( AMP__FILE__ ) . "assets/fonts/$font_filename" );
-					$converted_count++;
+					$font_file = plugin_dir_url( AMP__FILE__ ) . "assets/fonts/$font_filename";
+					$data_url->getURL()->setString( $font_file );
+					if ( 'inline' === $first_src_type ) {
+						$first_src_type = 'file';
+						$font_file      = $font_file;
+					}
 				}
 			} // End foreach $source_data_url_objects.
 		} // End foreach $src_properties.
 
-		/*
-		 * If a data: URL has been replaced with an external file URL, then we add a font-display:swap to the @font-face
-		 * rule if one isn't already present. This prevents FO
-		 *
-		 *  If no font-display is already present, add font-display:swap since the font is now being loaded externally.
-		 */
-		if ( $converted_count && 0 === count( $ruleset->getRules( 'font-display' ) ) ) {
+		// Override the 'font-display' property to improve font performance.
+		if ( $font_family && in_array( $font_family, array_keys( $this->args['font_face_display_overrides'] ), true ) ) {
+			$ruleset->removeRule( 'font-display' );
 			$font_display_rule = new Rule( 'font-display' );
-			$font_display_rule->setValue( 'swap' );
+			$font_display_rule->setValue( $this->args['font_face_display_overrides'][ $font_family ] );
 			$ruleset->addRule( $font_display_rule );
 		}
+
+		// If the font-display is auto, block, or swap then we should automatically add the preload link for the first font file.
+		$properties = $ruleset->getRules( 'font-display' );
+		$property   = end( $properties ); // Last since the last property wins in CSS.
+		if (
+			(
+				// Defaults to 'auto', hence should be preloaded as well.
+				! $property instanceof Rule
+				||
+				in_array( $property->getValue(), [ 'auto', 'block', 'swap' ], true )
+			)
+			&&
+			'file' === $first_src_type
+			&&
+			! empty( $font_file )
+		) {
+			$this->dom->links->addPreload( $font_file, RequestDestination::FONT );
+		}
 	}
 
 	/**

tests/php/test-amp-style-sanitizer.php

@@ -1898,20 +1898,115 @@ public function test_font_data_url_handling_without_file_sources() {
 		$this->assertStringNotContainsString( 'data:', $actual_stylesheets[0] );
 		$this->assertStringContainsString( 'fonts/NonBreakingSpaceOverride.woff2', $actual_stylesheets[0] );
 		$this->assertStringContainsString( 'fonts/NonBreakingSpaceOverride.woff', $actual_stylesheets[0] );
-		$this->assertStringContainsString( 'font-display:swap', $actual_stylesheets[0] );
+		$this->assertStringContainsString( 'font-display:optional', $actual_stylesheets[0] );
 
 		// Check font not included in theme, but included in plugin.
 		$this->assertStringContainsString( '@font-face{font-family:"Genericons";', $actual_stylesheets[1] );
 		$this->assertStringContainsString( 'format("woff")', $actual_stylesheets[1] );
 		$this->assertStringNotContainsString( 'data:', $actual_stylesheets[1] );
 		$this->assertStringContainsString( 'assets/fonts/genericons.woff', $actual_stylesheets[1] );
-		$this->assertStringContainsString( 'font-display:swap', $actual_stylesheets[1] );
+		$this->assertStringContainsString( 'font-display:block', $actual_stylesheets[1] );
 
 		// Check font not included anywhere, so must remain inline.
 		$this->assertStringContainsString( '@font-face{font-family:"Custom";', $actual_stylesheets[2] );
 		$this->assertStringContainsString( 'url("data:application/x-font-woff;charset=utf-8;base64,d09GRgABAAA")', $actual_stylesheets[2] );
 		$this->assertStringContainsString( 'format("woff")', $actual_stylesheets[2] );
-		$this->assertStringNotContainsString( 'font-display:swap', $actual_stylesheets[2] );
+		$this->assertStringNotContainsString( 'font-display:', $actual_stylesheets[2] );
+	}
+
+	/** @return array */
+	public function get_data_to_test_font_files_preloading() {
+		return [
+			'twentynineteen' => [
+				'theme_slug'    => 'twentynineteen',
+				'expected_urls' => [], // Twenty Nineteen theme uses "NonBreakingSpaceOverride" font which should use 'font-display:optional' property, thus should not be preloaded.
+			],
+			'twentytwenty' => [
+				'theme_slug'    => 'twentytwenty',
+				'expected_urls' => [], // Twenty Twenty theme uses "Inter var" and "NonBreakingSpaceOverride" font which should use 'font-display:optional' property, thus should not be preloaded.
+			],
+			'twentytwentyone' => [
+				'theme_slug'    => 'twentytwentyone',
+				'expected_urls' => [], // Twenty Twenty-One theme uses system font stack, no extra fonts are enqueued.
+			],
+			'custom_swap'     => [
+				'theme_slug'    => '',
+				'expected_urls' => [
+					'/fonts/OpenSans-Regular-webfont.woff2',
+				],
+				'html'          => '<html amp><head><meta charset="utf-8"><style>@font-face{font-family:"Open Sans";src:url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2");font-display:swap}</style></head><body></body></html>',
+			],
+			'custom_optional' => [
+				'theme_slug'    => '',
+				'expected_urls' => [],
+				'html'          => '<html amp><head><meta charset="utf-8"><style>@font-face{font-family:"Open Sans";src:url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2");font-display:optional}</style></head><body></body></html>',
+			],
+			'custom_combined' => [
+				'theme_slug'    => '',
+				'expected_urls' => [
+					'/fonts/OpenSans-Regular-webfont.woff2',
+					'/fonts/Lato-Regular-webfont.woff2',
+				],
+				'html'          => '<html amp><head><meta charset="utf-8"><style>@font-face{font-family:"Open Sans";src:url("/fonts/OpenSans-Regular-webfont.woff2") format("woff2");font-display:swap}@font-face{font-family:"Roboto";src:url("/fonts/Roboto-Regular-webfont.woff2") format("woff2");font-display:optional}@font-face{font-family:"Lato";src:url("/fonts/Lato-Regular-webfont.woff2") format("woff2"),url("/fonts/Lato-Regular-webfont.woff") format("woff")}</style></head><body></body></html>',
+			],
+			'custom_inlined' => [
+				'theme_slug'    => '',
+				'expected_urls' => [], // No preload link should be present because the data: URL was not converted into an external link.
+				'html'          => '<html amp><head><meta charset="utf-8"><style>@font-face{font-family:"Open Sans";src:url("data:application/font-woff2;charset=utf-8;base64,...") format("woff2"), url("/fonts/OpenSans-Regular-webfont.woff") format("woff");font-display:swap}</style></head><body></body></html>',
+			],
+		];
+	}
+
+	/**
+	 * Test that font files are preloaded with <link> element.
+	 *
+	 * @dataProvider get_data_to_test_font_files_preloading
+	 * @covers AMP_Style_Sanitizer::process_font_face_at_rule()
+	 */
+	public function test_font_files_preloading( $theme_slug, $expected_urls, $html = '' ) {
+		if ( ! empty( $theme_slug ) ) {
+			$theme = new WP_Theme( $theme_slug, ABSPATH . 'wp-content/themes' );
+			if ( $theme->errors() ) {
+				$this->markTestSkipped( $theme->errors()->get_error_message() );
+			}
+
+			$html  = '<html amp><head><meta charset="utf-8">';
+			$html .= sprintf( '<link rel="stylesheet" href="%s">', esc_url( $theme->get_stylesheet_directory_uri() . '/style.css' ) ); // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedStylesheet
+			$html .= '</head><body></body></html>';
+		}
+
+		$dom         = Document::fromHtml( $html, Options::DEFAULTS );
+		$error_codes = [];
+		$sanitizer   = new AMP_Style_Sanitizer(
+			$dom,
+			[
+				'use_document_element' => true,
+			]
+		);
+		$sanitizer->sanitize();
+		$this->assertEquals( [], $error_codes );
+
+		$link_elements       = $dom->getElementsByTagName( 'link' );
+		$link_elements_count = $link_elements->length;
+
+		$actual_urls = array_map(
+			static function ( Element $link_element ) {
+				return $link_element->getAttribute( 'href' );
+			},
+			iterator_to_array( $link_elements )
+		);
+
+		$this->assertEqualSets( $expected_urls, $actual_urls );
+
+		for ( $i = 0; $i < $link_elements_count; $i++ ) {
+			$this->assertStringEndsWith(
+				$expected_urls[ $i ],
+				$link_elements->item( $i )->getAttribute( 'href' )
+			);
+			$this->assertEquals( $link_elements->item( $i )->getAttribute( 'rel' ), 'preload' );
+			$this->assertEquals( $link_elements->item( $i )->getAttribute( 'as' ), 'font' );
+			$this->assertEquals( $link_elements->item( $i )->getAttribute( 'crossorigin' ), '' );
+		}
 	}
 
 	/**