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.
- Status changed to Needs review
almost 2 years ago 1:33pm 9 May 2023 - last update
almost 2 years ago Custom Commands Failed - last update
almost 2 years ago Custom Commands Failed - 🇳🇱Netherlands JesperN
Added rerolls of patch #20 and the non-`.htaccess` version of #22 on Drupal 10.1.x.
- Status changed to Needs work
almost 2 years ago 2:04pm 9 May 2023 - 🇺🇸United States smustgrave
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!
- 🇫🇷France andypost
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: falseNot sure it makes sense to introduce new configuration and without UI, it could re-use "gzip" setting to produce compressed assets using gzip/brotli
- 🇬🇧United Kingdom catch
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.
- 🇳🇱Netherlands JesperN
Added rerolls of patches in #24 on Drupal 10.2.x. I think they should also apply on 11.x-dev.
- 🇺🇸United States SocialNicheGuru
Is there a variable we can set in settings.php?
- 🇫🇷France andypost
Not sure how tests can cover it but CR should mention update of nginx conf as well
- Merge request !9980Issue #3184242 by JesperN, prudloff, webflo, yogeshmpawar, ravi.shankar: Support brotli compression of assets → (Open) created by prudloff
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: truestatus 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.
- 🇫🇷France prudloff Lille
I added a test but it requires the brotli PHP extension.
I managed to build it in the CI but it does not seem to be enabled in tests so I'm not sure that's the correct way to do this.
Should we create an issue in the drupalci project to get the extension included in the Docker image?Makes me wonder if we should just do 'compress' as a boolean and write out everything we possibly can.
We might want to add support for zstandard compression after this ( #3346184: Does Drupal support ZSTD compression? → ) so I agree it might be easier to keep a single boolean that enables every possible algorithm depending on what the server can generate.
I'm not sure there is a use case for enabling gzip but not brotli. - 🇫🇷France andypost
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 →
- 🇫🇷France prudloff Lille
Postponed until ✨ Add brotli PHP extension Active .
