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

Gracefully handle conflicting version of PHP-CSS-Parser being loaded #1743

Merged
merged 1 commit into from Dec 14, 2018
Merged

Gracefully handle conflicting version of PHP-CSS-Parser being loaded #1743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
39 changes: 39 additions & 0 deletions includes/options/class-amp-options-manager.php
View file
Open in desktop
Expand Up @@ -52,6 +52,7 @@ public static function register_settings() {
add_action( 'admin_notices', array( __CLASS__, 'render_welcome_notice' ) );
add_action( 'admin_notices', array( __CLASS__, 'persistent_object_caching_notice' ) );
add_action( 'admin_notices', array( __CLASS__, 'render_cache_miss_notice' ) );
add_action( 'admin_notices', array( __CLASS__, 'render_php_css_parser_conflict_notice' ) );
}

/**
Expand Down Expand Up @@ -439,6 +440,44 @@ public static function render_cache_miss_notice() {
);
}

/**
* Render PHP-CSS-Parser conflict notice.
*
* @return void
*/
public static function render_php_css_parser_conflict_notice() {
if ( 'toplevel_page_' . self::OPTION_NAME !== get_current_screen()->id ) {
return;
}

if ( AMP_Style_Sanitizer::has_required_php_css_parser() ) {
return;
}

try {
$reflection = new ReflectionClass( 'Sabberworm\CSS\CSSList\CSSList' );
$source_dir = str_replace(
trailingslashit( WP_CONTENT_DIR ),
'',
preg_replace( '#/vendor/sabberworm/.+#', '', $reflection->getFileName() )
);

printf(
'<div class="notice notice-warning"><p>%s</p></div>',
sprintf(
/* translators: %s is location where conflicting lib was found */
esc_html__( "A conflicting version of PHP-CSS-Parser appears to be installed by another plugin/theme (located in '%s'). Because of this CSS processing will be limited, and tree shaking will not be available.", 'amp' ),
Copy link
Contributor

Choose a reason for hiding this comment

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

This notice looks good when Mailpoet is active:

notice-mailpoet-active

esc_html( $source_dir )
)
);
} catch ( ReflectionException $e ) {
printf(
'<div class="notice notice-warning"><p>%s</p></div>',
esc_html__( 'PHP-CSS-Parser is not available so CSS processing will not be available.', 'amp' )
);
}
}

/**
* Show the response cache disabled notice.
*
Expand Down
108 changes: 86 additions & 22 deletions includes/sanitizers/class-amp-style-sanitizer.php
View file
Open in desktop
Expand Up @@ -250,6 +250,43 @@ public static function get_css_parser_validation_error_codes() {
);
}

/**
* Determine whether the version of PHP-CSS-Parser loaded has all required features for tree shaking and CSS processing.
*
* @since 1.0.2
*
* @return bool Returns true if the plugin's forked version of PHP-CSS-Parser is loaded by Composer.
*/
public static function has_required_php_css_parser() {
$has_required_methods = (
method_exists( 'Sabberworm\CSS\CSSList\Document', 'splice' )
&&
method_exists( 'Sabberworm\CSS\CSSList\Document', 'replace' )
);
if ( ! $has_required_methods ) {
return false;
}

$reflection = new ReflectionClass( 'Sabberworm\CSS\OutputFormat' );

$has_output_format_extensions = (
$reflection->hasProperty( 'sBeforeAtRuleBlock' )
&&
$reflection->hasProperty( 'sAfterAtRuleBlock' )
&&
$reflection->hasProperty( 'sBeforeDeclarationBlock' )
&&
$reflection->hasProperty( 'sAfterDeclarationBlockSelectors' )
&&
$reflection->hasProperty( 'sAfterDeclarationBlock' )
);
if ( ! $has_output_format_extensions ) {
return false;
}

return true;
}

/**
* AMP_Base_Sanitizer constructor.
*
Expand Down Expand Up @@ -937,7 +974,7 @@ private function parse_import_stylesheet( Import $item, CSSList $css_list, $opti
*/
$css_document = $parsed_stylesheet['css_document'];

if ( ! empty( $parsed_stylesheet['css_document'] ) ) {
if ( ! empty( $parsed_stylesheet['css_document'] ) && method_exists( $css_list, 'replace' ) ) {
$css_list->replace( $item, $css_document->getContents() );
} else {
$css_list->remove( $item );
Expand Down Expand Up @@ -1065,12 +1102,15 @@ private function prepare_stylesheet( $stylesheet_string, $options = array() ) {
$before_at_rule = '/*AMP_WP_BEFORE_AT_RULE*/';
$after_at_rule = '/*AMP_WP_AFTER_AT_RULE*/';

$output_format->set( 'BeforeDeclarationBlock', $before_declaration_block );
$output_format->set( 'SpaceBeforeSelectorSeparator', $between_selectors );
$output_format->set( 'AfterDeclarationBlockSelectors', $after_declaration_block_selectors );
$output_format->set( 'AfterDeclarationBlock', $after_declaration_block );
$output_format->set( 'BeforeAtRuleBlock', $before_at_rule );
$output_format->set( 'AfterAtRuleBlock', $after_at_rule );
// Add comments to stylesheet if PHP-CSS-Parser has the required extensions for tree shaking.
if ( self::has_required_php_css_parser() ) {
$output_format->set( 'BeforeDeclarationBlock', $before_declaration_block );
$output_format->set( 'SpaceBeforeSelectorSeparator', $between_selectors );
$output_format->set( 'AfterDeclarationBlockSelectors', $after_declaration_block_selectors );
$output_format->set( 'AfterDeclarationBlock', $after_declaration_block );
$output_format->set( 'BeforeAtRuleBlock', $before_at_rule );
$output_format->set( 'AfterAtRuleBlock', $after_at_rule );
}

$stylesheet_string = $css_document->render( $output_format );

Expand Down Expand Up @@ -1709,7 +1749,7 @@ function( Selector $old_selector ) {
$important_ruleset->setRules( $importants );

$i = array_search( $ruleset, $css_list->getContents(), true );
if ( false !== $i ) {
if ( false !== $i && method_exists( $css_list, 'splice' ) ) {
$css_list->splice( $i + 1, 0, array( $important_ruleset ) );
} else {
$css_list->append( $important_ruleset );
Expand Down Expand Up @@ -1910,27 +1950,47 @@ private function finalize_styles() {
$comment = '';
if ( ! empty( $included_sources ) && $included_original_size > 0 ) {
$comment .= esc_html__( 'The style[amp-custom] element is populated with:', 'amp' ) . "\n" . implode( "\n", $included_sources ) . "\n";
$comment .= sprintf(
/* translators: %1$d is number of included bytes, %2$d is percentage of total CSS actually included after tree shaking, %3$d is total included size */
esc_html__( 'Total included size: %1$s bytes (%2$d%% of %3$s total after tree shaking)', 'amp' ),
number_format_i18n( $included_size ),
$included_size / $included_original_size * 100,
number_format_i18n( $included_original_size )
) . "\n";
if ( self::has_required_php_css_parser() ) {
$comment .= sprintf(
/* translators: %1$d is number of included bytes, %2$d is percentage of total CSS actually included after tree shaking, %3$d is total included size */
esc_html__( 'Total included size: %1$s bytes (%2$d%% of %3$s total after tree shaking)', 'amp' ),
number_format_i18n( $included_size ),
$included_size / $included_original_size * 100,
number_format_i18n( $included_original_size )
) . "\n";
} else {
$comment .= sprintf(
/* translators: %1$d is number of included bytes */
esc_html__( 'Total included size: %1$s bytes', 'amp' ),
number_format_i18n( $included_size ),
$included_size / $included_original_size * 100,
number_format_i18n( $included_original_size )
) . "\n";
}
}
if ( ! empty( $excluded_sources ) && $excluded_original_size > 0 ) {
if ( $comment ) {
$comment .= "\n";
}
$comment .= esc_html__( 'The following stylesheets are too large to be included in style[amp-custom]:', 'amp' ) . "\n" . implode( "\n", $excluded_sources ) . "\n";

$comment .= sprintf(
/* translators: %1$d is number of excluded bytes, %2$d is percentage of total CSS actually excluded even after tree shaking, %3$d is total excluded size */
esc_html__( 'Total excluded size: %1$s bytes (%2$d%% of %3$s total after tree shaking)', 'amp' ),
number_format_i18n( $excluded_size ),
$excluded_size / $excluded_original_size * 100,
number_format_i18n( $excluded_original_size )
) . "\n";
if ( self::has_required_php_css_parser() ) {
$comment .= sprintf(
/* translators: %1$d is number of excluded bytes, %2$d is percentage of total CSS actually excluded even after tree shaking, %3$d is total excluded size */
esc_html__( 'Total excluded size: %1$s bytes (%2$d%% of %3$s total after tree shaking)', 'amp' ),
number_format_i18n( $excluded_size ),
$excluded_size / $excluded_original_size * 100,
number_format_i18n( $excluded_original_size )
) . "\n";
} else {
$comment .= sprintf(
/* translators: %1$d is number of excluded bytes */
esc_html__( 'Total excluded size: %1$s bytes', 'amp' ),
number_format_i18n( $excluded_size ),
$excluded_size / $excluded_original_size * 100,
number_format_i18n( $excluded_original_size )
) . "\n";
}

$total_size = $included_size + $excluded_size;
$total_original_size = $included_original_size + $excluded_original_size;
Expand All @@ -1946,6 +2006,10 @@ private function finalize_styles() {
}
}

if ( ! self::has_required_php_css_parser() ) {
$comment .= "\n" . esc_html__( '!!!WARNING!!! AMP CSS processing is limited because a conflicting version of PHP-CSS-Parser has been loaded by another plugin/theme. Tree shaking is not available.', 'amp' ) . "\n";
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks good when the Mailpoet plugin is active.

warning-appears


if ( $comment ) {
$this->amp_custom_style_element->parentNode->insertBefore(
$this->dom->createComment( "\n$comment" ),
Expand Down