Wp-rocket: CSS files with media queries are not excluded during CSS combination

[vmanthos]

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

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

Describe the bug

Currently, when combining CSS files we check for media queries here:
https://github.com/wp-media/wp-rocket/blob/443f2e1b902c592bd517cbaa641571bb2ddce1a5/inc/Engine/Optimization/Minify/CSS/AbstractCSSOptimization.php#L124

We do not exclude link elements containing media queries like:

• max-width
• min-width

For example, the following file will be combined:

<link rel='stylesheet' media='screen and (max-width: 900px)' href='css/medium.css' />

When that happens, we do not wrap the file's content around the media query, which would be the ideal solution.
Instead, we add it directly to the combined file.

The rules contained there may be applied to elements regardless of what the original media query dictated.

To Reproduce
Steps to reproduce the behavior:

1. Add a CSS file using a media query, e.g. max-width. In that, change, e.g. the color of h1.
2. Enable CSS combination.
3. Visit the site. The rule from step 1 will be applied regardless of the width of the viewport.

Expected behavior

Files containing such media queries should be either automatically excluded or combined while wrapped in the original media query.

Additional context

Related ticket: https://secure.helpscout.net/conversation/1305858288/201378?folderId=2135277

Backlog Grooming (for WP Media dev team use only)

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

[arunbasillal]

Thanks for the details Vasilis. I think the easiest solution would be to exclude such files from CSS combination, or maybe we could do the ideal solution as suggested.

Since we will be seeing CSS related changes in 3.8, going to keep this as pending until then. We can change this and fix this if we get too many tickets related to this. If not, let's see how 3.8 turns out to decide on the future of this feature.

[vmanthos]

@arunbasillal Just a quick note.

We can change this and fix this if we get too many tickets related to this.

It could be that a good percentage of the tickets related to the Combine CSS files feature is because of the specific issue.
Something worth taking into consideration. 🙏

[crystinutzaa]

@arunbasillal & @vmanthos 3.8 with CSS Tree Shaker will be able to handle this situation just fine.

Scope a solution ✅

The solution in here is the one proposed by @vmanthos:

• identify the Media link attribute and if is different from all to simply enclose the content of the CSS in the proper media tag.

With a regex we can identify the media attribute and then will be a simple change similar to this one:
sprintf( '@media %1$s { %2$s }', $media, $css_content );

Estimate the effort ✅

[S]