Repository navigation
Fix WordPress 7.1 core integration: Chromium detection, init timing, settings default - #27
Conversation
Core ships wp_get_chromium_major_version() and Gutenberg ships gutenberg_get_chromium_major_version(); the plugin called nonexistent *_chrome_* variants, so DIP-capable browsers were never detected and COEP/COOP headers were sent on Chrome 137+ alongside core's Document-Isolation-Policy header.
The Gutenberg feature gate was checked at file include time, so when this plugin loaded before Gutenberg the check failed and the plugin bailed out entirely. Deferring to plugins_loaded makes activation order irrelevant.
The default was computed from the User-Agent of whoever saved the settings and then persisted site-wide for all browsers. DIP detection already happens per-request in csme_should_use_coep_coop(), so the stored option can simply default to enabled.
📝 WalkthroughWalkthroughThe plugin now initializes on ChangesInitialization and isolation behavior
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant WordPress
participant csme_init
participant PluginModules
participant ChromiumHelpers
WordPress->>csme_init: fires plugins_loaded
csme_init->>PluginModules: loads settings and isolation code
PluginModules->>ChromiumHelpers: retrieves Chromium major version
ChromiumHelpers-->>PluginModules: returns version for DIP decision
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/Test_Should_Use_Coep_Coop.php (1)
31-68: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPrevent test pollution from conditionally defined global functions.
Defining a function (e.g.,
wp_get_chromium_major_versionorgutenberg_get_chromium_major_version) inside a test method defines it globally for the remainder of the PHP process.
Becausetest_returns_true_when_chromium_below_137()andtest_returns_false_when_chromium_137_or_higher()try to define the same function with different return values, whichever test runs second will encounterfunction_exists() === trueand silently reuse the value defined by the first test. This causes test pollution, leading to false positives or test failures depending on execution order. It also pollutes other test classes (likeTest_Settings).Consider using PHPUnit's process isolation for these tests to prevent global state leakage, or utilize a mocking library if available in your test suite.
♻️ Proposed fix using process isolation
/** * Returns false when Chromium version is 137+ (DIP is used). + * + * `@runInSeparateProcess` + * `@preserveGlobalState` disabled */ public function test_returns_false_when_chromium_137_or_higher() { if ( ! function_exists( 'wp_get_chromium_major_version' ) ) { function wp_get_chromium_major_version() { return 140; } } $this->assertFalse( csme_should_use_coep_coop() ); } /** * Returns true when Chromium version is below 137 (no DIP support). + * + * `@runInSeparateProcess` + * `@preserveGlobalState` disabled */ public function test_returns_true_when_chromium_below_137() { if ( ! function_exists( 'wp_get_chromium_major_version' ) ) { function wp_get_chromium_major_version() { return 130; } } $this->assertTrue( csme_should_use_coep_coop() ); } /** * Falls back to the Gutenberg plugin's version function when core's is absent. + * + * `@runInSeparateProcess` + * `@preserveGlobalState` disabled */ public function test_falls_back_to_gutenberg_version_function() { if ( ! function_exists( 'gutenberg_get_chromium_major_version' ) ) { function gutenberg_get_chromium_major_version() { return 140; } } $this->assertFalse( csme_should_use_coep_coop() ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/Test_Should_Use_Coep_Coop.php` around lines 31 - 68, Prevent global function definitions in test_returns_false_when_chromium_137_or_higher, test_returns_true_when_chromium_below_137, and test_falls_back_to_gutenberg_version_function from leaking between tests by enabling PHPUnit process isolation for these test methods or the containing test class. Preserve each test’s existing version value and assertions while ensuring they execute in isolated PHP processes.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@tests/Test_Should_Use_Coep_Coop.php`:
- Around line 31-68: Prevent global function definitions in
test_returns_false_when_chromium_137_or_higher,
test_returns_true_when_chromium_below_137, and
test_falls_back_to_gutenberg_version_function from leaking between tests by
enabling PHPUnit process isolation for these test methods or the containing test
class. Preserve each test’s existing version value and assertions while ensuring
they execute in isolated PHP processes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6f704cc4-7551-4440-be76-75ca9751becf
📒 Files selected for processing (6)
client-side-media-experiments.phpincludes/cross-origin-isolation.phpincludes/settings.phptests/Test_Get_Enabled_Default.phptests/Test_Settings.phptests/Test_Should_Use_Coep_Coop.php
💤 Files with no reviewable changes (1)
- tests/Test_Get_Enabled_Default.php
Fixes #23. Part of the 7.1 readiness plan in #25.
Summary
wp_get_chromium_major_version()and Gutenberg shipsgutenberg_get_chromium_major_version(); the plugin called nonexistent*_chrome_*variants. Because detection always failed, COEP/COOP headers were emitted on Chrome 137+ in addition to core'sDocument-Isolation-Policyheader. The plugin is now inert on DIP-capable browsers.plugins_loaded: the Gutenberg feature gate was checked at file include time, so activation order determined whether the plugin worked at all.csme_enabledpreviously defaulted from the User-Agent of whoever saved the settings, persisting that browser's value site-wide. The option now defaults to 1; per-request DIP detection stays incsme_should_use_coep_coop().csme_get_enabled_default()is removed.Tests
Test_Should_Use_Coep_Coopfor the renamed functions, including a new Gutenberg-fallback case and a Chromium < 137 case.Test_Get_Enabled_DefaultwithTest_Settings, which verifies the registered default is 1 and the option gatescsme_should_use_coep_coop().Test_Output_Bufferfailures are pre-existing on main and environment-specific (no xdebug locally, soheaders_list()is empty under CLI).Test plan
COOP: same-origin+COEP: credentialless,window.crossOriginIsolated === trueCOOP: same-origin+COEP: require-corp,window.crossOriginIsolated === trueSummary by CodeRabbit
Bug Fixes
Tests