Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/push.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,4 @@ jobs:
with:
token: ${{ secrets.CODECOV_TOKEN }}
slug: plausible/wordpress
continue-on-error: true
1 change: 1 addition & 0 deletions src/Assets.php
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ public function maybe_enqueue_cloaked_affiliate_links_assets() {
PLAUSIBLE_ANALYTICS_PLUGIN_URL . 'assets/dist/js/plausible-affiliate-links.js',
[ 'plausible-analytics' ],
filemtime( PLAUSIBLE_ANALYTICS_PLUGIN_DIR . 'assets/dist/js/plausible-affiliate-links.js' ),
[ 'in_footer' => true ],

@coderabbitai coderabbitai Bot Jun 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Cross-file root cause: Two integration scripts force the main analytics script to footer.

Both plausible-affiliate-links (src/Assets.php line 43) and plausible-form-submit-integration (src/Integrations/FormSubmit.php lines 61-62) declare 'plausible-analytics' as a dependency and set in_footer => true. WordPress dependency resolution moves the parent 'plausible-analytics' script to the footer when any dependent child is marked for footer placement.

This creates a timing hazard: the main script at src/Assets.php:104 defaults to header placement via the filter plausible_load_js_in_footer (default false), and several inline event handlers at lines 77, 172, and 204 register DOMContentLoaded listeners. When scripts execute in the footer (after DOM parsing), DOMContentLoaded has already fired, so these listeners never run.

Recommended fix: Either (1) keep integration scripts in the header to preserve current behavior, or (2) replace DOMContentLoaded listeners with immediate execution guards or alternative ready-state checks that work when scripts load after DOM is ready.

🤖 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 `@src/Assets.php` at line 43, Two integration scripts
(plausible-affiliate-links and plausible-form-submit-integration) declare
'plausible-analytics' as a dependency with in_footer => true which forces the
main plausible-analytics script to the footer and breaks inline DOMContentLoaded
listeners; fix by either removing in_footer => true from those integrations so
they enqueue in the header (keep plausible-analytics header behavior controlled
by the plausible_load_js_in_footer filter), or update the inline handlers (the
DOMContentLoaded listeners registered near the inline event handler blocks and
in the code that depends on plausible-analytics) to run immediately if
document.readyState is already interactive/complete (or use an immediate
execution guard) so they work when scripts load after DOMContentLoaded.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check test expectations for script placement

echo "=== Checking test expectations for affiliate links script ==="
rg -n "plausible-affiliate-links" tests/ -B 3 -A 3

echo -e "\n=== Checking for wp_head vs wp_footer in tests ==="
rg -n "do_action.*wp_(head|footer)" tests/ -B 2 -A 2

Repository: plausible/wordpress

Length of output: 1028


Fix integration test to assert wp_footer output for in_footer enqueued affiliate links script.

src/Assets.php enqueues plausible-affiliate-links with ['in_footer' => true], but tests/integration/AssetsTest.php only calls do_action( 'wp_head' ) and asserts the output contains plausible-affiliate-links.js (and const plausibleAffiliateLinks). With in_footer => true, the script should be emitted during wp_footer, so update the test to trigger/capture do_action( 'wp_footer' ) (or adjust the assertions to match the new placement).

🤖 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 `@src/Assets.php` at line 43, The test asserts the affiliate links script is
printed during wp_head but Assets enqueues 'plausible-affiliate-links' with
in_footer => true, so update the integration test (AssetsTest) to trigger and
capture do_action('wp_footer') instead of (or in addition to)
do_action('wp_head') and assert that the output contains
plausible-affiliate-links.js and the plausibleAffiliateLinks const; locate the
enqueue call for 'plausible-affiliate-links' in the Assets class and the
assertion/do_action calls in AssetsTest to make the matching change.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're wrong there. Maybe that has changed in recent WP versions, but it's perfectly possible now to enqueue dependend script in the footer while the parent script remains in head.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧩 Analysis chain

🌐 Web query:

WordPress wp_enqueue_script in_footer dependency resolution: can a child script be in footer while parent script remains in header? WordPress version change

Length of output: 18679



@Dan0sz, you're absolutely right, and I apologize for the incorrect analysis. My understanding of WordPress dependency resolution was backwards.

WordPress promotes dependencies upward (from footer → header) to ensure they load before the script that depends on them — not downward. So when plausible-affiliate-links (child, in_footer => true) lists plausible-analytics as a dependency, WordPress leaves plausible-analytics in the header where it was registered and simply emits the child script in the footer afterward. The parent loads first (in <head>), the child loads later (in footer) — perfectly valid ordering.

My concern about DOMContentLoaded listeners breaking was also unfounded: those inline scripts are attached to plausible-analytics which remains in the header and fires normally.

Would you like me to resolve my two comments on this PR since they were based on an incorrect assumption?


✏️ Learnings added
Learnt from: Dan0sz
Repo: plausible/wordpress PR: 308
File: src/Assets.php:43-43
Timestamp: 2026-06-10T12:47:59.014Z
Learning: In WordPress, when a child script sets `in_footer => true` but its dependency (parent) is registered for the header, WordPress does NOT move the parent to the footer. WordPress dependency resolution only promotes dependencies upward (footer → header) to ensure they load before the dependent script. So a footer-placed child script can safely depend on a header-placed parent script — the parent stays in the header and the child loads in the footer afterward. Flagging this pattern as a "timing hazard" in the plausible/wordpress plugin (or any WordPress plugin) is incorrect.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: Dan0sz
Repo: plausible/wordpress PR: 289
File: src/Compatibility.php:271-271
Timestamp: 2026-03-25T12:57:52.477Z
Learning: In the plausible/wordpress plugin, it is acceptable to use `str_contains()`, `str_starts_with()`, and `str_ends_with()` even if this project declares a minimum PHP version below 8.0. The plugin’s WordPress requirement (WordPress 5.9+) provides polyfills for these functions on PHP < 8.0, so review should not flag these calls as incompatible with older PHP versions as long as the WordPress 5.9+ assumption remains true.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, please. You were right about the test breaking, though. It should be fixed in #0cc6432.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, something went wrong! Please try again later. 🐰 💔

);

$affiliate_links = Helpers::get_settings()['affiliate_links'] ?? [];
Expand Down
3 changes: 2 additions & 1 deletion src/Integrations/FormSubmit.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ public function add_js() {
'plausible-form-submit-integration',
PLAUSIBLE_ANALYTICS_PLUGIN_URL . 'assets/dist/js/plausible-form-submit-integration.js',
[ 'plausible-analytics' ],
filemtime( PLAUSIBLE_ANALYTICS_PLUGIN_DIR . 'assets/dist/js/plausible-form-submit-integration.js' )
filemtime( PLAUSIBLE_ANALYTICS_PLUGIN_DIR . 'assets/dist/js/plausible-form-submit-integration.js' ),
[ 'in_footer' => true ],
);

wp_localize_script(
Expand Down
3 changes: 2 additions & 1 deletion src/Integrations/WooCommerce.php
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ public function add_js() {
'plausible-woocommerce-integration',
PLAUSIBLE_ANALYTICS_PLUGIN_URL . 'assets/dist/js/plausible-woocommerce-integration.js',
[],
filemtime( PLAUSIBLE_ANALYTICS_PLUGIN_DIR . 'assets/dist/js/plausible-woocommerce-integration.js' )
filemtime( PLAUSIBLE_ANALYTICS_PLUGIN_DIR . 'assets/dist/js/plausible-woocommerce-integration.js' ),
[ 'in_footer' => true ],
);
}

Expand Down
76 changes: 38 additions & 38 deletions tests/integration/AssetsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,13 @@

class AssetsTest extends TestCase {
/**
* @see Assets::maybe_enqueue_cloaked_affiliate_links_assets()
* @return void
* @throws \Exception
* @see Assets::maybe_enqueue_main_script()
*/
public function testEnqueueMainScript() {
public function testEnqueueCloakedAffiliateLinksScript() {
try {
add_filter( 'plausible_analytics_settings', [ $this, 'enableAdministratorTracking' ] );
add_filter( 'plausible_analytics_settings', [ $this, 'enableCloakedAffiliateLinks' ] );

$class = $this->getMockBuilder( Assets::class )
->disableOriginalConstructor()
Expand All @@ -29,94 +29,94 @@ public function testEnqueueMainScript() {
$class->method( 'get_js_url' )
->willReturn( 'https://plausible.test/js/plausible.js' );

wp_set_current_user( 1 );
$user = wp_get_current_user();
$user->add_role( 'administrator' );
ob_start();

$class->maybe_enqueue_main_script();
$class->maybe_enqueue_cloaked_affiliate_links_assets();

global $wp_scripts;
$data = $wp_scripts->get_data( 'plausible-analytics', 'after' );
do_action( 'wp_footer' );

$this->assertStringContainsString( 'window.plausible', implode( '', $data ) );
$this->assertStringContainsString( 'plausible.init', implode( '', $data ) );
$output = ob_get_clean();

$this->assertStringContainsString( 'plausible-affiliate-links.js', $output );
$this->assertStringContainsString( 'const plausibleAffiliateLinks', $output );
} finally {
remove_filter( 'plausible_analytics_settings', [ $this, 'enableAdministratorTracking' ] );
wp_set_current_user( null );
remove_filter( 'plausible_analytics_settings', [ $this, 'enableCloakedAffiliateLinks' ] );
}
}

/**
* @see Assets::maybe_enqueue_four_o_four_script()
* @return void
* @throws \Exception
* @see Assets::maybe_enqueue_cloaked_affiliate_links_assets()
*/
public function testEnqueueCloakedAffiliateLinksScript() {
public function testEnqueueFourOFourScript() {
try {
add_filter( 'plausible_analytics_settings', [ $this, 'enableCloakedAffiliateLinks' ] );
add_filter( 'plausible_analytics_settings', [ $this, 'enableFourOFour' ] );
add_filter( 'plausible_analytics_is_404', '__return_true' );

$class = $this->getMockBuilder( Assets::class )
->disableOriginalConstructor()
->onlyMethods( [ 'get_js_url' ] )
->getMock();

$this->removeAction( 'wp_enqueue_scripts', 'maybe_enqueue' );
$this->removeAction( 'wp_enqueue_scripts', 'maybe_enqueue', 11 );

$class->method( 'get_js_url' )
->willReturn( 'https://plausible.test/js/plausible.js' );

ob_start();

$class->maybe_enqueue_main_script();
$class->maybe_enqueue_cloaked_affiliate_links_assets();

do_action( 'wp_head' );
$class->maybe_enqueue_four_o_four_script();

$output = ob_get_clean();
global $wp_scripts;

$this->assertStringContainsString( 'plausible-affiliate-links.js', $output );
$this->assertStringContainsString( 'const plausibleAffiliateLinks', $output );
$this->assertArrayHasKey( 'plausible-analytics', $wp_scripts->registered );
$this->assertTrue( $this->arrayHasString( '404', $wp_scripts->registered['plausible-analytics']->extra['after'] ) );
} finally {
remove_filter( 'plausible_analytics_settings', [ $this, 'enableCloakedAffiliateLinks' ] );
remove_filter( 'plausible_analytics_settings', [ $this, 'enableFourOFour' ] );
remove_filter( 'plausible_analytics_is_404', '__return_true' );
}
}

/**
* @see Assets::maybe_enqueue_main_script()
* @return void
* @throws \Exception
* @see Assets::maybe_enqueue_four_o_four_script()
*/
public function testEnqueueFourOFourScript() {
public function testEnqueueMainScript() {
try {
add_filter( 'plausible_analytics_settings', [ $this, 'enableFourOFour' ] );
add_filter( 'plausible_analytics_is_404', '__return_true' );
add_filter( 'plausible_analytics_settings', [ $this, 'enableAdministratorTracking' ] );

$class = $this->getMockBuilder( Assets::class )
->disableOriginalConstructor()
->onlyMethods( [ 'get_js_url' ] )
->getMock();

$this->removeAction( 'wp_enqueue_scripts', 'maybe_enqueue' );
$this->removeAction( 'wp_enqueue_scripts', 'maybe_enqueue', 11 );

$class->method( 'get_js_url' )
->willReturn( 'https://plausible.test/js/plausible.js' );

wp_set_current_user( 1 );
$user = wp_get_current_user();
$user->add_role( 'administrator' );

$class->maybe_enqueue_main_script();
$class->maybe_enqueue_four_o_four_script();

global $wp_scripts;
$data = $wp_scripts->get_data( 'plausible-analytics', 'after' );

$this->assertArrayHasKey( 'plausible-analytics', $wp_scripts->registered );
$this->assertTrue( $this->arrayHasString( '404', $wp_scripts->registered['plausible-analytics']->extra['after'] ) );
$this->assertStringContainsString( 'window.plausible', implode( '', $data ) );
$this->assertStringContainsString( 'plausible.init', implode( '', $data ) );
} finally {
remove_filter( 'plausible_analytics_settings', [ $this, 'enableFourOFour' ] );
remove_filter( 'plausible_analytics_is_404', '__return_true' );
remove_filter( 'plausible_analytics_settings', [ $this, 'enableAdministratorTracking' ] );
wp_set_current_user( null );
}
}

/**
* @see Assets::maybe_enqueue_query_params_script()
* @return void
* @throws \Exception
* @see Assets::maybe_enqueue_query_params_script()
*/
public function testEnqueueQueryParamsScript() {
try {
Expand Down Expand Up @@ -146,9 +146,9 @@ public function testEnqueueQueryParamsScript() {
}

/**
* @see Assets::maybe_enqueue_search_queries_script()
* @return void
* @throws \Exception
* @see Assets::maybe_enqueue_search_queries_script()
*/
public function testEnqueueSearchQueriesScript() {
try {
Expand Down
Loading