Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The mobile version of the homepage isn't preloaded in priority when separate cache for mobile devices is enabled #6040

Closed
4 tasks
vmanthos opened this issue Jul 12, 2023 · 6 comments · Fixed by #6067
Assignees
Labels
effort: [S] 1-2 days of estimated development time module: cache priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@vmanthos
Copy link
Contributor

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

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

Describe the bug

On certain occasions, e.g. when activating WP Rocket, or when clearing the used CSS, we are preloading the homepage in priority:

wp_remote_get(
home_url(),
[
'timeout' => 0.01,
'blocking' => false,
'user-agent' => 'WP Rocket/Homepage Preload',
'sslverify' => apply_filters( 'https_local_ssl_verify', false ), // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound
]
);

If the "Separate cached files for mobile devices" is enabled we aren't preloading that version. Eventually, the mobile-specific file will be preloaded but that can take a while.

To Reproduce

Steps to reproduce the behavior:

  1. Enable "Separate cached files for mobile devices" and Optimize CSS Delivery.
  2. Clear the used CSS.
  3. Look into the /cache/wp-rocket/ folder.
  4. The desktop version of the homepage is created immediately while for the mobile version, it takes a while.

Expected behavior

Preload the mobile-specific cached file of the homepage whenever we do that for the desktop version.

Screenshots

N/A

Additional context

The user-agent we use is WP Rocket/Homepage Preload. We need to add a request using a mobile user-agent wherever that one is used.

Backlog Grooming (for WP Media dev team use only)

  • Reproduce the problem
  • Identify the root cause
  • Scope a solution
  • Estimate the effort
@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement module: cache priority: medium Issues which are important, but no one will go out of business. needs: grooming needs: product direction and removed needs: grooming needs: product direction labels Jul 14, 2023
@piotrbak
Copy link
Contributor

piotrbak commented Jul 14, 2023

Acceptance Criteria:

1. When separate cache for mobiles is enabled when activating the plugin:

2. When separate cache for mobiles is enabled when clearing the Used CSS:

3. When separate cache for mobiles is disabled when activating the plugin:

  • We should send only desktop request

4. When separate cache for mobiles is disabled when clearing the Used CSS:

  • We should send only desktop request after clearing the cache

@Tabrisrp
Copy link
Contributor

Tabrisrp commented Jul 17, 2023

@vmanthos When clearing the used CSS, did you do it from the admin side or from the front side via the admin bar?

The homepage preload is hooked on the used CSS clear, but only on the admin side.

@vmanthos
Copy link
Contributor Author

@Tabrisrp I cleared it using the admin bar menu item while in WP Rocket's dashboard.
I believe the same would happen if the theme is switched or the permalinks are changed.

@Tabrisrp
Copy link
Contributor

Reproduce the issue ✅

Reproduced

Identify the root cause ✅

There is 2 different parts in this issue, the activation one and the RUCSS clean one.

For the activation, we do a request to the homepage when activating, but we do not do the request for mobile.

For the RUCSS clean one, I noticed that we do have a method hooked on the related action, but right after in the truncate_used_css_handler() method, there is a call to rocket_clean_domain(), so the preload happens and is deleted right away.

There is only a cache file for the desktop version because there is a manual call to the homepage in the method after.

Scope a solution ✅

in WP_Rocket\Engine\Activation\Activation.php

  • remove the call to the homepage
  • add a new action rocket_after_activation

in WP_Rocket\Engine\Preload\Activation.php

  • Add the PreloadUrl controller as a dependency to the class
  • In the activate method, add a new add_action on rocket_after_activation, to preload the homepage
  • add a method preload_homepage(), which will call $this->preload_url->preload_url( home_url() );

in WP_Rocket\Engine\Optimization\RUCSS\Admin\Subscriber

  • Remove no longer necessary preload calls to the homepage in truncate_used_css_handler(), and truncate_used_css()
  • in truncate_used_css_handler(), switch the calls to $this->delete_used_css_rows(); and rocket_clean_domain(); to make sure the clean happens before the preload.

Estimate the effort ✅

Effort [S]

@Tabrisrp Tabrisrp added effort: [S] 1-2 days of estimated development time and removed needs: grooming labels Jul 19, 2023
@MathieuLamiot
Copy link
Contributor

MathieuLamiot commented Jul 21, 2023

@vmanthos, @Mai-Saad, @piotrbak: following Tabrisrp's investigation, he pointed out 2 issues. Would it be interesting to rework the acceptance criteria here? I might not understand everything, but it seems the AC cover the activation part (checking we send the right requests on activation), but not the RUCSS side-effect.
A few e2e scenarios might be needed to cover Tabrisrp's proposal?

@piotrbak
Copy link
Contributor

@MathieuLamiot @vmanthos Updated the AC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [S] 1-2 days of estimated development time module: cache priority: medium Issues which are important, but no one will go out of business. type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants