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.

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

Use wp_unique_id-based block class names instead of uniqid #6949

Merged
merged 23 commits into from
Mar 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
fcb7244
Use `wp_unique_id`-based block class names instead of `uniqid`
delawski Mar 1, 2022
90b7886
Merge branch 'develop' of github.com:ampproject/amp-wp into fix/6925-…
westonruter Mar 9, 2022
41fa877
Utilize static variable for constructed regex
westonruter Mar 9, 2022
54746e3
Reorganize transform_class_names_in_block_content slightly
westonruter Mar 9, 2022
e7c349f
Add failing test case for class being incorrectly transformed in text…
westonruter Mar 9, 2022
b441abe
Restrict replacements to class attribute values in start tags
westonruter Mar 9, 2022
473a8e7
Provide polyfill for wp_unique_id() for WP<5.0.3
westonruter Mar 9, 2022
e41fa08
Implement AMP_Block_Uniqid_Sanitizer
westonruter Mar 10, 2022
9fb0510
Use transformer only with some versions of Gutenberg
delawski Mar 11, 2022
703e853
Transform uniqid in WordPress 5.8 through 6.0
delawski Mar 11, 2022
f68c8e2
Add support for legacy version of duotone prefix
delawski Mar 11, 2022
76c1415
Test for correct WP and Gutenberg versions
delawski Mar 11, 2022
af994b1
Do not check if test is dealing with AMP request
delawski Mar 11, 2022
5c7cffe
Migrate block uniqid tests cases to `AMP_Block_Uniqid_Sanitizer`
delawski Mar 11, 2022
605a746
Extend MonitorCssTransientCaching with BlockUniqidTransformer awareness
westonruter Mar 17, 2022
6e6d332
Merge branch 'develop' of github.com:ampproject/amp-wp into fix/6925-…
westonruter Mar 17, 2022
4a6846a
Ignore wp_unique_id() polyfill code for coverage
westonruter Mar 17, 2022
a8e8f84
Add tests for MonitorCssTransientCaching and BlockUniqidTransformer
westonruter Mar 18, 2022
f95a1af
Normalize to allow AMP_Block_Uniqid_Sanitizer tests to run in isolation
westonruter Mar 18, 2022
8a8ae96
Use proper coversDefaultClass phpdoc tag
westonruter Mar 18, 2022
c27435c
Restore wp_version global after tests
westonruter Mar 18, 2022
35dda9d
Use data provider for test_handle_plugin_update
westonruter Mar 18, 2022
d87b78b
Ensure CSS transients disabling is not reset with each version update
westonruter Mar 18, 2022
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
1 change: 1 addition & 0 deletions .phpstorm.meta.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
'admin.validation_counts' => \AmpProject\AmpWP\Admin\ValidationCounts::class,
'amp_slug_customization_watcher' => \AmpProject\AmpWP\AmpSlugCustomizationWatcher::class,
'background_task_deactivator' => \AmpProject\AmpWP\BackgroundTask\BackgroundTaskDeactivator::class,
'block_uniqid_transformer' => \AmpProject\AmpWP\BlockUniqidTransformer::class,
'cli.command_namespace' => \AmpProject\AmpWP\CliCli\CommandNamespaceRegistration::class,
'cli.optimizer_command' => \AmpProject\AmpWP\CliCli\OptimizerCommand::class,
'cli.transformer_command' => \AmpProject\AmpWP\CliCli\TransformerCommand::class,
Expand Down
6 changes: 0 additions & 6 deletions includes/options/class-amp-options-manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -351,12 +351,6 @@ public static function validate_options( $new_options ) {
}
}

if ( array_key_exists( Option::DISABLE_CSS_TRANSIENT_CACHING, $new_options ) && true === $new_options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] ) {
$options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] = true;
} else {
unset( $options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] );
}

/**
* Filter the options being updated, so services can handle the sanitization and validation of
* their respective options.
Expand Down
204 changes: 204 additions & 0 deletions includes/sanitizers/class-amp-block-uniqid-sanitizer.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,204 @@
<?php
/**
* Class AMP_Block_Uniqid_Sanitizer.
*
* @package AMP
*/

use AmpProject\Dom\Element;
use AmpProject\Html\Attribute;
use AmpProject\Html\Tag;
use AmpProject\Dom\Document;

/**
* Work around use of uniqid() in blocks which breaks parsed CSS caching.
*
* @link https://github.com/ampproject/amp-wp/issues/6925
* @link https://github.com/WordPress/gutenberg/issues/38889
*
* @since 2.2.2
* @internal
*/
class AMP_Block_Uniqid_Sanitizer extends AMP_Base_Sanitizer {

/**
* Prefixes of class names that should be transformed.
*
* @var string[]
*/
const KEY_PREFIXES = [
'wp-container-',
'wp-duotone-',
'wp-duotone-filter-',
'wp-elements-',
];

/**
* Class name pattern.
*
* @var string
*/
private $key_pattern;

/**
* The mapping between uniqid- and wp_unique_id-based class names.
*
* @var array
*/
private $key_mapping = [];

/**
* @param Document $dom DOM.
* @param array $args Args.
*/
public function __construct( $dom, $args = [] ) {
parent::__construct( $dom, $args );

$this->key_pattern = sprintf(
'/\b(?P<prefix>%s)(?P<uniqid>[0-9a-f]{13})\b/',
implode(
'|',
self::KEY_PREFIXES
)
);
}

/**
* Sanitize.
*/
public function sanitize() {
$elements = $this->dom->xpath->query(
sprintf(
'//*[ %s ]',
implode(
' or ',
array_map(
static function ( $class_name_prefix ) {
return sprintf(
'contains( @class, "%s" )',
$class_name_prefix
);
},
self::KEY_PREFIXES
)
)
)
);

$replaced_count = 0;
foreach ( $elements as $element ) {
if ( $this->transform_element_with_class_attribute( $element ) ) {
$replaced_count++;
}
}

if ( $replaced_count > 0 ) {
$this->transform_styles();
}
}

/**
* Transform element with class.
*
* @param Element $element Element.
*/
public function transform_element_with_class_attribute( Element $element ) {
$class_name = $element->getAttribute( Attribute::CLASS_ );

$count = 0;

$new_class_name = preg_replace_callback(
$this->key_pattern,
function ( $matches ) {
$old_key = $matches[0];

if ( ! isset( $this->key_mapping[ $old_key ] ) ) {
$this->key_mapping[ $old_key ] = self::unique_id( $matches['prefix'] );
}
$new_key = $this->key_mapping[ $old_key ];

if ( in_array( $matches['prefix'], [ 'wp-duotone-', 'wp-duotone-filter-' ], true ) ) {
$this->transform_duotone_filter( $old_key, $new_key );
}

return $new_key;
},
$class_name,
-1,
$count
);
if ( 0 === $count ) {
return false;
} else {
$element->setAttribute( Attribute::CLASS_, $new_class_name );
return true;
}
}

/**
* Transform duotone filter by updating its ID.
*
* @param string $old_key Old identifier.
* @param string $new_key New identifier.
*
* @return void
*/
public function transform_duotone_filter( $old_key, $new_key ) {
$svg_filter = $this->dom->getElementById( $old_key );
if ( $svg_filter instanceof Element && Tag::FILTER === $svg_filter->tagName ) {
$svg_filter->setAttribute( Attribute::ID, $new_key );
}
}

/**
* Transform styles.
*/
public function transform_styles() {
$styles = $this->dom->xpath->query(
sprintf(
'//style[ %s ]',
implode(
' or ',
array_map(
static function ( $key_prefix ) {
return sprintf(
'contains( text(), "%s" )',
$key_prefix
);
},
self::KEY_PREFIXES
)
)
)
);

foreach ( $styles as $style ) {
$style->textContent = str_replace(
array_keys( $this->key_mapping ),
array_values( $this->key_mapping ),
$style->textContent
);
}
}

/**
* Gets unique ID.
*
* This is a polyfill for WordPress <5.0.3.
*
* @see wp_unique_id()
*
* @param string $prefix Prefix for the returned ID.
* @return string Unique ID.
*/
private static function unique_id( $prefix = '' ) {
if ( function_exists( 'wp_unique_id' ) ) {
return wp_unique_id( $prefix );
} else {
// @codeCoverageIgnoreStart
static $id_counter = 0;
return $prefix . (string) ++$id_counter;
// @codeCoverageIgnoreEnd
}
}
}
1 change: 1 addition & 0 deletions src/AmpWpPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ final class AmpWpPlugin extends ServiceBasedPlugin {
'admin.amp_themes' => Admin\AmpThemes::class,
'amp_slug_customization_watcher' => AmpSlugCustomizationWatcher::class,
'background_task_deactivator' => BackgroundTaskDeactivator::class,
'block_uniqid_transformer' => BlockUniqidTransformer::class,
'cli.command_namespace' => Cli\CommandNamespaceRegistration::class,
'cli.optimizer_command' => Cli\OptimizerCommand::class,
'cli.transformer_command' => Cli\TransformerCommand::class,
Expand Down
117 changes: 110 additions & 7 deletions src/BackgroundTask/MonitorCssTransientCaching.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
namespace AmpProject\AmpWP\BackgroundTask;

use AMP_Options_Manager;
use AmpProject\AmpWP\BlockUniqidTransformer;
use AmpProject\AmpWP\Option;
use DateTimeImmutable;
use DateTimeInterface;
Expand Down Expand Up @@ -54,13 +55,40 @@ final class MonitorCssTransientCaching extends RecurringBackgroundTask {
*/
const DEFAULT_SAMPLING_RANGE = 14;

/**
* @string
*/
const WP_VERSION = 'wp_version';

/**
* @string
*/
const GUTENBERG_VERSION = 'gutenberg_version';

/**
* @var BlockUniqidTransformer
*/
private $block_uniqid_transformer;

/**
* Constructor.
*
* @param BackgroundTaskDeactivator $background_task_deactivator Deactivator.
* @param BlockUniqidTransformer $block_uniqid_transformer Transformer.
*/
public function __construct( BackgroundTaskDeactivator $background_task_deactivator, BlockUniqidTransformer $block_uniqid_transformer ) {
parent::__construct( $background_task_deactivator );
$this->block_uniqid_transformer = $block_uniqid_transformer;
}

/**
* Register the service with the system.
*
* @return void
*/
public function register() {
add_action( 'amp_plugin_update', [ $this, 'handle_plugin_update' ] );
add_filter( 'amp_options_updating', [ $this, 'sanitize_disabled_option' ], 10, 2 );
parent::register();
}

Expand Down Expand Up @@ -133,15 +161,60 @@ public function process( ...$args ) {
*
* @return bool Whether transient caching of stylesheets is disabled.
*/
private function is_css_transient_caching_disabled() {
return AMP_Options_Manager::get_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
public function is_css_transient_caching_disabled() {
return (bool) AMP_Options_Manager::get_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
}

/**
* Enable transient caching of stylesheets.
*/
public function enable_css_transient_caching() {
AMP_Options_Manager::update_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
}

/**
* Disable transient caching of stylesheets.
*/
private function disable_css_transient_caching() {
AMP_Options_Manager::update_option( Option::DISABLE_CSS_TRANSIENT_CACHING, true );
public function disable_css_transient_caching() {
AMP_Options_Manager::update_option(
Option::DISABLE_CSS_TRANSIENT_CACHING,
[
self::WP_VERSION => get_bloginfo( 'version' ),
self::GUTENBERG_VERSION => defined( 'GUTENBERG_VERSION' ) ? GUTENBERG_VERSION : null,
]
);
}

/**
* Sanitize the option.
*
* @param array $options Existing options.
* @param array $new_options New options.
* @return array Sanitized options.
*/
public function sanitize_disabled_option( $options, $new_options ) {
$value = null;

if ( array_key_exists( Option::DISABLE_CSS_TRANSIENT_CACHING, $new_options ) ) {
$unsanitized_value = $new_options[ Option::DISABLE_CSS_TRANSIENT_CACHING ];

if ( is_bool( $unsanitized_value ) ) {
$value = (bool) $unsanitized_value;
} elseif ( is_array( $unsanitized_value ) ) {
$value = [];
foreach ( wp_array_slice_assoc( $unsanitized_value, [ self::WP_VERSION, self::GUTENBERG_VERSION ] ) as $key => $version ) {
$value[ $key ] = preg_replace( '/[^a-z0-9_\-.]/', '', $version );
}
}
}

if ( empty( $value ) ) {
unset( $options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] );
} else {
$options[ Option::DISABLE_CSS_TRANSIENT_CACHING ] = $value;
}

return $options;
}

/**
Expand All @@ -164,9 +237,39 @@ public function query_css_transient_count() {
* @param string $old_version Old version.
*/
public function handle_plugin_update( $old_version ) {
// Reset the disabling of the CSS caching subsystem when updating from versions 1.5.0 or 1.5.1.
if ( version_compare( $old_version, '1.5.0', '>=' ) && version_compare( $old_version, '1.5.2', '<' ) ) {
AMP_Options_Manager::update_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
// Note: We cannot use the is_css_transient_caching_disabled method because we need to get the underlying stored value.
$disabled = AMP_Options_Manager::get_option( Option::DISABLE_CSS_TRANSIENT_CACHING, false );
if ( empty( $disabled ) ) {
return;
}

// Obtain the version of WordPress and Gutenberg at which time the functionality was disabled, if available.
$wp_version = isset( $disabled[ self::WP_VERSION ] ) ? $disabled[ self::WP_VERSION ] : null;
$gutenberg_version = isset( $disabled[ self::GUTENBERG_VERSION ] ) ? $disabled[ self::GUTENBERG_VERSION ] : null;

if (
// Reset the disabling of the CSS caching subsystem when updating from versions 1.5.0 or 1.5.1.
(
version_compare( $old_version, '1.5.0', '>=' )
&&
version_compare( $old_version, '1.5.2', '<' )
)
||
// Reset when it was disabled prior to the versions of WP/Gutenberg being captured,
// or if the captured versions were affected at the time of disabling.
(
version_compare( strtok( $old_version, '-' ), '2.2.2', '<' )
&&
(
! is_array( $disabled )
||
$this->block_uniqid_transformer->is_affected_gutenberg_version( $gutenberg_version )
||
$this->block_uniqid_transformer->is_affected_wordpress_version( $wp_version )
Comment on lines +265 to +268
Copy link
Member

Choose a reason for hiding this comment

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

I realize that these conditions probably will never happen because when the $old_version is older than 2.2.2, the $disabled value will always be true here.

)
)
) {
$this->enable_css_transient_caching();
}
}

Expand Down
Loading