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 KBcam8001
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
StatusFileSize
new2.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

StatusFileSize
new2.89 KB
FAILED: [[SimpleTest]]: [MySQL] Invalid PHP syntax in core/modules/system/system.install.
[ View ]
new732 bytes

Here is the update hook

alexpott’s picture

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

vijaycs85’s picture

StatusFileSize
new2.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

StatusFileSize
new2.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
StatusFileSize
new3.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
StatusFileSize
new3.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
StatusFileSize
new3.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

cam8001’s picture

Code looks good to me, pending a retest.

cam8001’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.

cam8001’s picture

Status:Needs work» Needs review
StatusFileSize
new3.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
StatusFileSize
new11.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
StatusFileSize
new11.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.