-
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
Open
lucadobrescu
wants to merge
4
commits into
fix/2954-autoload-missing-class
from
fix/2942-sabberworm-collision
Open
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
1dc6109
fix: fatal when another plugin loads a different php-css-parser release
834608b
fix: serve the stock animation stylesheet from the frontend loader on…
237f9a9
fix: reject any preloaded foreign Sabberworm symbol, not only the sen…
4f78a8d
fix: match foreign Sabberworm declarations case-insensitively
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
22 changes: 22 additions & 0 deletions
22
packages/e2e-tests/mu-plugins/includes/foreign-sabberworm-interface.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| 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; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
131 changes: 131 additions & 0 deletions
131
src/blocks/test/e2e/blocks/sabberworm-collision.spec.js
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| /** | ||
| * 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 — 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 | ||
| * reach the parser again. | ||
| * | ||
| * Serial project: flips a site-wide scenario flag that affects every request. | ||
| */ | ||
|
|
||
| // 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 = `<!-- wp:paragraph {"className":"animated fadeIn"} --> | ||
| <p class="animated fadeIn">Animated collision probe</p> | ||
| <!-- /wp:paragraph --> | ||
|
|
||
| <!-- wp:themeisle-blocks/progress-bar {"id":"wp-block-themeisle-blocks-progress-bar-e2e2942","title":"Collision probe","percentage":75,"titleColor":"#123abc","height":36} --> | ||
| <div id="wp-block-themeisle-blocks-progress-bar-e2e2942" class="wp-block-themeisle-blocks-progress-bar"><div class="wp-block-themeisle-blocks-progress-bar__title">Collision probe</div><div class="wp-block-themeisle-blocks-progress-bar__area"><div class="wp-block-themeisle-blocks-progress-bar__area__bar"></div></div></div> | ||
| <!-- /wp:themeisle-blocks/progress-bar -->`; | ||
|
|
||
| const OWN_POST_CONTENT = `<!-- wp:paragraph {"className":"animated fadeIn"} --> | ||
| <p class="animated fadeIn">Animated collision probe</p> | ||
| <!-- /wp:paragraph -->`; | ||
|
|
||
| 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( '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', FOREIGN_POST_CONTENT ); | ||
|
|
||
| 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/ ); | ||
|
|
||
| // 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' ); | ||
| } | ||
| }); | ||
|
|
||
| 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', OWN_POST_CONTENT ); | ||
|
|
||
| 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' ); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 237f9a9 —
has_own_css_parser()now scans every already-declared class, interface, and trait underSabberworm\CSSand rejects any that does not resolve to the bundled vendor directory, before the sentinel checks can autoload anything (which also addresses the ordering concern aboutParserloading ahead of the interface check). The sentinels remain only as forced entry-point resolution for the nothing-loaded-yet case. Regression coverage added: the sandbox's newoutputformatscenario preloads a foreign non-sentinel class — it fatals against the sentinel-only guard and passes now.