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

Fetch (local) stylesheets with @import instead of removing #1181

Merged
merged 8 commits into from
May 30, 2018
Merged

Fetch (local) stylesheets with @import instead of removing #1181

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
658ccf1
Add fetching local @import urls.
miina May 29, 2018
cab6728
Re-use real_path_urls method to determine absolute URL for import
westonruter May 29, 2018
1910107
Add support for import media queries
westonruter May 29, 2018
350a120
Merge branch 'develop' of https://github.com/Automattic/amp-wp into u…
westonruter May 29, 2018
0e3c5c5
Move dirty native AMP CSS imports to beginning of stylesheet to be ho…
westonruter May 29, 2018
025d1bb
Guard against double-reporting validation errors when sanitization re…
westonruter May 29, 2018
a404fd2
Fix construction of validation preview URL; prevent JSON term descrip…
westonruter May 29, 2018
ffe3c09
Test CSS import handling
westonruter May 30, 2018
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
49 changes: 44 additions & 5 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,7 @@ private function process_stylesheet( $stylesheet, $node, $options = array() ) {
} else {
$parsed = get_transient( $cache_key . $cache_group );
}

if ( ! $parsed || ! isset( $parsed['stylesheet'] ) || ! is_array( $parsed['stylesheet'] ) ) {
$parsed = $this->parse_stylesheet( $stylesheet, $options );
if ( wp_using_ext_object_cache() ) {
Expand All @@ -573,7 +574,7 @@ private function process_stylesheet( $stylesheet, $node, $options = array() ) {
}
}

if ( ! empty( $this->args['validation_error_callback'] ) && ! empty( $parsed['validation_errors'] ) ) {
if ( $node && ! empty( $this->args['validation_error_callback'] ) && ! empty( $parsed['validation_errors'] ) ) {
foreach ( $parsed['validation_errors'] as $validation_error ) {
call_user_func( $this->args['validation_error_callback'], array_merge( $validation_error, compact( 'node' ) ) );
}
Expand All @@ -582,6 +583,47 @@ private function process_stylesheet( $stylesheet, $node, $options = array() ) {
return $parsed['stylesheet'];
}

/**
* Parse imported stylesheet.
*
* @param Import $item Import object.
*/
private function parse_import_stylesheet( Import $item ) {
$import_url = $item->getLocation()->getURL()->getString();

// If it's local.
if ( false === filter_var( $import_url, FILTER_VALIDATE_URL ) && ! file_exists( $import_url ) ) {
if ( 0 === strpos( $import_url, '.' ) ) {
$import_url = str_replace( '.', '', 0 );
}
if ( 0 === strpos( $import_url, '/' ) ) {
$import_url = substr_replace( $import_url, '', 0, 1 );
}
$import_url = get_stylesheet_directory_uri() . '/' . $import_url;
}

// @todo else for external.
$css_file_path = $this->get_validated_url_file_path( $import_url, array( 'css', 'less', 'scss', 'sass' ) );

// Load the CSS from the filesystem.
$stylesheet = file_get_contents( $css_file_path ); // phpcs:ignore -- It's a local filesystem path not a remote request.

$stylesheet = $this->process_stylesheet( $stylesheet, null, array(
'allowed_at_rules' => $this->style_custom_cdata_spec['css_spec']['allowed_at_rules'],
'property_whitelist' => $this->style_custom_cdata_spec['css_spec']['allowed_declarations'],
'stylesheet_url' => $import_url,
'stylesheet_path' => $css_file_path,
) );

$pending_stylesheet = array(
'keyframes' => false,
'stylesheet' => $stylesheet,
'node' => null,
);

$this->pending_stylesheets[] = $pending_stylesheet;
}

/**
* Parse stylesheet.
*
Expand Down Expand Up @@ -837,10 +879,7 @@ private function process_css_list( CSSList $css_list, $options ) {
$css_list->remove( $css_item );
}
} elseif ( $css_item instanceof Import ) {
$validation_errors[] = array(
'code' => 'illegal_css_import_rule',
'message' => __( 'CSS @import is currently disallowed.', 'amp' ),
);
$this->parse_import_stylesheet( $css_item );
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@westonruter Still WIP but started adding the support for @imported stylesheets, any thought on the approach?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, your approach is great. I went ahead and iterated on it because of the changes introduced by the refactored error validation handling.

$css_list->remove( $css_item );
} elseif ( $css_item instanceof AtRuleSet ) {
if ( in_array( $css_item->atRuleName(), $options['allowed_at_rules'], true ) ) {
Expand Down