Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
If page caching is on, and css aggregation is on:
- Anonymous user goes to /, front page gets cached with references to an aggregated CSS file
- If CSS changes and anon user goes back to front page, it still references the old CSS file because the HTML is cached. This is okay
- If an admin goes to admin/build/themes, system_themes gets called. The first function call is drupal_clear_css_cache() which deletes all aggregated CSS files.
- If an anon user goes back to /, it still references the same CSS file, but it no longer exists!
Whenever the CSS cache is cleared, page cache should also be cleared.
This patch provides that in common.inc
Comment | File | Size | Author |
---|---|---|---|
#30 | system.admin-276615.patch | 947 bytes | mikeytown2 |
#23 | system.admin_.276615.patch | 961 bytes | mikeytown2 |
#18 | system.admin_.276615.patch | 1.22 KB | mikeytown2 |
#16 | system.admin_.inc_.276615.patch | 965 bytes | mikeytown2 |
#1 | common.inc_.diff | 642 bytes | drumm |
Comments
Comment #1
drummCommitted to 5.x. Needs to go into 6.x and HEAD.
Comment #2
drummComment #3
Robin Monks CreditAttribution: Robin Monks commentedPatch works on 7.x. RTBC!
Robin
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis one is crying for a test.
Comment #5
Robin Monks CreditAttribution: Robin Monks commented#4, Damien: I just tested for 7.x. Let's get it in there at least and then "backport" to 6.x after.
Robin
Comment #6
Damien Tournoud CreditAttribution: Damien Tournoud commentedComment #7
Robin Monks CreditAttribution: Robin Monks commentedComment #8
Dries CreditAttribution: Dries commentedPutting cache_clear_all() in drupal_clear_css_cache() is a bit of a hack, IMO. All of a sudden, drupal_clear_css_cache() does a ton more than it is supposed to do. Needs more discussion, and most likely needs work too.
Comment #9
JacobSingh CreditAttribution: JacobSingh commentedHmm...
I had the same thought, but couldn't think of a case when this wasn't the minimum desired action.
In what case would you want to clear out the CSS without also clearing out the page cache?
Comment #10
Robin Monks CreditAttribution: Robin Monks commentedPerhaps this should be fixed by going through core and in every case where css is cleared, also making sure the page cache is cleared. It would be more code in the end, but would allow drupal_clear_css_cache to remain simple and dedicated to it's task.
Another alternate is doing away with drupal_clear_css_cache and only having cache_clear_all.
I'm just thinking out loud here.
Robin
Comment #11
Anonymous (not verified) CreditAttribution: Anonymous commentedThe last submitted patch failed testing.
Comment #12
mikeytown2 CreditAttribution: mikeytown2 commentedcache should only be cleared if the form on that page was submitted. Boost uses this hook to clear its cache on a submit if js and/or css aggregation is on
Core should do the same; clear cache on submit.
Comment #13
ARray CreditAttribution: ARray commentedwaiting for 6.x bugfix.
Comment #14
mikeytown2 CreditAttribution: mikeytown2 commentedRemove
drupal_clear_css_cache();
from http://api.drupal.org/api/function/system_themes_form/7.Add
drupal_clear_css_cache();
to http://api.drupal.org/api/function/system_themes_form_submit/7.And problem solved, right?
Comment #15
najibx CreditAttribution: najibx commentedwaiting for 6.x bugfix ...
Comment #16
mikeytown2 CreditAttribution: mikeytown2 commentedComment #17
mikeytown2 CreditAttribution: mikeytown2 commentedAs a side note, this issue prevented me from writing a patch sooner #475826: CVS Instructions on Project Page <- Please fix asap!
Comment #18
mikeytown2 CreditAttribution: mikeytown2 commenteddrupal_clear_js_cache() & drupal_clear_css_cache() should only be called from drupal_flush_all_caches(). Right now it seems a little messy. This patch puts all calls to flushing the css/js cache through drupal_flush_all_caches().
Comment #19
mikeytown2 CreditAttribution: mikeytown2 commentedThere needs to be some sort of hook/timestamp that will let modules like boost know that one aspect of the file cache has been cleared. Modules out there call drupal_clear_js_cache() #477830: Refactor local file caching and cache clear logic and there is no good way of detecting that it was flushed. Any ideas on how we can address this?
Comment #20
JirkaRybka CreditAttribution: JirkaRybka commentedAs for #18 patch - we shouldn't use drupal_flush_all_caches() this way (it was already discussed on the initial issue that added the central flushing button to the performance settings page). While it surely ensures all relevant data being flushed, it's a huge overkill too - things like rebuilding menus etc. are not needed on Themes page, and may take quite long and/or slow down large production sites. The flush all caches thing is for the button on Performance page, but otherwise we need to keep the granularity of cache flushing.
Comment #21
mikeytown2 CreditAttribution: mikeytown2 commentedHow about a new function for flushing the file cache, that can be hooked; so when it's called the boost file cache gets cleared as well? #16 needs to be back ported though.
Comment #22
catchYeah the patch as it stands is going to lead to similar complaints to what we have for admin/build/modules, needs something more specific.
Comment #23
mikeytown2 CreditAttribution: mikeytown2 commentedAt least for admin/build/modules it doesn't nuke the cache on when viewing that page. Re-roll of #16
Comment #24
mikeytown2 CreditAttribution: mikeytown2 commentedFixed a spelling error in the title.
Opened a new issue #484524: Hook for file caches.
Comment #26
lilou CreditAttribution: lilou commentedSetting to 'needs review' - testbot was broken.
Comment #27
alexanderpas CreditAttribution: alexanderpas commenteddo we need a test for this? otherwise RTBC.
Comment #28
mikeytown2 CreditAttribution: mikeytown2 commentedI can't think of a test for this, it is a fairly straightforward bug fix.
Comment #29
Dries CreditAttribution: Dries commentedCommitted to CVS HEAD. Thanks.
Comment #30
mikeytown2 CreditAttribution: mikeytown2 commentedFor 6.x
Comment #31
alexanderpas CreditAttribution: alexanderpas commentedComment #32
EnekoAlonso-1 CreditAttribution: EnekoAlonso-1 commentedWell, today I have set up CacheRouter on my server with file as the cache method and it has been only a few hours until I have noticed the issue with the cached/obsolete css and js compressed filenames. As most of you are aware, it happens than when the css and/or js compressed files are regenerated, all the previously cached pages get obsolete, since the old path is no longer available.
Now, although I think adding hooks to clear the page cache when these css and js files are recreated, I think that is not the appropriate solution. Instead, I think the main issue resides in giving a new file name to these compressed files, by using an md5 string that changes every time.
Yes, I understand this is done to prevent browser cache issues, but, to tell you the truth, I'll rather have that issue than having to regenerate all cached pages on my site (currently I have 26k nodes and 500k comments with their own page).
So I have opted for patching common.inc to keep the filename of the compressed files constant over time:
Another solution, obviously, is to disable css and js compression, which I still would think would be better than having to expire all my cached pages.
Comment #33
Gábor HojtsyCommitted to Drupal 6 too, thanks.