From cc7695033d99832e76e6b0b6084dee9ab22ea642 Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 3 Aug 2026 13:23:29 +0300 Subject: [PATCH 1/6] fix: fatal when another plugin loads a different php-css-parser release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Base_CSS::get_animation_css() parsed the animation stylesheet with the bundled Sabberworm parser even when another plugin had already loaded classes from a different release of it. Mixing the two fatals at class-link time ("Declaration of CSSList::addComments... must be compatible") and that error is not catchable, so a guard now verifies every sentinel class resolves to Otter's own vendor directory before anything parser-related loads — including the transient read, which stores parser objects. When the guard fails, the full stock otter-animation stylesheet is enqueued instead, matching the optimize-off delivery path. Adds an isolated-process PHPUnit regression test and a frontend e2e spec covering both the foreign-parser fallback and the bundled-parser optimized path. Fixes #2942 Co-Authored-By: Claude Fable 5 --- inc/class-base-css.php | 50 ++++++++++ .../includes/foreign-sabberworm-interface.php | 22 +++++ .../mu-plugins/otter-e2e-bootstrap.php | 52 +++++++++++ .../e2e/blocks/sabberworm-collision.spec.js | 93 +++++++++++++++++++ src/blocks/test/e2e/fixtures.ts | 8 ++ src/blocks/test/e2e/playwright.config.js | 5 +- tests/php/foreign-sabberworm-sandbox.php | 73 +++++++++++++++ tests/test-animation-css.php | 70 ++++++++++++++ 8 files changed, 372 insertions(+), 1 deletion(-) create mode 100644 packages/e2e-tests/mu-plugins/includes/foreign-sabberworm-interface.php create mode 100644 src/blocks/test/e2e/blocks/sabberworm-collision.spec.js create mode 100644 tests/php/foreign-sabberworm-sandbox.php create mode 100644 tests/test-animation-css.php diff --git a/inc/class-base-css.php b/inc/class-base-css.php index 8d804cc3b..e6504cab9 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -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', + ); + + 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 * diff --git a/packages/e2e-tests/mu-plugins/includes/foreign-sabberworm-interface.php b/packages/e2e-tests/mu-plugins/includes/foreign-sabberworm-interface.php new file mode 100644 index 000000000..308bace7c --- /dev/null +++ b/packages/e2e-tests/mu-plugins/includes/foreign-sabberworm-interface.php @@ -0,0 +1,22 @@ + \WP_REST_Server::CREATABLE, + 'permission_callback' => __NAMESPACE__ . '\\require_admin', + 'callback' => function ( \WP_REST_Request $request ) { + $mode = $request->get_param( 'mode' ); + + if ( ! in_array( $mode, array( 'foreign', 'own' ), true ) ) { + return new \WP_Error( + 'otter_e2e_invalid_sabberworm_mode', + 'Mode must be "foreign" or "own".', + array( 'status' => 400 ) + ); + } + + if ( 'foreign' === $mode ) { + update_option( FOREIGN_SABBERWORM_OPTION, true, false ); + } else { + delete_option( FOREIGN_SABBERWORM_OPTION ); + } + + // Force the next frontend request through the parse path. + delete_transient( 'otter_animations_parsed' ); + + return rest_ensure_response( array( 'ok' => true ) ); + }, + ) + ); + register_rest_route( REST_NAMESPACE, '/widgets/seed', @@ -1497,6 +1548,7 @@ function () { delete_option( CAPTCHA_MODE_OPTION ); delete_option( OPENAI_STUB_OPTION ); delete_option( FS_BLOCKED_OPTION ); + delete_option( FOREIGN_SABBERWORM_OPTION ); cleanup_form_records(); return rest_ensure_response( array( 'ok' => true ) ); }, diff --git a/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js b/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js new file mode 100644 index 000000000..39d5cdff5 --- /dev/null +++ b/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js @@ -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 = ` +

Animated collision probe

+`; + +const createProbePost = async( requestUtils, title ) => { + const post = await requestUtils.rest({ + method: 'POST', + path: '/wp/v2/posts', + data: { + status: 'publish', + title, + content: POST_CONTENT + } + }); + + // 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' ); + }); +}); diff --git a/src/blocks/test/e2e/fixtures.ts b/src/blocks/test/e2e/fixtures.ts index dd13ff773..374f22a75 100644 --- a/src/blocks/test/e2e/fixtures.ts +++ b/src/blocks/test/e2e/fixtures.ts @@ -70,6 +70,13 @@ export type OtterUtils = { /** Remove the seeded widget, its CSS file/options, and the filesystem block. */ cleanupOtterWidget: () => Promise; + /** + * 'foreign' predefines a typed php-css-parser 9.x Commentable interface before + * plugins load (issue #2942 scenario); 'own' restores the bundled parser. + * Both modes clear the parsed-animations transient. + */ + setSabberwormMode: ( mode: 'foreign' | 'own' ) => Promise; + /** All stored Submission Records with their Delivery Status meta. */ getFormRecords: () => Promise; @@ -105,6 +112,7 @@ export const test = base.extend<{ otterUtils: OtterUtils }>({ setFilesystemMode: ( mode ) => call( 'filesystem', { mode }), seedOtterWidget: ( sidebar ) => call( 'widgets/seed', sidebar ? { sidebar } : undefined ), cleanupOtterWidget: () => call( 'widgets/cleanup' ), + setSabberwormMode: ( mode ) => call( 'sabberworm', { mode }), getFormRecords: () => call( 'form/records' ) as Promise, cleanupFormRecords: () => call( 'form/records/cleanup' ) }); diff --git a/src/blocks/test/e2e/playwright.config.js b/src/blocks/test/e2e/playwright.config.js index 79ec41825..7e8340d59 100644 --- a/src/blocks/test/e2e/playwright.config.js +++ b/src/blocks/test/e2e/playwright.config.js @@ -63,7 +63,10 @@ const SERIAL_SPECS = [ '**/blocks/widgets-css-frontend.spec.js', // Flips a site-wide flag that breaks Otter's autoloader for every request. - '**/blocks/autoloader-resilience.spec.js' + '**/blocks/autoloader-resilience.spec.js', + + // Flips a site-wide flag that injects a foreign Sabberworm interface for every request. + '**/blocks/sabberworm-collision.spec.js' ]; const config = defineConfig({ diff --git a/tests/php/foreign-sabberworm-sandbox.php b/tests/php/foreign-sabberworm-sandbox.php new file mode 100644 index 000000000..93e652305 --- /dev/null +++ b/tests/php/foreign-sabberworm-sandbox.php @@ -0,0 +1,73 @@ + '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"; +} diff --git a/tests/test-animation-css.php b/tests/test-animation-css.php new file mode 100644 index 000000000..a5cc4ae9a --- /dev/null +++ b/tests/test-animation-css.php @@ -0,0 +1,70 @@ +> + */ + private function animated_blocks() { + return array( + array( + 'blockName' => 'core/paragraph', + 'attrs' => array( 'className' => 'animated fadeIn' ), + ), + ); + } + + /** + * With a foreign typed `Commentable` interface already loaded, parsing must be + * skipped in favor of the stock stylesheet — not fatal at class-link time. + * + * The collision poisons every later Sabberworm use in the process, so the + * scenario runs in a separate PHP process against a predefined 9.x interface. + */ + public function test_get_animation_css_falls_back_when_foreign_parser_is_loaded() { + $sandbox = __DIR__ . '/php/foreign-sabberworm-sandbox.php'; + + $command = escapeshellarg( PHP_BINARY ) . ' -d display_errors=1 ' . escapeshellarg( $sandbox ) . ' 2>&1'; + + exec( $command, $output, $exit_code ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_exec + + $output = implode( "\n", $output ); + + $this->assertSame( 0, $exit_code, 'The sandbox request fataled instead of degrading gracefully: ' . $output ); + $this->assertStringContainsString( 'ENQUEUED:otter-animation', $output, 'The full stock stylesheet should be enqueued as the fallback: ' . $output ); + $this->assertStringContainsString( 'REQUEST COMPLETED WITHOUT FATAL', $output ); + $this->assertStringNotContainsString( 'must be compatible', $output ); + } + + /** + * Sanity check: with only the bundled parser present, the guard passes and the + * optimized subset is produced. + */ + public function test_get_animation_css_parses_with_bundled_parser() { + $this->assertTrue( Base_CSS::has_own_css_parser() ); + + delete_transient( 'otter_animations_parsed' ); + + $css = ( new Base_CSS() )->get_animation_css( $this->animated_blocks() ); + + $this->assertStringContainsString( 'fadeIn', $css ); + $this->assertStringContainsString( '@keyframes', $css ); + } +} From 3e83b345cac3d90f491414b3f3790bfaf45f41d2 Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 3 Aug 2026 14:21:51 +0300 Subject: [PATCH 2/6] fix: serve the stock animation stylesheet from the frontend loader on parser collision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review: the get_animation_css() fallback enqueue was lost once the generated post CSS was cached — later requests never reach the parser path, and the cached CSS carries no animation rules. Move the fallback delivery into Blocks_Animation::frontend_load(), which runs on every request that renders an animated block, and cover a cached-CSS reload in the e2e spec. Also delete the spec's probe posts in afterAll. Co-Authored-By: Claude Fable 5 --- inc/class-base-css.php | 11 +-- inc/class-blocks-animation.php | 10 ++- .../e2e/blocks/sabberworm-collision.spec.js | 76 ++++++++++++++----- tests/test-animation-css.php | 2 +- 4 files changed, 70 insertions(+), 29 deletions(-) diff --git a/inc/class-base-css.php b/inc/class-base-css.php index e6504cab9..ee393c57c 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -387,14 +387,9 @@ public function get_animation_css( $blocks ) { 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' ); + // Skip the optimization — Blocks_Animation::frontend_load() serves the + // full stock stylesheet whenever this guard fails, so the animation + // rules never depend on the optimized fragment generated here. return $style; } diff --git a/inc/class-blocks-animation.php b/inc/class-blocks-animation.php index 78181b08c..badbffcdb 100644 --- a/inc/class-blocks-animation.php +++ b/inc/class-blocks-animation.php @@ -172,7 +172,15 @@ public function frontend_load( $block_content, $block ) { } if ( ! self::$scripts_loaded['animation'] && strpos( $block_content, 'animated' ) ) { - if ( ! defined( 'OTTER_BLOCKS_VERSION' ) || ( defined( 'OTTER_BLOCKS_VERSION' ) && ! get_option( 'themeisle_blocks_settings_optimize_animations_css', true ) ) ) { + // The stock stylesheet also serves pages whose generated CSS was built + // while a foreign php-css-parser copy blocked the optimization (issue + // #2942) — the cached CSS carries no animation rules, so this delivery + // must not depend on the optimized path having run. + if ( + ! defined( 'OTTER_BLOCKS_VERSION' ) || + ! get_option( 'themeisle_blocks_settings_optimize_animations_css', true ) || + ! Base_CSS::has_own_css_parser() + ) { wp_enqueue_style( 'otter-animation' ); } diff --git a/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js b/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js index 39d5cdff5..99f470a4f 100644 --- a/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js +++ b/src/blocks/test/e2e/blocks/sabberworm-collision.spec.js @@ -11,7 +11,9 @@ import { test, expect } from '../fixtures'; * 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. + * enqueued instead of the optimized inline subset — including on later + * requests served from the generated post-CSS cache, which carries no + * animation rules while the guard fails. * * Each test creates its own fresh post AFTER switching modes: the first * singular view generates and caches the post CSS, and cached requests never @@ -20,39 +22,68 @@ import { test, expect } from '../fixtures'; * Serial project: flips a site-wide scenario flag that affects every request. */ -const POST_CONTENT = ` +// The Progress Bar makes the generated post CSS non-empty, so the second +// request is served from the cached stylesheet file. +const FOREIGN_POST_CONTENT = `

Animated collision probe

-`; + -const createProbePost = async( requestUtils, title ) => { - const post = await requestUtils.rest({ - method: 'POST', - path: '/wp/v2/posts', - data: { - status: 'publish', - title, - content: POST_CONTENT - } - }); + +
Collision probe
+`; - // Plain query form: independent of the permalink structure. - return `/?p=${ post.id }`; -}; +const OWN_POST_CONTENT = ` +

Animated collision probe

+`; test.describe( 'Sabberworm collision fallback', () => { + const createdPosts = []; + + const createProbePost = async( requestUtils, title, content ) => { + const post = await requestUtils.rest({ + method: 'POST', + path: '/wp/v2/posts', + data: { + status: 'publish', + title, + content + } + }); + + createdPosts.push( post.id ); + + // Plain query form: independent of the permalink structure. + return `/?p=${ post.id }`; + }; + test.afterAll( async({ requestUtils }) => { await requestUtils.rest({ method: 'POST', path: '/otter-e2e/v1/sabberworm', data: { mode: 'own' } }); + + // Only this spec's own posts — other specs run against the same site. + // Best-effort per post: one failed request must not orphan the rest. + while ( createdPosts.length ) { + const postId = createdPosts.pop(); + try { + await requestUtils.rest({ + method: 'DELETE', + path: `/wp/v2/posts/${ postId }`, + params: { force: true } + }); + } catch ( error ) { + console.warn( `Could not delete post ${ postId }:`, error.message ); + } + } }); - test( 'renders the page with the full stylesheet when a foreign parser is loaded', async({ page, otterUtils, requestUtils }) => { + test( 'serves the full stylesheet when a foreign parser is loaded, also from the CSS cache', async({ page, otterUtils, requestUtils }) => { await otterUtils.setSabberwormMode( 'foreign' ); try { - const postUrl = await createProbePost( requestUtils, 'Foreign parser probe' ); + const postUrl = await createProbePost( requestUtils, 'Foreign parser probe', FOREIGN_POST_CONTENT ); const response = await page.goto( postUrl ); @@ -70,6 +101,13 @@ test.describe( 'Sabberworm collision fallback', () => { // The optimization is skipped, so the stock stylesheet carries the animations. expect( html ).toContain( 'otter-animation-css' ); expect( html ).toMatch( /animation\/index\.css/ ); + + // The first request generated and cached the post CSS without animation + // rules; the fallback must survive requests served from that cache. + const cachedResponse = await page.goto( postUrl ); + const cachedHtml = await cachedResponse.text(); + expect( cachedHtml ).toContain( 'otter-animation-css' ); + expect( cachedHtml ).not.toContain( 'Fatal error' ); } finally { await otterUtils.setSabberwormMode( 'own' ); } @@ -78,7 +116,7 @@ test.describe( 'Sabberworm collision fallback', () => { 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 postUrl = await createProbePost( requestUtils, 'Bundled parser probe', OWN_POST_CONTENT ); const response = await page.goto( postUrl ); diff --git a/tests/test-animation-css.php b/tests/test-animation-css.php index a5cc4ae9a..eb057a860 100644 --- a/tests/test-animation-css.php +++ b/tests/test-animation-css.php @@ -48,7 +48,7 @@ public function test_get_animation_css_falls_back_when_foreign_parser_is_loaded( $output = implode( "\n", $output ); $this->assertSame( 0, $exit_code, 'The sandbox request fataled instead of degrading gracefully: ' . $output ); - $this->assertStringContainsString( 'ENQUEUED:otter-animation', $output, 'The full stock stylesheet should be enqueued as the fallback: ' . $output ); + $this->assertStringContainsString( 'CSS_LENGTH:0', $output, 'The optimization should be skipped when a foreign parser is loaded: ' . $output ); $this->assertStringContainsString( 'REQUEST COMPLETED WITHOUT FATAL', $output ); $this->assertStringNotContainsString( 'must be compatible', $output ); } From b2d3f81e97446feba66d2221a82329702e206efd Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 3 Aug 2026 14:39:33 +0300 Subject: [PATCH 3/6] fix: reject any preloaded foreign Sabberworm symbol, not only the sentinels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copilot review: the parser touches more classes than the three sentinels (OutputFormat, DeclarationBlock, KeyFrame, and the cached object graph), so a foreign copy of any of them slipped past the guard and still mixed releases. has_own_css_parser() now first rejects every already-declared class, interface, or trait under Sabberworm\CSS that does not resolve to the bundled vendor directory — before the sentinel checks can autoload anything — and only then resolves the sentinel entry points. The sandbox gains an outputformat scenario covering a foreign non-sentinel class. Co-Authored-By: Claude Fable 5 --- inc/class-base-css.php | 38 +++++++++++++++++--- tests/php/foreign-sabberworm-sandbox.php | 45 +++++++++++++++++------- tests/test-animation-css.php | 29 +++++++++++++-- 3 files changed, 92 insertions(+), 20 deletions(-) diff --git a/inc/class-base-css.php b/inc/class-base-css.php index ee393c57c..6bd9ec055 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -467,7 +467,26 @@ public function get_animation_css( $blocks ) { */ public static function has_own_css_parser() { $own_vendor = wp_normalize_path( OTTER_BLOCKS_PATH . '/vendor/' ); + $prefix = 'Sabberworm\\CSS\\'; + // Reject anything already in memory from a foreign copy first, before the + // sentinel checks below can autoload any bundled class: the parser touches + // more classes than the sentinels (OutputFormat, KeyFrame, the cached + // object graph...), and any preloaded foreign one poisons the process. + $declared = array_merge( get_declared_classes(), get_declared_interfaces(), get_declared_traits() ); + + foreach ( $declared as $declared_name ) { + if ( 0 !== strpos( $declared_name, $prefix ) ) { + continue; + } + + if ( ! self::is_bundled_class( $declared_name, $own_vendor ) ) { + return false; + } + } + + // Entry points nothing may have loaded yet: whichever autoloader resolves + // them must serve the bundled copy. $sentinels = array( '\Sabberworm\CSS\Parser', '\Sabberworm\CSS\Comment\Commentable', @@ -479,10 +498,7 @@ public static function has_own_css_parser() { return false; } - $reflection = new \ReflectionClass( $sentinel ); - $file = $reflection->getFileName(); - - if ( false === $file || 0 !== strpos( wp_normalize_path( $file ), $own_vendor ) ) { + if ( ! self::is_bundled_class( $sentinel, $own_vendor ) ) { return false; } } @@ -490,6 +506,20 @@ public static function has_own_css_parser() { return true; } + /** + * Check that a class, interface, or trait was loaded from this plugin's vendor directory. + * + * @param string $name Fully qualified name. + * @param string $own_vendor Normalized path of this plugin's vendor directory. + * @return bool + */ + private static function is_bundled_class( $name, $own_vendor ) { + $reflection = new \ReflectionClass( $name ); + $file = $reflection->getFileName(); + + return false !== $file && 0 === strpos( wp_normalize_path( $file ), $own_vendor ); + } + /** * Get Animation Classes * diff --git a/tests/php/foreign-sabberworm-sandbox.php b/tests/php/foreign-sabberworm-sandbox.php index 93e652305..21c32becd 100644 --- a/tests/php/foreign-sabberworm-sandbox.php +++ b/tests/php/foreign-sabberworm-sandbox.php @@ -2,27 +2,46 @@ /** * 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. + * Simulates another plugin having already loaded classes from a different + * php-css-parser release, which makes mixing in Otter's bundled copy fatal at + * class-link or call time. Base_CSS::get_animation_css() must detect the + * foreign copy and skip the optimization instead of parsing. * * Run in a separate PHP process (no WordPress loaded): - * php foreign-sabberworm-sandbox.php + * php foreign-sabberworm-sandbox.php [commentable|outputformat] + * + * - `commentable` (default): the typed 9.x `Commentable` interface is preloaded — + * loading the bundled untyped `CSSList` then fatals at class-link time on + * PHP 8.1+ with "Declaration of ... must be compatible ...". + * - `outputformat`: a foreign copy of the non-sentinel `OutputFormat` class is + * preloaded — proving the guard rejects any foreign `Sabberworm\CSS` symbol, + * not only its sentinels. * * @package gutenberg-blocks */ // phpcs:ignoreFile -- multi-namespace sandbox executed outside WordPress. +namespace { + $GLOBALS['otter_sandbox_scenario'] = isset( $argv[1] ) ? $argv[1] : 'commentable'; +} + 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; + if ( 'commentable' === $GLOBALS['otter_sandbox_scenario'] ) { + // 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 Sabberworm\CSS { + if ( 'outputformat' === $GLOBALS['otter_sandbox_scenario'] ) { + // A foreign copy of a class the parser uses but the guard's sentinels + // do not cover, as another plugin's autoloader would leave behind. + class OutputFormat {} } } @@ -39,7 +58,7 @@ 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"; } + function wp_enqueue_style( $handle ) {} // Minimal autoloader for Otter's bundled parser only — mirrors the situation // where Otter's Composer autoloader serves the remaining Sabberworm classes. diff --git a/tests/test-animation-css.php b/tests/test-animation-css.php index eb057a860..7bc8e236a 100644 --- a/tests/test-animation-css.php +++ b/tests/test-animation-css.php @@ -39,18 +39,41 @@ private function animated_blocks() { * scenario runs in a separate PHP process against a predefined 9.x interface. */ public function test_get_animation_css_falls_back_when_foreign_parser_is_loaded() { + $output = $this->run_sandbox( 'commentable' ); + + $this->assertStringContainsString( 'CSS_LENGTH:0', $output, 'The optimization should be skipped when a foreign parser is loaded: ' . $output ); + $this->assertStringNotContainsString( 'must be compatible', $output ); + } + + /** + * The guard must reject any preloaded foreign `Sabberworm\CSS` symbol, not + * only its sentinel entry points — here a foreign `OutputFormat` class. + */ + public function test_get_animation_css_falls_back_when_foreign_non_sentinel_class_is_loaded() { + $output = $this->run_sandbox( 'outputformat' ); + + $this->assertStringContainsString( 'CSS_LENGTH:0', $output, 'The optimization should be skipped when any foreign Sabberworm class is loaded: ' . $output ); + } + + /** + * Run the collision sandbox in a separate PHP process and assert it completes. + * + * @param string $scenario Sandbox scenario name. + * @return string Combined process output. + */ + private function run_sandbox( $scenario ) { $sandbox = __DIR__ . '/php/foreign-sabberworm-sandbox.php'; - $command = escapeshellarg( PHP_BINARY ) . ' -d display_errors=1 ' . escapeshellarg( $sandbox ) . ' 2>&1'; + $command = escapeshellarg( PHP_BINARY ) . ' -d display_errors=1 ' . escapeshellarg( $sandbox ) . ' ' . escapeshellarg( $scenario ) . ' 2>&1'; exec( $command, $output, $exit_code ); // phpcs:ignore WordPress.PHP.DiscouragedPHPFunctions.system_calls_exec $output = implode( "\n", $output ); $this->assertSame( 0, $exit_code, 'The sandbox request fataled instead of degrading gracefully: ' . $output ); - $this->assertStringContainsString( 'CSS_LENGTH:0', $output, 'The optimization should be skipped when a foreign parser is loaded: ' . $output ); $this->assertStringContainsString( 'REQUEST COMPLETED WITHOUT FATAL', $output ); - $this->assertStringNotContainsString( 'must be compatible', $output ); + + return $output; } /** From dd2adc6f7e8dea92f9fb839b3bbe3dad19a96177 Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 3 Aug 2026 14:45:58 +0300 Subject: [PATCH 4/6] fix: match foreign Sabberworm declarations case-insensitively Copilot review (suppressed note): PHP class and namespace names are case-insensitive, so a foreign symbol declared with different casing is the same runtime class but slipped past the case-sensitive prefix scan. Use stripos so every colliding declaration is rejected. Co-Authored-By: Claude Fable 5 --- inc/class-base-css.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/inc/class-base-css.php b/inc/class-base-css.php index 6bd9ec055..146269f9b 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -476,7 +476,9 @@ public static function has_own_css_parser() { $declared = array_merge( get_declared_classes(), get_declared_interfaces(), get_declared_traits() ); foreach ( $declared as $declared_name ) { - if ( 0 !== strpos( $declared_name, $prefix ) ) { + // Case-insensitive: PHP class and namespace names are case-insensitive, + // so a foreign copy declared with different casing is the same class. + if ( 0 !== stripos( $declared_name, $prefix ) ) { continue; } From 2b075463068cc6c6f2ce21aaa88a78f4eabc3fa4 Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Mon, 10 Aug 2026 15:16:16 +0300 Subject: [PATCH 5/6] docs: trim animation-CSS collision comments to the load-bearing why Cut the verbose guard and preflight comments in Base_CSS and Blocks_Animation down to the constraint a reader cannot infer: the foreign-parser fatal is uncatchable, so the guard must run before any parser class loads. Also correct the has_own_css_parser() docblock to say it checks every loaded Sabberworm\CSS symbol, not only the ones the parser touches. Co-Authored-By: Claude Opus 4.8 --- inc/class-base-css.php | 28 +++++++++++----------------- inc/class-blocks-animation.php | 6 ++---- 2 files changed, 13 insertions(+), 21 deletions(-) diff --git a/inc/class-base-css.php b/inc/class-base-css.php index 146269f9b..feadbde87 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -385,11 +385,9 @@ public function get_animation_css( $blocks ) { } 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 — Blocks_Animation::frontend_load() serves the - // full stock stylesheet whenever this guard fails, so the animation - // rules never depend on the optimized fragment generated here. + // A foreign php-css-parser release is loaded; parsing now fatals + // uncatchably at class-link time (#2942). The frontend loader serves + // the stock stylesheet instead. return $style; } @@ -455,13 +453,11 @@ public function get_animation_css( $blocks ) { } /** - * Check that every Sabberworm class the animation parser touches resolves to - * the copy bundled with this plugin. + * Check that every loaded Sabberworm\CSS symbol resolves to this plugin's copy. * - * 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. + * Another plugin can ship a different php-css-parser release under the same + * namespace. Once any of its classes loads, loading the bundled counterparts + * fatals at class-link time, and that error is not catchable. * * @return bool */ @@ -469,15 +465,13 @@ public static function has_own_css_parser() { $own_vendor = wp_normalize_path( OTTER_BLOCKS_PATH . '/vendor/' ); $prefix = 'Sabberworm\\CSS\\'; - // Reject anything already in memory from a foreign copy first, before the - // sentinel checks below can autoload any bundled class: the parser touches - // more classes than the sentinels (OutputFormat, KeyFrame, the cached - // object graph...), and any preloaded foreign one poisons the process. + // Reject any foreign copy already in memory before the sentinel checks + // autoload a bundled class: the parser uses more classes than the + // sentinels, and one preloaded foreign symbol poisons the process. $declared = array_merge( get_declared_classes(), get_declared_interfaces(), get_declared_traits() ); foreach ( $declared as $declared_name ) { - // Case-insensitive: PHP class and namespace names are case-insensitive, - // so a foreign copy declared with different casing is the same class. + // PHP class names are case-insensitive; match a foreign copy in any casing. if ( 0 !== stripos( $declared_name, $prefix ) ) { continue; } diff --git a/inc/class-blocks-animation.php b/inc/class-blocks-animation.php index badbffcdb..3bc12d174 100644 --- a/inc/class-blocks-animation.php +++ b/inc/class-blocks-animation.php @@ -172,10 +172,8 @@ public function frontend_load( $block_content, $block ) { } if ( ! self::$scripts_loaded['animation'] && strpos( $block_content, 'animated' ) ) { - // The stock stylesheet also serves pages whose generated CSS was built - // while a foreign php-css-parser copy blocked the optimization (issue - // #2942) — the cached CSS carries no animation rules, so this delivery - // must not depend on the optimized path having run. + // Foreign-parser pages (#2942) cache post-CSS with no animation rules, + // so deliver the stock stylesheet here rather than via the parser path. if ( ! defined( 'OTTER_BLOCKS_VERSION' ) || ! get_option( 'themeisle_blocks_settings_optimize_animations_css', true ) || From 29b77c6c15ceb9d4faef298e871a092190de568c Mon Sep 17 00:00:00 2001 From: Luca Dobrescu Date: Wed, 12 Aug 2026 14:23:32 +0300 Subject: [PATCH 6/6] fix: log when a foreign php-css-parser release disables animation CSS The parser-collision guard silently served the stock animation stylesheet, so a conflicting Sabberworm release looked like the optimization had stopped working for no reason. Log the skip, mirroring the autoload-skip notice from #2956, so the cause is visible in the debug log. Co-Authored-By: Claude Opus 4.8 --- inc/class-base-css.php | 1 + 1 file changed, 1 insertion(+) diff --git a/inc/class-base-css.php b/inc/class-base-css.php index feadbde87..0739628ba 100644 --- a/inc/class-base-css.php +++ b/inc/class-base-css.php @@ -388,6 +388,7 @@ public function get_animation_css( $blocks ) { // A foreign php-css-parser release is loaded; parsing now fatals // uncatchably at class-link time (#2942). The frontend loader serves // the stock stylesheet instead. + error_log( '[Otter Blocks] A conflicting Sabberworm php-css-parser release is loaded; skipping animation CSS optimization and serving the stock stylesheet instead.' ); // phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_error_log return $style; }