Skip to content

Commit

Permalink
Merge pull request #2079 from ampproject/fix/font-face-rule-processing
Browse files Browse the repository at this point in the history 
Expand externalization of data: URLs in @font-face rules
  • Loading branch information
@westonruter
westonruter committed Apr 7, 2019
2 parents 28b70b0 + 842a43a commit 4a0e736
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 33 deletions.
122 changes: 89 additions & 33 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Expand Up @@ -10,6 +10,7 @@
use \Sabberworm\CSS\Property\Selector;
use \Sabberworm\CSS\RuleSet\RuleSet;
use \Sabberworm\CSS\Property\AtRule;
use \Sabberworm\CSS\Rule\Rule;
use \Sabberworm\CSS\CSSList\KeyFrame;
use \Sabberworm\CSS\RuleSet\AtRuleSet;
use \Sabberworm\CSS\Property\Import;
Expand Down Expand Up @@ -1105,7 +1106,7 @@ private function fetch_external_stylesheet( $url ) {
private function process_stylesheet( $stylesheet, $options = array() ) {
$parsed = null;
$cache_key = null;
$cache_group = 'amp-parsed-stylesheet-v14'; // This should be bumped whenever the PHP-CSS-Parser is updated.
$cache_group = 'amp-parsed-stylesheet-v15'; // This should be bumped whenever the PHP-CSS-Parser is updated.

$cache_impacting_options = array_merge(
wp_array_slice_assoc(
Expand Down Expand Up @@ -1290,6 +1291,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti
*
* @type Document $css_document CSS Document.
* @type array $validation_results Validation results, array containing arrays with error and sanitized keys.
* @type string $stylesheet_url Stylesheet URL, if available.
* }
*/
private function parse_stylesheet( $stylesheet_string, $options ) {
Expand Down Expand Up @@ -1790,7 +1792,7 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
}

if ( $ruleset instanceof AtRuleSet && 'font-face' === $ruleset->atRuleName() ) {
$this->process_font_face_at_rule( $ruleset );
$this->process_font_face_at_rule( $ruleset, $options );
}

$results = array_merge(
Expand All @@ -1807,18 +1809,47 @@ private function process_css_declaration_block( RuleSet $ruleset, CSSList $css_l
}

/**
* Process @font-face by making src URLs non-relative and converting data: URLs into (assumed) file URLs.
* Process @font-face by making src URLs non-relative and converting data: URLs into file URLs (with educated guessing).
*
* @since 1.0
*
* @param AtRuleSet $ruleset Ruleset for @font-face.
* @param array $options {
* Options.
*
* @type string $stylesheet_url Stylesheet URL, if available.
* }
*/
private function process_font_face_at_rule( AtRuleSet $ruleset ) {
private function process_font_face_at_rule( AtRuleSet $ruleset, $options ) {
$src_properties = $ruleset->getRules( 'src' );
if ( empty( $src_properties ) ) {
return;
}

// Obtain the font-family name to guess the filename.
$font_family = null;
$font_basename = null;
$properties = $ruleset->getRules( 'font-family' );
if ( isset( $properties[0] ) ) {
$font_family = trim( $properties[0]->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.
}

// Obtain the stylesheet base URL from which to guess font file locations.
$stylesheet_base_url = null;
if ( ! empty( $options['stylesheet_url'] ) ) {
$stylesheet_base_url = preg_replace(
':[^/]+(\?.*)?(#.*)?$:',
'',
$options['stylesheet_url']
);
$stylesheet_base_url = trailingslashit( $stylesheet_base_url );
}

// 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 ) ) {
Expand Down Expand Up @@ -1868,52 +1899,77 @@ private function process_font_face_at_rule( AtRuleSet $ruleset ) {
/**
* Source URL lists.
*
* @var URL[] $source_file_urls
* @var URL[] $source_data_urls
* @var string[] $source_file_urls
* @var URL[] $source_data_url_objects
*/
$source_file_urls = array();
$source_data_urls = array();
$source_file_urls = array();
$source_data_url_objects = array();
foreach ( $sources as $i => $source ) {
if ( $source[0] instanceof URL ) {
if ( 'data:' === substr( $source[0]->getURL()->getString(), 0, 5 ) ) {
$source_data_urls[ $i ] = $source[0];
$value = $source[0]->getURL()->getString();
if ( 'data:' === substr( $value, 0, 5 ) ) {
$source_data_url_objects[ $i ] = $source[0];
} else {
$source_file_urls[ $i ] = $source[0];
$source_file_urls[ $i ] = $value;
}
}
}

// Convert data: URLs into regular URLs, assuming there will be a file present (e.g. woff fonts in core themes).
if ( empty( $source_file_urls ) ) {
continue;
}
$source_file_url = current( $source_file_urls );
foreach ( $source_data_urls as $i => $data_url ) {
foreach ( $source_data_url_objects as $i => $data_url ) {
$mime_type = strtok( substr( $data_url->getURL()->getString(), 5 ), ';' );
if ( ! $mime_type ) {
continue;
}
$extension = preg_replace( ':.+/(.+-)?:', '', $mime_type );
$guessed_url = preg_replace(
':(?<=\.)\w+(\?.*)?(#.*)?$:', // Match the file extension in the URL.
$extension,
$source_file_url->getURL()->getString(),
1,
$count
);
if ( 1 !== $count ) {
continue;
$extension = preg_replace( ':.+/(.+-)?:', '', $mime_type );

$guessed_urls = array();

// Guess URLs based on any other font sources that are not using data: URLs (e.g. truetype fallback for inline woff2).
foreach ( $source_file_urls as $source_file_url ) {
$guessed_url = preg_replace(
':(?<=\.)\w+(\?.*)?(#.*)?$:', // Match the file extension in the URL.
$extension,
$source_file_url,
1,
$count
);
if ( 1 === $count ) {
$guessed_urls[] = $guessed_url;
}
}

// Ensure font file exists.
$path = $this->get_validated_url_file_path( $guessed_url, array( 'woff', 'woff2', 'ttf', 'otf', 'svg' ) );
if ( is_wp_error( $path ) ) {
continue;
/*
* Guess some font file URLs based on the font name in a fonts directory based on precedence of Twenty Nineteen.
* For example, the NonBreakingSpaceOverride woff2 font file is located at fonts/NonBreakingSpaceOverride.woff2.
*/
if ( $stylesheet_base_url && $font_basename ) {
$guessed_urls[] = $stylesheet_base_url . sprintf( 'fonts/%s.%s', $font_basename, $extension );
$guessed_urls[] = $stylesheet_base_url . sprintf( 'fonts/%s.%s', strtolower( $font_basename ), $extension );
}

$data_url->getURL()->setString( $guessed_url );
break;
}
// Find the font file that exists, and then replace the data: URL with the external URL for the font.
foreach ( $guessed_urls as $guessed_url ) {
$path = $this->get_validated_url_file_path( $guessed_url, array( 'woff', 'woff2', 'ttf', 'otf', 'svg' ) );
if ( ! is_wp_error( $path ) ) {
$data_url->getURL()->setString( $guessed_url );
$converted_count++;
break;
}
}
} // 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' ) ) ) {
$font_display_rule = new Rule( 'font-display' );
$font_display_rule->setValue( 'swap' );
$ruleset->addRule( $font_display_rule );
}
}

Expand Down
41 changes: 41 additions & 0 deletions tests/test-amp-style-sanitizer.php
View file
Expand Up @@ -905,6 +905,47 @@ public function test_font_data_url_handling() {
$this->assertContains( '.dashicons-format-chat:before', $actual_stylesheets[0] );
}

/**
* Test handling of stylesheets with @font-face that have data: url source.
*
* Also confirm that class-based tree-shaking is working.
*
* @link https://github.com/ampproject/amp-wp/pull/2079
*
* @covers AMP_Style_Sanitizer::process_font_face_at_rule()
*/
public function test_font_data_url_handling_without_file_sources() {
$theme = new WP_Theme( 'twentynineteen', ABSPATH . 'wp-content/themes' );
if ( $theme->errors() ) {
$this->markTestSkipped( 'Twenty Nineteen is not installed.' );
}

$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 = AMP_DOM_Utils::get_dom( $html );
$error_codes = array();
$sanitizer = new AMP_Style_Sanitizer(
$dom,
array(
'use_document_element' => true,
)
);
$sanitizer->sanitize();
$this->assertEquals( array(), $error_codes );
$actual_stylesheets = array_values( $sanitizer->get_stylesheets() );
$this->assertCount( 1, $actual_stylesheets );

$this->assertContains( '@font-face{font-family:"NonBreakingSpaceOverride";', $actual_stylesheets[0] );
$this->assertContains( 'format("woff2")', $actual_stylesheets[0] );
$this->assertContains( 'format("woff")', $actual_stylesheets[0] );
$this->assertNotContains( 'data:', $actual_stylesheets[0] );
$this->assertContains( 'fonts/NonBreakingSpaceOverride.woff2', $actual_stylesheets[0] );
$this->assertContains( 'fonts/NonBreakingSpaceOverride.woff', $actual_stylesheets[0] );
$this->assertContains( 'font-display:swap', $actual_stylesheets[0] );
}

/**
* Test that auto-removal (tree shaking) does not remove rules for classes mentioned in class and [class] attributes.
*
Expand Down

0 comments on commit 4a0e736

Please sign in to comment.