Files: 
CommentFileSizeAuthor
#30 1824778-css_gzip_compression-js_gzip_compression-30.patch11.66 KBACF
PASSED: [[SimpleTest]]: [MySQL] 49,284 pass(es). View
#27 1824778-css_gzip_compression-js_gzip_compression-27.patch11.56 KBACF
PASSED: [[SimpleTest]]: [MySQL] 49,047 pass(es). View
#21 1824778-css_gzip_compression-js_gzip_compression-21.patch3.52 KBCameron Tod
PASSED: [[SimpleTest]]: [MySQL] 48,211 pass(es). View
#16 1824778-css_gzip_compression-js_gzip_compression-16.patch3.52 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#13 1824778-css_gzip_compression-js_gzip_compression-7.patch3.5 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1824778-css_gzip_compression-js_gzip_compression-7.patch. Unable to apply patch. See the log in the details link for more information. View
#11 1824778-css_gzip_compression-js_gzip_compression-6.patch3.5 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 46,357 pass(es). View
#9 1824778-css_gzip_compression-js_gzip_compression-5.patch2.89 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 46,240 pass(es). View
#6 1824778-css_gzip_compression-js_gzip_compression-4.patch2.89 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#4 3-4-diff.txt732 bytesvijaycs85
#4 1824778-css_gzip_compression-js_gzip_compression-2.patch2.89 KBvijaycs85
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
#1 1824778-css_gzip_compression-js_gzip_compression.patch2.18 KBvijaycs85
PASSED: [[SimpleTest]]: [MySQL] 46,249 pass(es). View

Comments

vijaycs85’s picture

Status:Active» Needs review
FileSize
2.18 KB
PASSED: [[SimpleTest]]: [MySQL] 46,249 pass(es). View
alexpott’s picture

Issue tags:+Configuration system

Tagging

alexpott’s picture

Needs a hook_update_N in system.install.

vijaycs85’s picture

FileSize
2.89 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View
732 bytes

Here is the update hook

alexpott’s picture

The update function description needs works - 80 characters and capitalisation.

vijaycs85’s picture

FileSize
2.89 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Fixed here.

vijaycs85’s picture

Fixed error in #4.

vijaycs85’s picture

Fixed error in #4.

vijaycs85’s picture

FileSize
2.89 KB
PASSED: [[SimpleTest]]: [MySQL] 46,240 pass(es). View

Fixed error in #4, sorry for the duplicate comments.

heyrocker’s picture

Status:Needs review» Needs work

In default.settings.php there are the following variables commented out by default

# $conf['css_gzip_compression'] = FALSE;
# $conf['js_gzip_compression'] = FALSE;

These should be changed to use the new format for referring to config variables in $conf, which is $conf[<config_object_name>][<config_variable>]. So this should become

# $conf['system.performance]['gzip_compression.css'] = FALSE;
# $conf['system.performance]['gzip_compression.js'] = FALSE;
vijaycs85’s picture

Status:Needs work» Needs review
FileSize
3.5 KB
PASSED: [[SimpleTest]]: [MySQL] 46,357 pass(es). View

Fixed default.settings.php

alexpott’s picture

Status:Needs review» Needs work

+++ b/sites/default/default.settings.phpundefined
@@ -458,8 +458,8 @@
+# $conf['system.performance]['gzip_compression.css'] = FALSE;
+# $conf['system.performance]['gzip_compression.js'] = FALSE;

There's two missing single quotes... at the end of system.performance

vijaycs85’s picture

Status:Needs work» Needs review
FileSize
3.5 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1824778-css_gzip_compression-js_gzip_compression-7.patch. Unable to apply patch. See the log in the details link for more information. View

fixed.

aspilicious’s picture

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1824778-css_gzip_compression-js_gzip_compression-7.patch, failed testing.

pcambra’s picture

Status:Needs work» Needs review
FileSize
3.52 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install. View

Just rerolling and seeing if the testbot wakes up.

aspilicious’s picture

Cameron Tod’s picture

Code looks good to me, pending a retest.

Cameron Tod’s picture

Status:Needs review» Needs work
Issue tags:+Configuration system

The last submitted patch, 1824778-css_gzip_compression-js_gzip_compression-16.patch, failed testing.

Cameron Tod’s picture

Status:Needs work» Needs review
FileSize
3.52 KB
PASSED: [[SimpleTest]]: [MySQL] 48,211 pass(es). View

Fixed duplicate update function name.

longwave’s picture

Is using both "gzip" and "compression" in the key redundant? Couldn't they just be "gzip.css" and "gzip.js"? Note that "response.gzip" is just that, not "response.gzip_compression".

alexpott’s picture

I agree with #22 gzip.js and gzip.css is concise and meaningful.

alexpott’s picture

Status:Needs review» Needs work

+++ b/core/modules/system/config/system.performance.ymlundefined
@@ -8,3 +8,6 @@ preprocess:
 response:
   gzip: '0'
+gzip_compression:
+  css: '1'
+  js: '1'

hmmm...

Actually this could then become...

gzip:
  response: '0'
  css: '1'
  js: '1'
longwave’s picture

That, or maybe

cache:
  page:
    enabled: '0'
    max_age: '0'
css:
  preprocess: '0'
  gzip: '1'
js:
  preprocess: '0'
  gzip: '1'
response:
  gzip: '0'
stale_file_threshold: '2592000'
alexpott’s picture

I like the proposal in #25

ACF’s picture

Status:Needs work» Needs review
FileSize
11.56 KB
PASSED: [[SimpleTest]]: [MySQL] 49,047 pass(es). View

Did a re-roll of the patch but changed the system.performance.yml config file to look like the layout in #25. It meant I had to rename the preprocess variables.

heyrocker’s picture

This looks good to me, and I like the new file layout a lot better. Thanks!

heyrocker’s picture

Status:Needs review» Reviewed & tested by the community
ACF’s picture

Status:Reviewed & tested by the community» Needs review
FileSize
11.66 KB
PASSED: [[SimpleTest]]: [MySQL] 49,284 pass(es). View

Just rerolling for system.install upgrade number hopefully can go back to rtbc.

typhonius’s picture

Status:Needs review» Reviewed & tested by the community

Applies, and looks good from here!

Dries’s picture

Status:Reviewed & tested by the community» Fixed

Committed to 8.x. Thanks.

catch’s picture

Title:Covert css_gzip_compression and js_gzip_compression variables to CMI system» Convert css_gzip_compression and js_gzip_compression variables to CMI system
Status:Fixed» Active

I'm not sure this was entirely OK to convert as is:

- the $conf overrides were mainly useful in the case that you had page_cache_without_database enabled, that's broken at the moment - see #1576322: page_cache_without_database doesn't return cached pages without the database which I've just re-opened, but this patch added to the inability to do that.

- if there's a UI for these and we're going to require CMI access to serve cached pages whatever happens, then they could be removed from settings.php

- should we reverse the extension_load() and the config() calls? - this would save loading the configuration object if it's never going to needed anyway.

vijaycs85’s picture

Assigned:vijaycs85» Unassigned
alexpott’s picture

@catch I can't see how the ability to return a cached page without the database is affected by this one. The compression config should not be accessed if we are doing this.

So yep let's remove from settings.php and you're correct that we should swap the extension_load and config calls.

alexpott’s picture

Issue summary:View changes

Updated issue summary.

alexpott’s picture

jibran’s picture

It is #2183075: Tidy up css.gzip and js.gzip configuration. :-) Thanks for the followup @alexpott.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.