Fixed elementor global color handling for not-found errors - #4635
Conversation
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
Review effort: Lite
Findings: 1
Open (3)
What changed in this PR
Updates Neve’s Elementor compatibility to treat Elementor’s global_not_found error as an expected case for Neve global color IDs, with added unit tests to validate fallback behavior.
Changes:
- Adjusted
alter_global_colors_front_end()to allow processing ofWP_Error('global_not_found')responses. - Added teardown cleanup to prevent static color state leaking between tests.
- Added new tests covering fallback resolution for Neve palette colors, custom Neve colors, and pass-through for unknown IDs.
| File | Description |
|---|---|
| tests/test-elementor-compatibility.php | Adds test helpers, teardown state reset, and new cases for global_not_found fallback behavior. |
| inc/compatibility/elementor.php | Changes error-handling logic to process global_not_found rather than early-returning. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| // Elementor never knows Neve IDs, so not-found is expected here. | ||
| if ( is_wp_error( $response ) && 'global_not_found' !== $response->get_error_code() ) { | ||
| return $response; |
There was a problem hiding this comment.
Not applied. After the guard, the function only reads $request and its own colour map; it never calls a method on $response. For an ID Neve doesn't own, it returns the original error at https://github.com/Codeinwp/neve/blob/bugfix/4633/inc/compatibility/elementor.php#L168-L170. For a Neve ID, it builds a new WP_REST_Response at https://github.com/Codeinwp/neve/blob/bugfix/4633/inc/compatibility/elementor.php#L172. That is already the branching the comment asks for, and test_global_colors_front_end_passes_through_not_found_for_unknown_color covers the pass-through.
| public function tearDown(): void { | ||
| $custom_colors = new ReflectionProperty( \Neve\Compatibility\Elementor::class, 'custom_global_colors' ); | ||
| $custom_colors->setAccessible( true ); | ||
| $custom_colors->setValue( null, null ); |
There was a problem hiding this comment.
Not applied. $custom_global_colors has no type and defaults to null https://github.com/Codeinwp/neve/blob/bugfix/4633/inc/compatibility/elementor.php#L56, so resetting it to null restores the declared default and can't throw a TypeError. Resetting to [] would leak state instead: init() loads the colours with ?? Mods::get(...), so an [] left behind would stop later tests from loading the saved colours.
There was a problem hiding this comment.
🟡 Changes requested
The fix works for color routes, but it also turns typography not-found errors into color responses. Restrict the conversion to color routes and tighten the test helper.
Validation details
- Files reviewed: 2/2 changed files.
- PHPUnit 9.6, PHP 8.3, WordPress 7.1.2:
TestElementorCompatibilitypasses 7/7 on HEAD. - With the old
elementor.php, the two new resolve tests fail as expected. - Elementor 4.3.2 as admin:
colors/nvprimaryaccentreturns 200 with the palette color. The base returns 404. - Anonymous requests still get 401
rest_forbidden. - A regenerated Elementor post CSS now keeps
var( --e-global-color-nvprimaryaccent ). The base drops it.
Untested areas
- The runtime used empty placeholder build assets because no yarn build ran.
- Browser rendering, Elementor Pro and Neve Pro were not exercised.
🤖 Automated review · run code-review-agent_6ab6199b821662.62379804.
🤖 Review agent — review posted ✅ on f0e2250e · changes requested · 3 findings · 10 min
Run code-review-agent_6ab6199b821662.62379804 · trail
There was a problem hiding this comment.
🟢 Approved
The PR passes. All three earlier findings are fixed, and the colors fix works with Elementor 4.3.2.
Validation details
- Files reviewed: 2/2 changed files.
- PHPUnit 9.6 on PHP 8.3 and WordPress 7.1.2 ran
TestElementorCompatibility. HEAD passed 8 tests. - The same tests with the
pr-basesource failed the two resolve tests. - The new typography test failed with the
f0e2250source. - Elementor 4.3.2 REST calls on HEAD returned 200 for the Neve colors route. The typography route kept its 404.
- Anonymous requests still returned 401.
- Regenerated Elementor post CSS on HEAD contains
--e-global-color-nvprimaryaccent. - The security check comes from run code-review-agent_6ab6199b821662.62379804 and was not rerun. The new commit only narrows the conversion.
Untested areas
- The site used empty placeholder build assets because no frontend build ran. The editor screen was not opened.
🤖 Automated review · run code-review-agent_6ab631480a67b0.82092786.
🤖 Review agent — review posted ✅ on 83f6cdff · approved · 0 findings · 5 min
Run code-review-agent_6ab631480a67b0.82092786 · trail
|
🎉 This PR is included in version 4.2.14 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
pirate-bot
added
the
released
Summary
Improves the handling of Elementor's "not found" errors for global color requests in Neve, ensuring that missing Neve-specific colors are resolved gracefully while unrelated errors are still passed through. It also expands test coverage to verify these behaviors and adds better test isolation.
Check before Pull Request is ready:
Closes #4633