wp-rocket icon indicating copy to clipboard operation
wp-rocket copied to clipboard

wpr-beacon.min.js is added even if DONOTROCKETOPTIMIZE is set to true

Open alfonso100 opened this issue 1 year ago • 5 comments

Before submitting an issue please check that you’ve completed the following steps: Yes - Made sure you’re on the latest version Yes - Used the search feature to ensure that the bug hasn’t been reported before

Describe the bug wpr-beacon.min.js is added even if DONOTROCKETOPTIMIZE is set to true. So when you visit a page, the script is added, the detection triggers, and the entry is added into the wpr_above_the_fold table.   OCI will only obey the filter: add_filter( 'rocket_above_the_fold_optimization', '__return_false' );

To Reproduce Steps to reproduce the behavior:

  1. add define('DONOTROCKETOPTIMIZE', true);
  2. Visit a page in incognito
  3. See the beacon script injected
  4. See the entry being added to the database

Expected behavior DONOTROCKETOPTIMIZE should stop all the optimizations including Optimize Critical Images. And maybe Lazy Render Content in 3.17. That's the logic for other optimizations

Screenshots 3K7V2lP

https://i.imgur.com/3K7V2lP.png

Additional context related slack thread: https://wp-media.slack.com/archives/C43T1AYMQ/p1726159393548059

Acceptance Criteria (for WP Media team use only) Clear instructions for developers, to be added before the grooming

alfonso100 avatar Sep 12 '24 20:09 alfonso100

Reproduce the issue:

  1. Add define('DONOTROCKETOPTIMIZE', true); to your wp-config.php.
  2. Visit a page in incognito mode.
  3. Observe that the beacon script is injected.
  4. Check the database to see the entry added to the wpr_above_the_fold table.

Identify the root cause

The issue is that the wpr-beacon.min.js script is being added even when DONOTROCKETOPTIMIZE is set to true. This causes the detection to trigger and an entry to be added to the wpr_above_the_fold table.

Scope a solution

To solve the issue, we need to modify the inject_beacon function in Processor.php to check for the DONOTROCKETOPTIMIZE constant and skip the injection if it is defined.

Code Changes

Modify Processor.php

  1. Check for DONOTROCKETOPTIMIZE constant:
    • Add a condition to skip the beacon injection if DONOTROCKETOPTIMIZE is defined.
private function inject_beacon( $html, $url, $is_mobile ): string {
    if ( defined( 'DONOTROCKETOPTIMIZE' ) && DONOTROCKETOPTIMIZE ) {
        return $html;
    }

    $min = ( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) ? '' : '.min';

    if ( ! $this->filesystem->exists( rocket_get_constant( 'WP_ROCKET_ASSETS_JS_PATH' ) . 'wpr-beacon' . $min . '.js' ) ) {
        return $html;
    }

    $default_width_threshold  = $is_mobile ? 393 : 1600;
    $default_height_threshold = $is_mobile ? 830 : 700;

    $width_threshold = rocket_apply_filter_and_deprecated(
        'rocket_performance_hints_optimization_width_threshold',
        [ $default_width_threshold, $is_mobile, $url ],
        '3.16.4',
        'rocket_lcp_width_threshold'
    );

    $height_threshold = rocket_apply_filter_and_deprecated(
        'rocket_performance_hints_optimization_height_threshold',
        [ $default_height_threshold, $is_mobile, $url ],
        '3.16.4',
        'rocket_lcp_height_threshold'
    );

    if ( ! is_int( $width_threshold ) ) {
        $width_threshold = $default_width_threshold;
    }

    if ( ! is_int( $height_threshold ) ) {
        $height_threshold = $default_height_threshold;
    }

    $default_delay = 500;

    $delay = rocket_apply_filter_and_deprecated(
        'rocket_performance_hints_optimization_delay',
        [ $default_delay, $is_mobile, $url ],
        '3.16.4',
        'rocket_lcp_delay'
    );

    if ( ! is_int( $delay ) ) {
        $delay = $default_delay;
    }

    $data = [
        'url'             => $url,
        'is_mobile'       => $is_mobile,
        'width_threshold' => $width_threshold,
        'height_threshold' => $height_threshold,
        'delay'           => $delay,
        'nonce'           => wp_create_nonce( 'rocket_beacon' ),
        'ajax_url'        => admin_url( 'admin-ajax.php' ),
        'status'          => [
            'atf' => true,
            'lrc' => true,
        ],
    ];

    $data_modified = null;
    foreach ( $this->factories as $factory ) {
        $data_modified = $factory->get_frontend_controller()->modify_beacon_data( $data );
    }

    $inline_script = '<script>var rocket_beacon_data = ' . wp_json_encode( $data_modified ) . '</script>';

    $script_url = rocket_get_constant( 'WP_ROCKET_ASSETS_JS_URL' ) . 'wpr-beacon' . $min . '.js';

    $script_tag = "<script data-name=\"wpr-wpr-beacon\" src='{$script_url}' async></script>"; // phpcs:ignore WordPress.WP.EnqueuedResources.NonEnqueuedScript

    return str_replace( '</body>', $inline_script . $script_tag . '</body>', $html );
}

Development steps:

  • [ ] Modify the inject_beacon function in Processor.php to check for the DONOTROCKETOPTIMIZE constant.
  • [ ] Ensure the beacon script is not injected when DONOTROCKETOPTIMIZE is set to true.

Effort estimation:

XS

Is a refactor needed in that part of the codebase?

No

Miraeld avatar Oct 07 '24 12:10 Miraeld

I want to relate between this one and the PR: https://github.com/wp-media/wp-rocket/pull/6998

Checking something before the final confirmation.

wordpressfan avatar Oct 08 '24 08:10 wordpressfan

One comment: you can use rocket_get_constant() to check for the constant definition and value, and it makes unit testing easier.

remyperona avatar Oct 08 '24 13:10 remyperona

Re-opening following this founding: https://wp-media.slack.com/archives/CUT7FLHF1/p1729070587176049 I'd suggest we:

  • Revert the merge to unlock develop branch
  • Consider this fix
  • Would there be a way to capture this issue in an integration test?
  • Re-create a PR.

MathieuLamiot avatar Oct 17 '24 06:10 MathieuLamiot

Related test plan https://wpmediaqa.testrail.io/index.php?/runs/view/915&group_by=cases:section_id&group_order=asc

Mai-Saad avatar Oct 21 '24 06:10 Mai-Saad

data-wpr-lazyrender keeps breaking my websites. setting rocket_above_the_fold_optimization and DONOTROCKETOPTIMIZE do not work. Hot fix please!?

bobhuf avatar Oct 24 '24 14:10 bobhuf