-
Notifications
You must be signed in to change notification settings - Fork 36
fix: Sabberworm dependency collision causes fatal during animation CSS parsing #2958
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
Changes from 1 commit
1dc6109
834608b
237f9a9
4f78a8d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -384,6 +384,20 @@ public function get_animation_css( $blocks ) { | |
| return $style; | ||
| } | ||
|
|
||
| if ( ! self::has_own_css_parser() ) { | ||
| // Another plugin loaded a different php-css-parser release; mixing its | ||
| // classes with the bundled ones fatals at class-link time (issue #2942). | ||
| // Skip the optimization and serve the full stock stylesheet instead. | ||
| // This can run before Blocks_Animation registers the handle, so | ||
| // register it here or the enqueue never prints. | ||
| if ( defined( 'BLOCKS_ANIMATION_URL' ) && ! wp_style_is( 'otter-animation', 'registered' ) ) { | ||
| wp_register_style( 'otter-animation', BLOCKS_ANIMATION_URL . 'build/animation/index.css', array(), OTTER_BLOCKS_VERSION ); | ||
| } | ||
|
|
||
| wp_enqueue_style( 'otter-animation' ); | ||
| return $style; | ||
| } | ||
|
|
||
| $prepared_classes = array( ':root' ); | ||
|
|
||
| foreach ( $classes as $class ) { | ||
|
|
@@ -445,6 +459,42 @@ public function get_animation_css( $blocks ) { | |
| return $style; | ||
| } | ||
|
|
||
| /** | ||
| * Check that every Sabberworm class the animation parser touches resolves to | ||
| * the copy bundled with this plugin. | ||
| * | ||
| * Another active plugin can ship a different php-css-parser release under the | ||
| * same global namespace. Once any of its classes or interfaces is loaded, | ||
| * loading the bundled counterparts fatals at class-link time with a | ||
| * declaration-compatibility error, and that error is not catchable. | ||
| * | ||
| * @return bool | ||
| */ | ||
| public static function has_own_css_parser() { | ||
| $own_vendor = wp_normalize_path( OTTER_BLOCKS_PATH . '/vendor/' ); | ||
|
|
||
| $sentinels = array( | ||
| '\Sabberworm\CSS\Parser', | ||
| '\Sabberworm\CSS\Comment\Commentable', | ||
| '\Sabberworm\CSS\Renderable', | ||
| ); | ||
|
Comment on lines
+492
to
+496
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 237f9a9 — |
||
|
|
||
| foreach ( $sentinels as $sentinel ) { | ||
| if ( ! class_exists( $sentinel ) && ! interface_exists( $sentinel ) ) { | ||
| return false; | ||
| } | ||
|
|
||
| $reflection = new \ReflectionClass( $sentinel ); | ||
| $file = $reflection->getFileName(); | ||
|
|
||
| if ( false === $file || 0 !== strpos( wp_normalize_path( $file ), $own_vendor ) ) { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| /** | ||
| * Get Animation Classes | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| <?php | ||
| /** | ||
| * Typed `Commentable` interface as shipped by php-css-parser 9.x. | ||
| * | ||
| * Lives in a subdirectory so WordPress does not auto-load it as an mu-plugin; | ||
| * otter-e2e-bootstrap.php requires it only while the foreign-Sabberworm | ||
| * scenario (issue #2942) is armed. Once defined, loading Otter's bundled | ||
| * untyped CSSList fatals at class-link time — exactly like a second plugin | ||
| * shipping a newer parser release. | ||
| * | ||
| * @package otter-blocks | ||
| */ | ||
|
|
||
| // phpcs:ignoreFile -- deliberately mirrors the upstream 9.x signatures. | ||
|
|
||
| namespace Sabberworm\CSS\Comment; | ||
|
|
||
| interface Commentable { | ||
| public function addComments( array $comments ): void; | ||
| public function getComments(): array; | ||
| public function setComments( array $comments ): void; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| /** | ||
| * Internal dependencies | ||
| */ | ||
| import { test, expect } from '../fixtures'; | ||
|
|
||
| /** | ||
| * Frontend animation-CSS coverage for https://github.com/Codeinwp/otter-blocks/issues/2942. | ||
| * | ||
| * When another plugin loads a different php-css-parser release, mixing its | ||
| * classes with Otter's bundled copy fatals at class-link time while | ||
| * Base_CSS::get_animation_css() parses the animation stylesheet. The scenario | ||
| * mu-plugin predefines the typed 9.x `Commentable` interface before plugins | ||
| * load; the page must still render, with the full stock animation stylesheet | ||
| * enqueued instead of the optimized inline subset. | ||
| * | ||
| * Each test creates its own fresh post AFTER switching modes: the first | ||
| * singular view generates and caches the post CSS, and cached requests never | ||
| * reach the parser again. | ||
| * | ||
| * Serial project: flips a site-wide scenario flag that affects every request. | ||
| */ | ||
|
|
||
| const POST_CONTENT = `<!-- wp:paragraph {"className":"animated fadeIn"} --> | ||
| <p class="animated fadeIn">Animated collision probe</p> | ||
| <!-- /wp:paragraph -->`; | ||
|
|
||
| const createProbePost = async( requestUtils, title ) => { | ||
| const post = await requestUtils.rest({ | ||
| method: 'POST', | ||
| path: '/wp/v2/posts', | ||
| data: { | ||
| status: 'publish', | ||
| title, | ||
| content: POST_CONTENT | ||
| } | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 834608b — the spec now tracks the created post IDs and hard-deletes them in |
||
| }); | ||
|
|
||
| // Plain query form: independent of the permalink structure. | ||
| return `/?p=${ post.id }`; | ||
| }; | ||
|
|
||
| test.describe( 'Sabberworm collision fallback', () => { | ||
| test.afterAll( async({ requestUtils }) => { | ||
| await requestUtils.rest({ | ||
| method: 'POST', | ||
| path: '/otter-e2e/v1/sabberworm', | ||
| data: { mode: 'own' } | ||
| }); | ||
| }); | ||
|
|
||
| test( 'renders the page with the full stylesheet when a foreign parser is loaded', async({ page, otterUtils, requestUtils }) => { | ||
| await otterUtils.setSabberwormMode( 'foreign' ); | ||
|
|
||
| try { | ||
| const postUrl = await createProbePost( requestUtils, 'Foreign parser probe' ); | ||
|
|
||
| const response = await page.goto( postUrl ); | ||
|
|
||
| expect( response.status() ).toBe( 200 ); | ||
|
|
||
| await expect( page.getByText( 'Animated collision probe' ) ).toBeVisible(); | ||
|
|
||
| // Assert on the server response: the animation frontend script rewrites | ||
| // the block's classes in the live DOM once the animation plays. | ||
| const html = await response.text(); | ||
| expect( html ).toContain( 'animated fadeIn' ); | ||
| expect( html ).not.toContain( 'Fatal error' ); | ||
| expect( html ).not.toContain( 'must be compatible' ); | ||
|
|
||
| // The optimization is skipped, so the stock stylesheet carries the animations. | ||
| expect( html ).toContain( 'otter-animation-css' ); | ||
| expect( html ).toMatch( /animation\/index\.css/ ); | ||
| } finally { | ||
| await otterUtils.setSabberwormMode( 'own' ); | ||
| } | ||
| }); | ||
|
|
||
| test( 'inlines the optimized animation CSS with the bundled parser', async({ page, otterUtils, requestUtils }) => { | ||
| await otterUtils.setSabberwormMode( 'own' ); | ||
|
|
||
| const postUrl = await createProbePost( requestUtils, 'Bundled parser probe' ); | ||
|
|
||
| const response = await page.goto( postUrl ); | ||
|
|
||
| await expect( page.getByText( 'Animated collision probe' ) ).toBeVisible(); | ||
|
|
||
| // The optimized subset is served: the fadeIn keyframe is present without | ||
| // the full stock stylesheet. | ||
| const html = await response.text(); | ||
| expect( html ).toContain( '@keyframes fadeIn' ); | ||
| expect( html ).not.toContain( 'otter-animation-css' ); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| <?php | ||
| /** | ||
| * Standalone sandbox for the Sabberworm dependency-collision regression (issue #2942). | ||
| * | ||
| * Simulates another plugin having already loaded a newer php-css-parser release: | ||
| * its `Commentable` interface declares typed signatures, so loading Otter's | ||
| * bundled untyped `CSSList` fatals at class-link time on PHP 8.1+ with | ||
| * "Declaration of ... addComments(array $aComments) must be compatible ...". | ||
| * Base_CSS::get_animation_css() must detect the foreign copy and fall back to | ||
| * enqueueing the full stock stylesheet instead of parsing. | ||
| * | ||
| * Run in a separate PHP process (no WordPress loaded): | ||
| * php foreign-sabberworm-sandbox.php | ||
| * | ||
| * @package gutenberg-blocks | ||
| */ | ||
|
|
||
| // phpcs:ignoreFile -- multi-namespace sandbox executed outside WordPress. | ||
|
|
||
| namespace Sabberworm\CSS\Comment { | ||
| // The typed interface shape shipped by php-css-parser 9.x. | ||
| interface Commentable { | ||
| public function addComments( array $comments ): void; | ||
| public function getComments(): array; | ||
| public function setComments( array $comments ): void; | ||
| } | ||
| } | ||
|
|
||
| namespace { | ||
| error_reporting( E_ALL ); | ||
|
|
||
| define( 'OTTER_BLOCKS_PATH', dirname( dirname( __DIR__ ) ) ); | ||
| define( 'MONTH_IN_SECONDS', 30 * 24 * 60 * 60 ); | ||
|
|
||
| function get_transient( $key ) { return false; } | ||
| function set_transient( $key, $value, $expiration ) { return true; } | ||
| function get_option( $key, $default_value = false ) { return $default_value; } | ||
| function add_action() {} | ||
| function add_filter() {} | ||
| function apply_filters( $tag, $value ) { return $value; } | ||
| function wp_normalize_path( $path ) { return str_replace( '\\', '/', $path ); } | ||
| function wp_enqueue_style( $handle ) { echo 'ENQUEUED:' . $handle . "\n"; } | ||
|
|
||
| // Minimal autoloader for Otter's bundled parser only — mirrors the situation | ||
| // where Otter's Composer autoloader serves the remaining Sabberworm classes. | ||
| spl_autoload_register( | ||
| function ( $class ) { | ||
| $prefix = 'Sabberworm\\CSS\\'; | ||
| if ( 0 !== strpos( $class, $prefix ) ) { | ||
| return; | ||
| } | ||
| $file = OTTER_BLOCKS_PATH . '/vendor/sabberworm/php-css-parser/src/' . str_replace( '\\', '/', substr( $class, strlen( $prefix ) ) ) . '.php'; | ||
| if ( is_file( $file ) ) { | ||
| require $file; | ||
| } | ||
| } | ||
| ); | ||
|
|
||
| require OTTER_BLOCKS_PATH . '/inc/class-base-css.php'; | ||
|
|
||
| $base = new \ThemeIsle\GutenbergBlocks\Base_CSS(); | ||
| $blocks = array( | ||
| array( | ||
| 'blockName' => 'core/paragraph', | ||
| 'attrs' => array( 'className' => 'animated fadeIn' ), | ||
| ), | ||
| ); | ||
|
|
||
| $css = $base->get_animation_css( $blocks ); | ||
|
|
||
| echo 'CSS_LENGTH:' . strlen( (string) $css ) . "\n"; | ||
| echo "REQUEST COMPLETED WITHOUT FATAL\n"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 834608b — you're right, the fallback died with the cache. The delivery now lives in
Blocks_Animation::frontend_load(), which runs on every request that renders ananimatedblock regardless of the generated-CSS cache;get_animation_css()keeps only the crash guard. The e2e spec now uses a post with non-empty Otter CSS and asserts the stock stylesheet is still enqueued on a second, cache-served request.