Support brotli compression of assets

Re-rolled the patch from @prudloff because this patch was from the substree-split (drupal/core).

And the same patch without patches to /.htaccess - the patch can not be applied with composer otherwise.

Updated patch will fix test failures.

Adding #1040534: Rewrite rules for gzipped CSS and JavaScript aggregates cause lots of lstats for files that will never exist as a related issue.

One issue here is that if brotli compression isn't available on the server, this will add even more lstats for files that will never exist. Trying to think through ways around this and how to reconcile it with the latest approach on that issue.

Does this just work with apache2?
Does it work with nginx?

#1040534: Rewrite rules for gzipped CSS and JavaScript aggregates cause lots of lstats for files that will never exist improves the regexp so that it doesn't look for files like .gz.gz, but that won't help on servers that don't have brotli compression enabled - where we'd be looking for a .br file that will never exist.

However one of the possible solutions in #1040534: Rewrite rules for gzipped CSS and JavaScript aggregates cause lots of lstats for files that will never exist was to write a .htaccess file to the aggregate folder with the rewrite rules, so that it only applies to those folders.

If we were to do that, we could also potentially write different .htaccess files dependent on whether brotli compression is available or not, then sites without it won't check file existence for .br files.

How would that work if you are using nginx server as they do not use .htaccess?

Added reroll of patch #12 on Drupal 10.1.x.

How would that work if you are using nginx server as they do not use .htaccess?

For nginx you would need to install the brotli module (see https://github.com/google/ngx_brotli), then add a `brotli_static on` directive to the correct block of your configuration.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Added rerolls of patch #20 and the non-.htaccess version of #22 on Drupal 10.1.x.

I also added some changes to the added .htaccess rules to make the rewriterule conditions consistent with the ones for gzip.

So taking a look at this

Could we add it's own test case? Seems some tests had to be updated for it but not sure if it's actually testing.

Since the schema is updating most likely will need an upgrade path + upgrade path tests.

If this new setting is required (which seems to be) it will require a change record also

Thanks!

What if new compression algo will be added to browsers?

Better to deprecate "gzip" option in favor of "compress" which will be common trigger to produce all possible assets archives.
Also it could be a sequence of algo to limit :

compress:
- gzip
- brotli

+++ b/core/lib/Drupal/Core/Asset/AssetDumper.php
@@ -77,6 +77,16 @@ public function dumpToUri(string $data, string $file_extension, string $uri): st
+    if (extension_loaded('brotli') && \Drupal::config('system.performance')->get($file_extension . '.brotli')) {

+++ b/core/modules/system/config/install/system.performance.yml
@@ -4,6 +4,7 @@ cache:
 css:
   preprocess: true
   gzip: true
+  brotli: false

@@ -12,4 +13,5 @@ fast_404:
 js:
   preprocess: true
   gzip: true
+  brotli: false

Not sure it makes sense to introduce new configuration and without UI, it could re-use "gzip" setting to produce compressed assets using gzip/brotli

A compress item with a sequence sounds good.

Having said that we've got the problem that the .htaccess rules can't be configured - they will always need to check in order of likelihood-of-support first. So if we add brotli support to htaccess but it's disabled by the site, the rewrite rules will still look for those files.

Makes me wonder if we should just do 'compress' as a boolean and write out everything we possibly can.

Added rerolls of patches in #24 on Drupal 10.2.x. I think they should also apply on 11.x-dev.

Added rerolls of patches in #29 on Drupal 10.2.x.

Reversed the naming in #30; added
https://www.drupal.org/files/issues/2024-01-05/3184242-asset-brotli-comp...
and
https://www.drupal.org/files/issues/2024-01-05/3184242-asset-brotli-comp...
which are correct.

Is there a variable we can set in settings.php?

Not sure how tests can cover it but CR should mention update of nginx conf as well

with a new D11 v11.1.0 install,

i see in generated assets,

ls -1 web/sites/default/files/css/ | sort | head -n 6
	css_1A5CXfNop7cNp-NPI4DF_OGMQem-KN6lxj_k9OzGOpU.css
	css_1A5CXfNop7cNp-NPI4DF_OGMQem-KN6lxj_k9OzGOpU.css.gz
	css_1ecH70dXxW6SrorXnALqr-8DiJZhZ4g3Y6cBbPZOuEc.css
	css_1ecH70dXxW6SrorXnALqr-8DiJZhZ4g3Y6cBbPZOuEc.css.gz
	css_1TuJBQ_3nvA_oZ-1Bd-SoJw7dHab08LO29K2pikMvXw.css
	css_1TuJBQ_3nvA_oZ-1Bd-SoJw7dHab08LO29K2pikMvXw.css.gz
	...

, no brotli (.br)

checking,

cat ./web/core/modules/system/config/install/system.performance.yml (yaml)                                                                                                                   cache:
	page:
		max_age: 0
	css:
	preprocess: true
	gzip: true
	fast_404:
	enabled: true
	paths: '/\.(?:txt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp)$/i'
	exclude_paths: '/\/(?:styles|imagecache)\//'
	html: '<!DOCTYPE html><html><head><title>404 Not Found</title></head><body><h1>Not Found</h1><p>The requested URL "@path" was not found on this server.</p></body></html>'
	js:
	preprocess: true
	gzip: true

status here says "Needs work", and "Remaining tasks: I will submit a patch."

the MR, #9980 submitted a month ago, is "Ready to merge" and "1 commit will be added to 11.x".
but has not been merged.

@prudloff

what's outstanding?
what release can this be expected in?

Hope this will be added soon...

It would be great if automatic creation of static gzip and brotli files is also added for all svg files people have in their themes and uploaded files directories. I am now creating those manually on my server.

as both kinds of compression are common I think better to add them to images, just file issue to https://www.drupal.org/project/issues/drupalci_environments

Added both brotli and zstd to dev images, rebased and looks like tests passed

PS: probably zstd could be added in related

Sure, let's focus the issue on title and there's so many "needs" tags so not sure it fits into 11.2

CI images now has brotli 0.17.0 with latest fixes https://github.com/kjdev/php-ext-brotli/commit/48c719a9f0e2b1ae53e08acf2...

tests are here

Created CR using Claude Opus 4

Fixed https://www.drupal.org/node/3526344/revisions/view/14005103/14005483

It talks about a setting on /admin/config/development/performance but I see no such setting.

this is where aggregate js/css checkboxes are

not the same

yes, this settings enabling aggregation, now having aggregated assets core applies compression

What about svg and webmanifest assets? Or are non-aggregated assets beyond the scope of this issue report?

Ok, maybe a suggestion for the nginx example config in the CR:

Isn't it better and cleaner to combine both as follows or can this induce problems?

location ~ ^/sites/.*/files/(css|js)/(.*)\.(css|js)$ {
    gzip_static on;
    brotli_static on;
    try_files $uri.br $uri.gz $uri =404;
}

Not sure if this should be in the example configuration, but it also can be with better caching parameters:

location ~ ^/sites/.*/files/(css|js)/(.*)\.(css|js)$ {
    gzip_static on;
    brotli_static on;
    expires max;
    add_header Cache-Control "public, max-age=31536000, immutable";
    try_files $uri.br $uri.gz $uri =404;
}

Shouldn't these keys be deprecated?

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

I left some comments on the MR.

The issue summary needs to be updated. The full summary template should be applied. The proposed resolution section needs to be updated with information about the changes to the system.performance config schema. And the UI Changes section still mentions modifying the Performance form, which is no longer happening.

My feedback was addressed.

One question on the MR.

Setting to Needs Work to fix the line flagged by @catch.

Deprecation version should be bumped before being committed. I also removed the line in the CR about migrating settings from "admin/config/development/performance", since the form does not control compression settings, and the only way to change them is programmatically or through CLI.

Needs Work for another rebase and for the latest comments made by @godotislate.

The Needs Review Queue Bot tested this issue. The merge request has merge conflicts and cannot be merged. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

Recent feedback from @godotislate was addressed. I'm going to go ahead and RTBC this, but I realized upon re-review that the removal version is still set as D12. That may need to be updated to D13. I'm not 100% sure because it's been a while since we had those discussions.

Went back and forth on the config schema deprecation a bit (with myself...).

Assuming we're able to land this in both 12.x and 11.4, then the update will be in both versions, so whether you go 11.3 -> 11.4 -> 12.0 or 11.3 -> 12.0 the update will run and your config will be correct.

This isn't a config entity so we don't need to worry about shipped configuration with modules, the only case where it could be shipped would be in a recipe (unlikely) or distribution (more likely if they ship essentially the entire contents of the config/sync directory).

I'm not entirely sure about the possibly disruption to distributions after typing that out, it might be easier to defer the deprecation for removal in 13.0 which would be the same version that the update path is removed in though.

We need a release note here because this changes site-owner managed files.

Also tagging for 11.4.0 release highlights since it'll be another thing to put in the performance section.

added snippet and IS a bit

added release note snippet

The added release note is a good one-sentence summary of the change.

Thought about the deprecation more and the use case for customising this in a distribution is so narrow I think we're fine to deprecate for removal in 12.x

Committed/pushed to main and 11.x, thanks! Manually resolved a straightforward merge conflict in 11.x for the post update, just context.

Think this has broken HEAD. Opened https://git.drupalcode.org/project/drupal/-/merge_requests/15602 to address.

Oh never mind, @alexpott already got it.

This also broke HEAD in 11.x.

Drupal\Tests\system\Kernel\Migrate\d7\MigrateSystemConfigurationTest::testConfigurationMigration
system.performance matches expected values.
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
     ],
     'css' => Array &3 [
         'preprocess' => true,
-        'gzip' => true,
+        'compress' => true,
     ],
     'fast_404' => Array &4 [
         'enabled' => true,
@@ @@
     ],
     'js' => Array &5 [
         'preprocess' => false,
-        'gzip' => true,
+        'compress' => true,
     ],
 ]

Hotfixed in 468b307.