URLs in CSS files aren't rewritten as soon as the CDN is enabled

[vmanthos]

When the CDN feature is enabled, we rewrite URLs of static assets that are in the source code of the page and URLs in CSS files, e.g. of background images.

The first time one enables the CDN, the minified/combined files aren't cleared. So, the URLs in there are not rewritten and the images aren't downloaded from the CDN, but from the origin server.

If the cache is cleared, using the admin menu button, the URLs in the newly created files are properly rewritten.

To Reproduce

Steps to reproduce the behavior:

1. Add a background-image:url(https://eaxmple.com/image.pnp) in the theme's style.css file.
2. Enable WP Rocket's CSS minification feature.
3. Note down the filename of the CSS file.
4. Enable WP Rocket's CDN feature.
5. The filename from step 3 will still be the same and the image's URL will not have been rewritten to the CDN.
6. Clear WP Rocket's path and check the CSS filename. It will have changed, as expected, and the URL of the image will point to the CDN.

Expected behavior
Rewrite the CSS files URLs, as soon as the option is enabled.

Possible Solution
Clear the minified files when the CDN option is enabled/disabled.

Additional context
An edge case would be if someone disables the CDN and then cancel their subscription.
The CSS files URLs' will still point to the CDN and this will cause errors 404.

Clearing the cache will fix this, but we can avoid this in the first place.

Related ticket
https://secure.helpscout.net/conversation/1101999708/149719/

Slack Thread
https://wp-media.slack.com/archives/C43T1AYMQ/p1583602741148200?thread_ts=1583602533.148100

Backlog Grooming

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

[crystinutzaa]

Reproduce the issue ✅
Reproduced the issue on local

Identify the root cause ✅
These are the lines :

wp-rocket/inc/admin/options.php

Lines 78 to 87 in fbb9380

	// Purge all minify cache files.
	// phpcs:ignore WordPress.Security.NonceVerification.Missing
	if ( ! empty( $_POST ) && ( $oldvalue['minify_css'] !== $value['minify_css'] || $oldvalue['exclude_css'] !== $value['exclude_css'] ) || ( isset( $oldvalue['cdn'] ) && ! isset( $value['cdn'] ) || ! isset( $oldvalue['cdn'] ) && isset( $value['cdn'] ) ) ) {
	rocket_clean_minify( 'css' );
	}
	// phpcs:ignore WordPress.Security.NonceVerification.Missing
	if ( ! empty( $_POST ) && ( $oldvalue['minify_js'] !== $value['minify_js'] || $oldvalue['exclude_js'] !== $value['exclude_js'] ) || ( isset( $oldvalue['cdn'] ) && ! isset( $value['cdn'] ) || ! isset( $oldvalue['cdn'] ) && isset( $value['cdn'] ) ) ) {
	rocket_clean_minify( 'js' );
	}

Mainly this condition is the issue:
( isset( $oldvalue['cdn'] ) && ! isset( $value['cdn'] ) || ! isset( $oldvalue['cdn'] ) && isset( $value['cdn'] )

It seems this condition affects also the JS cache clean, so I assume both should be changed: JS & CSS.
Also I was wondering if the cache should be cleaned also when CNAME is changed?

Scope a solution ✅
The solution will be fix that condition by switching to $oldvalue['cdn'] !== $value['cdn'].

Estimate the effort ✅

Effort: [S]

The code change itself is very low effort, however it needs both Unit tests & Integration tests for rocket_after_save_options function.

@hellofromtonya I would need to check what would happen at CNAME change, should I clean the CSS / JS files?

[Tabrisrp]

It seems this condition affects also the JS cache clean, so I assume both should be changed: JS & CSS.
We don't rewrite inside JS files, so it's not necessary.

Also I was wondering if the cache should be cleaned also when CNAME is changed?
Yes indeed, it's a good catch, CSS minified files should also be cleaned in that case.

About the solution
I'm thinking it would be a good opportunity to continue removing code from this function, and move it to the CSS minify subscriber as a separate method. It will be easier to test and improve the code quality.

[crystinutzaa]

So, with JS, then this condition can be removed from line 85 related to rocket_clean_minify( 'js' ); ?

wp-rocket/inc/admin/options.php

Line 85 in fbb9380

if ( ! empty( $_POST ) && ( $oldvalue['minify_js'] !== $value['minify_js'] || $oldvalue['exclude_js'] !== $value['exclude_js'] ) || ( isset( $oldvalue['cdn'] ) && ! isset( $value['cdn'] ) || ! isset( $oldvalue['cdn'] ) && isset( $value['cdn'] ) ) ) {

It can be moved in CSS Minify subscriber in a new function which is hooked into:
'update_option_' . WP_ROCKET_SLUG, right?

[Tabrisrp]

the CSS part, not the JS part which is unaffected by the issue at hand. Le'ts move the minified CSS files cleaning to the minify CSS subscriber, and handle the cleaning there, with a new method callback on 'update_option_' . WP_ROCKET_SLUG yes.

What do you think?

[crystinutzaa]

This PR #2392 needs to be approved and merged first into develop. PR2392 moves the code in the Engine new architecture and this code should rely in there.
So this one is blocked until PR 2392 is approved and merged.
@hellofromtonya I think that only you need to revisit PR2392 and approve it.

[hellofromtonya]

@crystinutzaa PR #2392 is now approved and in QA.