If you enable/disable the compress cached pages-setting (if you have cache enabled), Drupal will send the correct headers for that setting, but not refresh content to actually match this.

Steps to repeat:
- do a fresh cvs update
- make sure you have caching enabled (I currently have 'anonymous caching' and 5 min expiry/minimum lifetime enabled)
- visit your page, make sure you see a X-Drupal-Cache: HIT in your headers
- toggle the "compress cached pages" checkbox (in my case off to on)
- visit your page again

You should now either see plain content with a gzip flag or the opposite depending on which way you toggle the setting. Firefox will most likely complain about "Content Encoding Error".

While looking at modules/system/system.module each successful save _should_ run cache_clear_all(); but that apparently doesn't work or does nothing in my case.

CommentFileSizeAuthor
#19 804328-compress-clear-cache-19.patch1.09 KBDavid_Rothstein
#6 804328-compress-clear-cache.patch971 bytesjcfiala
#3 drupal-804328.patch992 bytesjcfiala

Comments

Anonymous’s picture

Anonymous (not verified) commented

Not sure if it matters, but I recall seeing something about theme specific overrides in post_save: I'm using the "seven" admin theme.

Anonymous’s picture

Anonymous (not verified) commented
Priority: Normal » Critical

I can still repeat this, and so will most likely other users. I consider this critical since drupal setups will stop to work for anonymous users.

Common settings (lifetime length isn't really relevant, only have it set):
* cache anonymous
* 5 min expiry lifetime
* 5 min minimum lifetime
* switch compress output flag

Here's anonymous curl output from from compressed initially set to checked: http://dpaste.com/hold/212325/
..and here's with the compress turned off: http://dpaste.com/hold/212323/

(spoiler: drupal still sends compressed data when the setting is switched to off since it serves from cache)

jcfiala’s picture

jcfiala commented
Status: Active » Needs review
StatusFileSize
new drupal-804328.patch992 bytes

So, here's a patch that does a cache_clear_all on 'cache_page' when the form is submitted.

alasda’s picture

alasda commented
Status: Needs review » Reviewed & tested by the community

Applied patch. Appears to work.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work 
+++ modules/system/system.admin.inc	28 Jun 2010 18:24:14 -0000
@@ -1694,11 +1694,21 @@ function system_performance_settings() {
+  $form['#submit'][] = 'system_performance_clear_page_cache';

I think we can write system_clear_page_cache().

+++ modules/system/system.admin.inc	28 Jun 2010 18:24:14 -0000
@@ -1694,11 +1694,21 @@ function system_performance_settings() {
+ * Submit callback; clear page caches.

There is only one page cache but multiple cached pages.

jcfiala’s picture

jcfiala commented
Status: Needs work » Needs review
StatusFileSize
new 804328-compress-clear-cache.patch971 bytes

Alright, I've adjusted my patch based on what Dries said.

davidburns’s picture

davidburns
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Reviewed & tested by the community

applied and tested. appears to work

dries’s picture

dries commented

Doing some more research, I discovered that it might be best to re-purpose http://api.drupal.org/api/function/system_clear_cache_submit/7.

jcfiala’s picture

jcfiala commented

Is that a great idea?

A) It clears _every_ cache, instead of just the page cache, which is all that was originally requested.
B) It will announce to the user 'Caches Cleared' when the user wasn't trying to clear a cache.

dries’s picture

dries commented

a) Given that this is a setting that you don't change every hour, it is OK to clear all caches.

b) That is not a great message to display.

We can introduce a new function per the original patch, but it would probably be great if we had consistent naming then. For example: system_clear_cache_submit and system_compress_cached_pages_submit or something?

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work

That sounds like a "needs work".

jcfiala’s picture

jcfiala commented
Assigned: Unassigned » jcfiala

@Dries:

Just so I'm not rerolling minor changes in this patch which _functionally_ fixes the problem for the next week, if I rename the submit function to 'system_compress_cached_pages_submit', will that be sufficient to get this patch cleared?

And if not, what do we need to get this passed?

David_Rothstein’s picture

David_Rothstein commented

Calling drupal_flush_all_caches() has been known to bring down websites, so let's not do that here unless we absolutely need to, please. It should really only happen when someone presses the big red button that's specifically for that purpose.

#6 looks pretty good. If we are going to rename functions, I don't think system_compress_cached_pages_submit() makes sense since the submit handler is not tied to that form element (arguably it could be, but probably not worth the effort); the function doesn't do anything with compressed pages specifically, but rather all pages. I would just rename it system_clear_page_cache_submit() and be done with it. It clears the page cache and it's a submit handler - seems like an obvious name :) If we want to explain the reason we're calling it, just add a code comment.

catch’s picture

catch
he/him
Primary language English
commented
Priority: Critical » Normal

Not critical - it's an edge case, and you can clear the cache manually.

Anonymous’s picture

Anonymous (not verified) commented

@catch: Not sure I agree with you. While it may be an edge case, it's probably hard to figure out "what's not working" from a (logged in) user perspective since he/her won't even recognize something's not working until logged out. The least thing to be done is to actually explain to the user that this isn't something you want to toggle every now and then (which "performance"-options are very tempting to mess with for overall increased snappyness).

marcingy’s picture

marcingy commented
Priority: Normal » Major

Changing to major as per tag.

mustanggb’s picture

mustanggb commented

Tag update

marcingy’s picture

marcingy commented
Status: Needs work » Needs review

#6: 804328-compress-clear-cache.patch queued for re-testing.

David_Rothstein’s picture

David_Rothstein commented
StatusFileSize
new 804328-compress-clear-cache-19.patch1.09 KB

Changed to system_clear_page_cache_submit() and added a code comment, as per #13.

marcingy’s picture

marcingy commented
Status: Needs review » Reviewed & tested by the community
dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Good fix -- thanks for incorporating my feedback. Committed to CVS HEAD.

Status: Fixed » Closed (fixed)

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