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

delawski · 2022-03-01T14:15:39Z

Summary

This transforms hash-based block class names into a more predictable class names based on wp_unique_id(). This should allow for re-enabling automatic parsed CSS transient caching.

Checklist

My code is tested and passes existing tests.
My code follows the Engineering Guidelines (updates are often made to the guidelines, check it out periodically).

github-actions · 2022-03-01T14:18:46Z

Plugin builds for d87b78b are ready 🛎️!

Download development build
Download production build

delawski · 2022-03-01T14:22:00Z

src/BlockUniqidClassNameTransformer.php

+	const CLASS_NAME_PREFIXES = [
+		'wp-container-',
+		'wp-duotone-',
+	];


Note that WordPress/gutenberg#38891 covers 2 more class name prefixes:

wp-block-archives- - the uniqid-based class names are already sanitized and there's no need to transform them here.

wp-elements- - I couldn't find a way to render a block with such a class name so I skipped it here.

wp-elements- - I couldn't find a way to render a block with such a class name so I skipped it here.

It appears that it is limited full site editing. I haven't been able to reproduce it yet myself.

I found a way to reproduce it. I went to the Site Editor, selected the paragraph in the footer:

I went to the block's settings:

I opened the menu and selected Link:

Then I selected one of the preset colors:

Then on the frontend, the footer template part's markup is:

<footer class="wp-block-template-part"> <div class="wp-container-622941a2cc259 wp-block-group alignfull"> <div class="wp-container-622941a2cb2af wp-block-group alignwide" style="padding-top:4rem;padding-bottom:4rem"> <h1 class="has-text-color has-primary-color has-background has-secondary-background-color wp-block-site-title"> <a href="https://wordpressdev.lndo.site" rel="home" aria-current="page">WordPress Develop</a> </h1> <p class="wp-elements-622941a2ca7ee has-text-align-right has-link-color"> Proudly powered by <a href="https://wordpress.org" rel="nofollow">WordPress</a> </p> </div> </div> </footer>

And there is a style added:

<style>.wp-elements-622941a2ca7ee a{color: var(--wp--preset--color--primary);}</style>

This is in Gutenberg v12.6.1.

delawski · 2022-03-01T14:24:27Z

tests/php/src/BlockUniqidClassNameTransformerTest.php

+		$this->instance->register();
+		$this->assertEquals( PHP_INT_MAX, has_filter( 'render_block', [ $this->instance, 'transform_class_names_in_block_content' ] ) );
+
+		$expected_hook_name = function_exists( 'wp_is_block_theme' ) && wp_is_block_theme() ? 'wp_enqueue_scripts' : 'wp_footer';


This isn't good but it seems that mocking the wp_is_block_theme function would be pretty hard (there are no hooks that would allow us to change the output of this function).

This transforms hash-based block class names into a more predictable class names based on `wp_unique_id()`. This should allow for re-enabling automatic parsed CSS transient caching.

…uniqid-class-names * 'develop' of github.com:ampproject/amp-wp: (166 commits) Test amp_auto_lightbox_disabled being filtered true Ensure consistent application of AMP_Auto_Lightbox_Disable_Sanitizer Copy referrerpolicy attribute from img to amp-pixel Improve nav menu E2E tests by creating test menu before each test suite Disable lightbox for images by default Bail out if there is no menu location or it is already selected Add E2E tests for Twenty Twenty-Two header nav menu block Test Twenty Twenty search modal on mobile breakpoint Remove redundant space Use `Extension` class constant for getting `amp-pixel` tag name Add missing replacement string to `str_replace` and reformat Use `::class` constant instead of hardcoded class name Examine parsed URL instead of using regex Remove `amp-pixel` from selector conversion mapping Bump dependabot/fetch-metadata from 1.1.1 to 1.3.0 Improve strings to account for one or more issues Fix type check by passing explicit string as href prop Include SCSS files in the lint-staged config for stylelint Fix stylelint issue Replace `img` with `amp-pixel` when dealing with Facebook tracking pixel ...

… content

westonruter · 2022-03-09T20:57:55Z

I'm finding that the Duotone transformation is not working. When I add Duotone to a given element, it's resulting in an SVG element being added to the footer:

<svg xmlns="http://www.w3.org/2000/svg" viewbox="0 0 0 0" width="0" height="0" focusable="false" role="none" data-amp-original-style="visibility: hidden; position: absolute; left: -9999px; overflow: hidden;" class="amp-wp-bf126db">
	<defs>
		<filter id="wp-duotone-6229131d7a940">
			<fecolormatrix color-interpolation-filters="sRGB" type="matrix" values=" .299 .587 .114 0 0 .299 .587 .114 0 0 .299 .587 .114 0 0 .299 .587 .114 0 0 "></fecolormatrix>
			<fecomponenttransfer color-interpolation-filters="sRGB">
				<fefuncr type="table" tablevalues="0.54901960784314 0.98823529411765"></fefuncr>
				<fefuncg type="table" tablevalues="0 1"></fefuncg>
				<fefuncb type="table" tablevalues="0.71764705882353 0.25490196078431"></fefuncb>
				<fefunca type="table" tablevalues="1 1"></fefunca>
			</fecomponenttransfer>
			<fecomposite in2="SourceGraphic" operator="in"></fecomposite>
		</filter>
	</defs>
</svg>

This is in addition to the block markup:

<figure class="wp-duotone-622912da977ab wp-block-image size-full">
	<img width="640" height="853" src="https://wordpressdev.lndo.site/content/uploads/2021/05/bison-web-stories-poster.jpg" alt="" class="wp-image-3004" srcset="https://wordpressdev.lndo.site/content/uploads/2021/05/bison-web-stories-poster.jpg 640w, https://wordpressdev.lndo.site/content/uploads/2021/05/bison-web-stories-poster-225x300.jpg 225w, https://wordpressdev.lndo.site/content/uploads/2021/05/bison-web-stories-poster-150x200.jpg 150w" sizes="(max-width: 640px) 100vw, 640px">
	<figcaption>Bison</figcaption>
</figure>

And the inline CSS:

<style id="wp-duotone-622912da977ab-inline-css">
	.wp-duotone-622912da977ab img{filter:url('#wp-duotone-622912da977ab') !important;}
</style>

It's the SVG that isn't being accounted for. It's being added via wp_get_duotone_filter_svg() which is printed at wp_footer. I don't see there being any way to manipulate this using filters 😦

Therefore it seems the only way to do the transformations fully is via a sanitizer and not via a render_block filter.

tests/php/src/BlockUniqidTransformerTest.php

westonruter · 2022-03-10T01:19:38Z

tests/php/src/BlockUniqidTransformerTest.php

+	 * @param string|null $expected_style_content
+	 */
+	public function test_transform_class_names( $block_content, $expected_block_content, $style_handle, $style_content, $expected_style_content ) {
+		$this->markTestIncomplete( 'Test neds to migrate over to tests for AMP_Block_Uniqid_Sanitizer.' );


I didn't port the tests over to a sanitizer test class.

I've moved them over in 5c7cffe.

src/BlockUniqidTransformer.php

westonruter · 2022-03-10T01:28:39Z

src/BlockUniqidTransformer.php

+			(
+				self::has_gutenberg_plugin()
+				||
+				version_compare( get_bloginfo( 'version' ), '5.9', '>=' )


Once backported to core this will need to be updated. I suppose we could update it now to just be 6.0:

Suggested change

version_compare( get_bloginfo( 'version' ), '5.9', '>=' )

(

version_compare( get_bloginfo( 'version' ), '5.9', '>=' )

&&

version_compare( get_bloginfo( 'version' ), '6.0', '<' )

)

The 6.0 could be replaced with 5.9.x when that lands.

There's one more thing about WordPress versions. Duotone was first released in 5.8 where it uses a slightly different selector pattern:

<figure class="wp-duotone-filter-622b62d997e58 wp-block-image size-large"> <img … /> </figure> <style> .wp-duotone-filter-622b62d997e58 img { filter: url( #wp-duotone-filter-622b62d997e58 ); } </style> <svg xmlns:xlink="http://www.w3.org/1999/xlink" viewBox="0 0 0 0" width="0" height="0" focusable="false" role="none" style="visibility: hidden; position: absolute; left: -9999px; overflow: hidden;"> <defs> <filter id="wp-duotone-filter-622b62d997e58"> … </filter> </defs> </svg>

It seems that we should sanitize this, too (the sanitizer needs to also cover not only wp-duotone- but also wp-duotone-filter-).

This has been addressed in 703e853 (WP versions check) and f68c8e2 (support legacy duotone prefix, i.e. wp-duotone-filter-).

westonruter · 2022-03-10T01:37:15Z

src/BlockUniqidTransformer.php

+ * @since 2.2.2
+ * @internal
+ */
+final class BlockUniqidTransformer implements Conditional, Service, Registerable {


There's one more thing that this service should probably do: detect if parsed CSS transient caching is disabled, and if so re-enable it.

Or rather, this can be done in MonitorCssTransientCaching which already has some of this logic actually:

amp-wp/src/BackgroundTask/MonitorCssTransientCaching.php

Lines 166 to 171 in fe0000b

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 );

}

}

It's just we wouldn't want to reset it every time the plugin is updated. So the current storage of a boolean value may not be sufficient for that.

@westonruter I didn't manage to cover this. Feel free to take over.

src/BlockUniqidTransformer.php

Co-authored-by: Weston Ruter <westonruter@google.com>

…uniqid-class-names * 'develop' of github.com:ampproject/amp-wp: (32 commits) Remove check for wp-dom-ready-js-translations which is no longer output in 6.0-alpha Account for variance in tests for CSS generated by postcss after #6980 Bump lint-staged from 12.3.5 to 12.3.6 Bump postcss from 8.4.11 to 8.4.12 Bump postcss from 8.4.8 to 8.4.11 Remove code that strip whitespace around equal signs from meta Bump eslint-plugin-jsdoc from 38.0.1 to 38.0.4 Bump eslint-plugin-react from 7.29.3 to 7.29.4 Bump @babel/core from 7.17.5 to 7.17.7 Update test data to account for fallback dimensions being provided Remove zero source width/height attrs Update tests to account for native img being merged Use empty check instead of not isset to set unknown class names for consistency Update Gutenberg package dependencies Approve PR after enabling auto-merge Run dependabot auto-merge for minor updates as well as patch updates Only run dependabot auto-merge after build, test, and measure has completed Improve checks for image dimensions extraction Bump eslint-plugin-jsdoc from 37.9.7 to 38.0.1 Bump eslint from 8.10.0 to 8.11.0 ...

westonruter · 2022-03-18T16:45:02Z

src/BackgroundTask/MonitorCssTransientCaching.php

+					||
+					$this->block_uniqid_transformer->is_affected_gutenberg_version( $gutenberg_version )
+					||
+					$this->block_uniqid_transformer->is_affected_wordpress_version( $wp_version )


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.

Co-authored-by: Piotr Delawski <piotr.delawski@gmail.com>

delawski added the Bug Something isn't working label Mar 1, 2022

delawski requested review from westonruter and dhaval-parekh March 1, 2022 14:15

delawski self-assigned this Mar 1, 2022

delawski added this to the v2.2.2 milestone Mar 1, 2022

delawski commented Mar 1, 2022

View reviewed changes

delawski force-pushed the fix/6925-uniqid-class-names branch 3 times, most recently from 3c91cc4 to 4154183 Compare March 1, 2022 16:20

Use wp_unique_id-based block class names instead of uniqid

fcb7244

This transforms hash-based block class names into a more predictable class names based on `wp_unique_id()`. This should allow for re-enabling automatic parsed CSS transient caching.

delawski force-pushed the fix/6925-uniqid-class-names branch from 4154183 to fcb7244 Compare March 1, 2022 17:49

westonruter added 6 commits March 9, 2022 11:00

Utilize static variable for constructed regex

41fa877

Reorganize transform_class_names_in_block_content slightly

54746e3

Add failing test case for class being incorrectly transformed in text…

e7c349f

… content

Restrict replacements to class attribute values in start tags

b441abe

Provide polyfill for wp_unique_id() for WP<5.0.3

473a8e7

Implement AMP_Block_Uniqid_Sanitizer

e41fa08

westonruter reviewed Mar 10, 2022

View reviewed changes

tests/php/src/BlockUniqidTransformerTest.php Outdated Show resolved Hide resolved

westonruter reviewed Mar 10, 2022

View reviewed changes

src/BlockUniqidTransformer.php Outdated Show resolved Hide resolved

westonruter reviewed Mar 10, 2022

View reviewed changes

src/BlockUniqidTransformer.php Outdated Show resolved Hide resolved

delawski and others added 3 commits March 11, 2022 15:07

Use transformer only with some versions of Gutenberg

9fb0510

Co-authored-by: Weston Ruter <westonruter@google.com>

Transform uniqid in WordPress 5.8 through 6.0

703e853

Add support for legacy version of duotone prefix

f68c8e2

delawski and others added 12 commits March 11, 2022 17:06

Test for correct WP and Gutenberg versions

76c1415

Do not check if test is dealing with AMP request

af994b1

Migrate block uniqid tests cases to AMP_Block_Uniqid_Sanitizer

5c7cffe

Extend MonitorCssTransientCaching with BlockUniqidTransformer awareness

605a746

Ignore wp_unique_id() polyfill code for coverage

4a6846a

Add tests for MonitorCssTransientCaching and BlockUniqidTransformer

a8e8f84

Normalize to allow AMP_Block_Uniqid_Sanitizer tests to run in isolation

f95a1af

Use proper coversDefaultClass phpdoc tag

8a8ae96

Restore wp_version global after tests

c27435c

Use data provider for test_handle_plugin_update

35dda9d

Ensure CSS transients disabling is not reset with each version update

d87b78b

westonruter reviewed Mar 18, 2022

View reviewed changes

westonruter approved these changes Mar 18, 2022

View reviewed changes

westonruter merged commit 7dbcde2 into develop Mar 18, 2022

westonruter deleted the fix/6925-uniqid-class-names branch March 18, 2022 17:17

westonruter added a commit that referenced this pull request Mar 18, 2022

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

997b351

Co-authored-by: Piotr Delawski <piotr.delawski@gmail.com>

westonruter added the Changelogged Whether the issue/PR has been added to release notes. label Apr 4, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

delawski commented Mar 1, 2022

github-actions bot commented Mar 1, 2022 •

edited

Loading

delawski Mar 1, 2022

westonruter Mar 9, 2022

westonruter Mar 10, 2022

delawski Mar 1, 2022

westonruter commented Mar 9, 2022

westonruter Mar 10, 2022

delawski Mar 11, 2022

westonruter Mar 10, 2022

delawski Mar 11, 2022 •

edited

Loading

delawski Mar 11, 2022

westonruter Mar 10, 2022

delawski Mar 11, 2022

westonruter Mar 18, 2022

	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 );
	}
	}

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

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

Conversation

delawski commented Mar 1, 2022

Summary

Checklist

github-actions bot commented Mar 1, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

westonruter commented Mar 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delawski Mar 11, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

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

github-actions bot commented Mar 1, 2022 •

edited

Loading

delawski Mar 11, 2022 •

edited

Loading