wp-media / wp-rocket

Performance optimization plugin for WordPress
https://wp-rocket.me
GNU General Public License v2.0
701 stars 219 forks source link

Delay Java Script Execution detects wrong '</Body>' tag #7033

Open osgamedeveloper opened 3 weeks ago

osgamedeveloper commented 3 weeks ago

Describe the bug In a script bilmur from WP I have the follow code:

var body_end = data.lastIndexOf('</body>');

unfortunately when I enable WP Rocket - Delay Java Script Execution function follow things happen: var body_end = data.lastIndexOf('<script>class RocketElementorAnimation ......RocketElementorAnimation.run);</script></body>');

Seems like WP Rocket thinks that '</body>' is the "real" and inserts some code into it.

Due this problem I cannot use Delay Java Script Execution function on my website. Is this a bug what you could fix and support or I can somehow adjust my settings to get around this?

To Reproduce Steps to reproduce the behavior:

  1. Go to WP Rocket settings
  2. Click on Delay Java Script Execution
  3. Clear WP Rocket cache
  4. See an error in a '' text

Expected behavior The real body modified, not a text one

Screenshots bug: Image expected result: Image

Desktop (please complete the following information):

joejoe04 commented 3 weeks ago

This line in their script: var body_end = data.lastIndexOf('</body>');

Causes the pattern here to match, and our script gets inserted there if DJE is enabled: https://imageshack.com/a/img924/8567/qQguqI.png

A possible solution: If we replaced this code: https://github.com/wp-media/wp-rocket/blob/8b46e6b8692f3b80931d352eaee78481fa5467e6/inc/ThirdParty/Plugins/PageBuilder/Elementor.php#L120-L126

With something like this:

$fix_elementor_animation_script = $this->filesystem->get_contents( rocket_get_constant( 'WP_ROCKET_PATH' ) . 'assets/js/elementor-animation.js' );

$sub_position = strripos( $html, '</body', 0 );

$html = substr_replace( $html, "<script>{$fix_elementor_animation_script}</script>", $sub_position, 0 );

The script is no longer inserted in the wrong place: https://imageshack.com/a/img922/1959/i4qC4X.png

And is instead inserted where it should be: https://imageshack.com/a/img922/6672/3mtTyS.png

My thinking is that we instead search for the last occurrence of </body>, which should almost always be the correct one. And if an instance of </body> does come after the real closing tag, it's a much easier fix (move it to before the real closing body tag).

Ticket: https://secure.helpscout.net/conversation/2734968002/517956 Slack: https://wp-media.slack.com/archives/C43T1AYMQ/p1729088561266079

joejoe04 commented 3 weeks ago

@wordpressfan provided a better solution in our slack thread, which is to change the regex as follows:

$pattern = '/(<\/body>(?!.*<\/body>))/is';

The idea is the same - find and use the last occurrence of </body>

Mai-Saad commented 1 week ago

The same applies to the beacon script: instead of var body_end = data.lastIndexOf('</body>'); , it will be var body_end = data.lastIndexOf('<script>var rocket_beacon_data = {"ajax_url":"https:\/\/new.rocketlabsqa.ovh\/wp-admin\/admin-ajax.php","nonce":"a2f3e29e45","url":"https:\/\/new.rocketlabsqa.ovh\/template_scriptwithbodyclosure","is_mobile":false,"width_threshold":1600,"height_threshold":700,"delay":500,"debug":true,"status":{"atf":true,"lrc":true},"elements":"img, video, picture, p, main, div, li, svg, section, header, span","lrc_threshold":1800}</script><script data-name="wpr-wpr-beacon" src='https://new.rocketlabsqa.ovh/wp-content/plugins/wp-rocket/assets/js/wpr-beacon.min.js' async></script></body>');