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

Preconnect is not removed in all cases

Open camilamadronero-zz opened this issue 3 years ago • 6 comments

Before submitting an issue please check that you’ve completed the following steps:

  • Made sure you’re on the latest version - Yes
  • Used the search feature to ensure that the bug hasn’t been reported before - Yes

Describe the bug From 3.8.8, WP Rocket is detecting if there are Google Fonts before adding "preconnect" to a site, here. However, this check doesn't work if you're viewing while logged-in (User Cache off) or bypassing WP Rocket with ?nowprocket and other query strings.

To Reproduce Steps to reproduce the behavior:

  1. Set up pa site without Google Fonts
  2. Install and activated WP Rocket
  3. Check the page's optimized version, it doesn't have the "preconnect"
  4. If you visit the page with ?nowprocket or while logged-in (User Cache off), it does have the "preconnect"

Expected behavior The Google Fonts check should be applied to all scenarios, including bypassed optimizations and while logged-in.

Screenshots The behavior at ?nowprocket for my test site, https://camila.com.co/ image

Additional context Slack thread: https://wp-media.slack.com/archives/C08N8J6VC/p1663690608273009 Ticket: https://secure.helpscout.net/conversation/1999948040/366451?folderId=2675957 This is currently a problem for customer because the external domain preconnect violates the GDPR law.

Thanks!

Backlog Grooming (for WP Media dev team use only)

  • [ ] Reproduce the problem
  • [ ] Identify the root cause
  • [ ] Scope a solution
  • [ ] Estimate the effort

camilamadronero-zz avatar Sep 26 '22 14:09 camilamadronero-zz

Reproduce the problem :heavy_check_mark:

Identify the root cause

first problem

in the is_allowed we are not checking if the user is logged in or not https://github.com/wp-media/wp-rocket/blob/60b76bc8bc9a366bf5b9644894df8203bba103fd/inc/Engine/Optimization/GoogleFonts/Subscriber.php#L118-L124

second problem

~~in rocket_bypass we look for nowprocket in $wp->query_vars which will not include the nowprocket URL parameter because it is added on the fly (in the browser for example) and is not added using [add_query_arg]~~(https://developer.wordpress.org/reference/functions/add_query_arg/)

After discussing with @engahmeds3ed $wp->query_vars in wp version 5.9 was returning null if there is no query_vars so the output of this line https://github.com/wp-media/wp-rocket/blob/60b76bc8bc9a366bf5b9644894df8203bba103fd/inc/API/bypass.php#L27 was something like this /?nowprocket&=http://wordpress.localhost/ which is working

but after version 6.0 it returns just http://wordpress.localhost/ which is not working

Scope a solution

first problem

we need to change is_allowed function to check if user is logged in and cache for logged in users is enabled

	protected function is_allowed() {
		if ( rocket_bypass() ) {
			return false;
		}

		if(! $this->options->get( 'minify_google_fonts', 0 )){
			return false;
		}

		return !is_user_logged_in() || (bool) $this->options->get( 'cache_logged_user', 0) ;
	}

second problem

in rocket_bypass we can not use what we do now and straight check nowprocket URL parameter is existing in the current URL

something like this

isset($_GET['nowprocket']) && $_GET['nowprocket'] !== 0`

this function is used in most of our optimizations so we need to check its effect there too

Estimate the effort

Effort S

mostafa-hisham avatar Feb 13 '23 09:02 mostafa-hisham

Although not entirely related to this issue, using ?nowprocket while the Link Preload feature is enabled, will still load the respective scripts.

The solution @mostafa-hisham proposed should resolve this too.

@wp-media/qa When testing the PR for this feature we should check if Link Preload is disabled as well when using ?nowprocket.

vmanthos avatar Feb 13 '23 14:02 vmanthos

Some users are still experiencing the issue of preconnect not removed in all cases, as reported in this ticket https://secure.helpscout.net/conversation/2854867074/541746?viewId=3572910.

On https://www.xps-qualitaet.de/wp/wp-login.php there is preconnect code for Google fonts https://jmp.sh/Di44nF1R

With ?nowprocket there is no preconnect code https://jmp.sh/wdkZoqqm

But with https://www.xps-qualitaet.de/wp/wp-login.php?randomstring, there is still preconnect code https://jmp.sh/87DLCTa8

The issue was discussed on Slack: https://group-onecom.slack.com/archives/C08EFUGUH5G/p1740491819564719

juricazuanovic avatar Feb 26 '25 17:02 juricazuanovic

Another case where the customer has the preconnect on their login page: https://secure.helpscout.net/conversation/2904556460/552793?viewId=377611

webtrainingwheels avatar Apr 10 '25 20:04 webtrainingwheels

New related case: https://secure.helpscout.net/conversation/2930092014/558808?viewId=273761

johan-las avatar May 06 '25 10:05 johan-las

Reopening this one as discuted here: https://group-onecom.slack.com/archives/C08F4M3BBJL/p1746533442269919

From the most recent tickets, it seems that this issue is only occurring on the wp-login.php page.

johan-las avatar May 06 '25 12:05 johan-las