Fixed block theme styles without dom - #561
Conversation
83aa6f5 to
a44e0ce
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Composer still requires ext-dom, and raw regex matching can activate style-like text from scripts or comments.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Replaces DOM-based block-theme style extraction with regex matching to prevent crashes when the PHP DOM extension is unavailable.
Changes:
- Extracts and compares inline style tags without
DOMDocument. - Adds block-theme style-flow unit tests.
- Updates wp-env and E2E CI to PHP 8.2.
File summaries
| File | Description |
|---|---|
includes/classes/wp-maintenance-mode.php |
Replaces DOM traversal with regex extraction. |
tests/block-theme-styles-test.php |
Tests head/footer style handling. |
.wp-env.json |
Updates wp-env to PHP 8.2. |
.github/workflows/test-e2e.yml |
Updates E2E setup to PHP 8.2. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The regression test inspects implementation text, and the unrelated PHP runtime upgrades reduce compatibility coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 5/6 changed files
- Comments generated: 3
- Review effort level: Balanced
| uses: shivammathur/setup-php@v2 | ||
| with: | ||
| php-version: "7.4" | ||
| php-version: "8.2" |
There was a problem hiding this comment.
The PHP 7.4 wp-env image is based on Debian Bullseye, whose security repository release file has expired. As a result, apt-get update fails and the image build cannot complete.
Therefore, I bumped the PHP version to 8.2 so the test environment can build and the test case can run successfully. This change is only for the test environment and does not mean that the plugin's minimum supported PHP version has been increased to 8.2.
| { | ||
| "core": null, | ||
| "phpVersion": "7.4", | ||
| "phpVersion": "8.2", |
| $source = file_get_contents( WPMM_PATH . 'includes/classes/wp-maintenance-mode.php' ); | ||
|
|
||
| $this->assertFalse( strpos( $source, 'DOMDocument' ), 'The block-theme style flow must not depend on the DOM extension.' ); |
There was a problem hiding this comment.
I have reverted this assertion in 09058cb, and verified that the custom logic for extracting the <style> tag works correctly by adding assertions for the extracted styles.
There was a problem hiding this comment.
Verified at HEAD. The implementation-text assertion is gone. The focused class passed with 4 tests and 6 behavioral assertions on WordPress 7.1 and PHP 8.3.33.
There was a problem hiding this comment.
Approved: The DOM-free block-theme style flow passed focused callback tests.
Composer no longer requires ext-dom, and the changed trust-boundary check found no security issue.
Validation details
- WordPress 7.1 and PHP 8.3 passed 4 tests and 6 assertions.
- The focused callback check excluded style text inside scripts and comments.
- Composer metadata validated after removing
ext-dom.
Untested areas
- CI owns full suites, builds, E2E checks, and PHP-version matrices.
🤖 Automated review · run code-review-agent_6aa28fa12f0325.41822098.
🤖 Review agent — review posted ✅
- Review: APPROVED · 1 finding
- Head: 09058cb
- Duration: 10 min 20 s
Run code-review-agent_6aa28fa12f0325.41822098 · trail
Summary
Improved the removal of the dependency on the PHP DOM extension (
DOMDocument) in favor of a regex-based approach, which increases compatibility and simplifies the code.Check before Pull Request is ready:
Closes #544