Rework admin/settings/performance

Looking at the screenshots, it feels like 'page compression' now belongs under bandwidth optimizations instead of 'caching'.

Maybe we should rename 'Optimize CSS' to 'Compress CSS'? Ditto for JS. That would make the 3 options nicely aligned, and it provides a bit more information to the reader.

One of the things that has always annoyed me, is how the 'clear cache' button is all the way at the bottom. As a developer, reviewer and committer I do a lot of navigation to that page, and then scrolling down to that button. This patch will help because it makes the page shorter.

We don't actually compress CSS and JS - they're neither packed nor gzipped in core, just aggregated together into one file, so I'd be wary of using compression there.

I think the last patch is an improvement. When testing the patch, I wondered if we could hide the cache lifetime field when neither the block or the page cache are disabled.

Either way, I'd move the lifetime setting to the bottom of the caching-fieldset. The primary concern is not the lifetime but enabling or disabling the caching.

#472698: admin/settings/performance: Keyboard <enter> key clears cache instead of applying settings marked as duplicate

OK, to pull some information from the duplicate issue: The initial problem over on that issue is, that using ENTER [keyboard] to submit the form performs the cache-clear operation instead, which is unexpected and changes to settings are lost (not saved). That's because the form have in fact two submit buttons, cache-clear being first.

rfay posted a patch to change the cache-clear button into a simple link. There seems to be agreement that such approach would need a confirmation form, but IMO that's not the way to go (making the simple cache-flush operation even more complicated on UI).

I mentioned, that we might split the page into two separate forms, each with it's own submit. The current flushing button is not really part of the settings already, so having it outside of the form makes sense. rfay said that he had problems with this approach due to system_settings_form() encapsulating some features, but I guess just doing return system_settings_form($form) . drupal_get_form($another_form); might work.

If the feature stays inside the form, it should change to something else than a submit button - we still have other options than plain link: Perhaps a checkbox to clear caches on the whole form's submit? That might be checked by default even (serving more as just a visual reminder/description of the flushing feature), as the other settings involve a lot of flushing already. I'm not sure whether we may have a button inside form, that's not submit [i.e. not treated like one in browser, when keyboard ENTER is used to submit] - I didn't investigate on this, there might be another chance in there too.

Personally, I would prefer two separate forms.

Another tab means another click needed :-( And I don't think there's really a need to add to the page - we already have a hook to add to this central cache flush, whose basic idea is to flush just *everything* so that the user gets simply and reliably out of *any* trouble with stale data, without further options to understand.

Can we just concantenate two forms on the same page, as I mentioned above, or is there some other problem with that? It won't change the page that much. Modules may even hook_form_alter() the flushing form, should they need that in some edge case.

Another opinion, now we have ertical tabs in core so it's possible to split cache settings/actions with different tabs.
As for me it's different functionality and which one is more recently used this should be opened by default.
Suppose settings is visited rarely then settings.

How do we move this issue forward? I'll be happy to help if I can. It seems the patch doesn't even apply any more.

- "This option should be disabled when using a webserver that performs compression." -> "Unnecessary if your webserver compresses pages."?

This might be not just "unnecessary" - I seem to recall there used to be issues with double-compressed pages. But I may be wrong, better check.

The "cache blocks" settings is visually odd. I think it would look more balanced with the page cache if we'd used radio-buttons in this case.
I

Since #472698: admin/settings/performance: Keyboard <enter> key clears cache instead of applying settings was closed in favor of this, I was looking for some mention of the cache-flush button getting fixed, but found none in this thread. The patch seems to change type from "submit" to "button" though. Does that help with the [keyboard] Enter submitting issue? (Let's state that here)

Doesn't block caching checkbox need a bit of description, to indicate what it is, and to be in-line with rest of the page?

Does we really need "Cache blocks" ? Every block already with own cache settings, suppose this should be deprecated.

--Page caching--

I'm not sure why we provide the option to choose between normal and aggressive caching. If any enabled modules will break under aggressive caching we should just disable it. And if none will break, we should just enable aggressive caching to give us the best performance.

As far as I can tell we are just requiring humans to make a technical decision that should be made in code.

This is particularly problematic because no one can really make an informed choice here unless they understand how caching works as a developer.

This would be another issue, of course. But this may save us worrying about this problem in this issue if others agree.

--Page caching 2--

We've lost the page caching description starting 'Enabling the page cache will offer a significant performance boost' and not replaced it with anything. I don't think this text was redundant.

--Block caching--

No explanation at all? I think we are assuming a lot of knowledge here. (Though the old version had a lot of redundant text, so this is actually probably better).

--Explanations for developers--

If you are a developer you should know that caching can break stuff. I think we should get rid of these warnings.

The new patch doesn't seem to address some of the concerns raised?

As for the clearing button, my suggestion is the same as before: Render the "Clear cache" fieldset as separate form, and append it to the same page after the settings form (or before, like Views do - visually at least, otherwise it suffers the same problem).

Also, the description of that button seems to be gone (on screenshot above), but I believe it should be kept. This is a troubleshooting feature, not only for developers. Users need to see this direction in troubleshooting, somewhere, which is why the button was first added. There's no obvious connection between "help, this my new translation doesn't take effect" and "Clear all caches", unless you know a lot.

causes it to be displayed either under the submit buttons

I believe this makes even more sense than what we have now, as this is not a part of the settings submitted by regular buttons.

Also, the last place a user is going to look when his theme doesn't work as expected is the bottom of a page called Performance.

Agreed. But we don't have a central "Troubleshooting" page, and it's better to have it somewhere, than nowhere. We don't need to explain caching all around, but we need that bit about "troubleshooting modules, themes, and translations" to make the feature recognizable for everyone.

+1 for #34.

the last place a user is going to look when his theme doesn't work as expected is the bottom of a page called Performance.

Exactly, but they will click on a page called "Troubleshooting". I think taking the "Clear caches" button away from performance makes sense, as it has nothing to do with performance (except draining it...) on an application level. It of course has to do with performance in that the caches that are flushed are only ever introduced to improve performance, but when you click that button, it has nothing to do with performance optimizations or performance settings.

Definitely a different patch, though.

I second the motion on #34: The clear cache possibly doesn't even belong here.

But having checkboxes for

• Clear database caches
• Clear CSS optimization file
• Clear JS consolidation file

Might work just fine here.

Moving it to a separate page could work for me (but might require some additional thought).

I still think we need to do something extra with 'block caching'.

I'd be happy to roll a patch for it on a different page.

What would we call it? Cache Maintenance?

It could have checkboxes for the various items, pre-checked, with a submit button and some explanatory text.

I created #496248: Refactor cache API

Just trying out the patch from #28 -

On firefox or seamonkey (Ubuntu or Windows) when you go to admin/settings/performance it shows the screen but immediately redirects to admin/settings/clean-urls. You see the performance screen go by, but there's an immediate redirect.

On IE8/WinXP, the patch works fine.

This patch seems to work fine if #479604: Drupal.behaviors.cleanURLsSettingsCheck() in system.js redirects to clean urls settings page incorrectly. is installed.

I note that the default settings are quite different than they used to be. (normal page caching, for example, used to be the default; now it seems to be no caching).

Xano: I think it lacks some context and help. If we used radio buttons, we could have two options: "(i) Disabled" and "(ii) Normal (recommended)". At least it would make a recommendation for people that don't understand block caching. I don't know, the checkbox feels 'very naked' to me without anything else.

Logically, I also think 'block caching' is secondary to 'page caching'. It's just a "feeling" but it makes me want to put it below page caching.

If it were radio buttons, you could put it below page caching because it would look as if it is on the same level a bit more -- I think.

I'd place the "cache blocks" option just above the cache life time option. I wouldn't use radio buttons for only 2 options though (a check box looks much cleaner than 2 radio buttons.)

Overall, the patch really cleans up the performance page by replacing radio widgets (only useful when there are 3 or more options) with check boxes.

+1 Still hoping we can get this done for D7. Especially getting that clear cache button to a happier place :-)

I did a casual once-over and it looks fine to me and works fine for me, except of course that I want the "Clear cache" to move out of here.

"Cache blocks" might be improved with a few more words, since both of those words can be either nouns or verbs, and it can be a little confusing. It might be a bit clearer as "Cache Drupal blocks for unauthenticated users" or something of that type.

Page caching is exclusively for anonymous users. You can't have it for logged in users, unless there was a big change in D7.

Blocks are cached for all/any users, pages are cached for anonymous users only.

I think this is a big step forward, so I committed it to CVS HEAD. We can follow-up with more refinements! Thanks Xano.

variables should use a bool instead of an int since it is a checkbox.
$_SERVER['HTTP_ACCEPT_ENCODING'] is the wrong check. Thats what the browser tells the server, if it will accept gzip encoding; I get gzip,deflate. Speaking of deflate, if checking, that needs to be checked for as well. One way to check if the server is already sending compressed output would be similar to how we check for clean urls; but look at the headers via http://api.drupal.org/api/function/drupal_http_request/7. Be sure to send it out with Accept-Encoding: gzip,deflate. Just make sure it hits a page that won't be cached when checking for Content-Encoding: gzip (do a post request).

btw, while everyone is here please look at this: #543892: Win XAMPP 1.7.2 Regex PHP Bug - Optimize css & long css comments fails

one more thing, saying compression on Aggregate and compress CSS files into one file. is not clear, especially when Compress cached pages is right above it. Changing these to "gzip compress" & "whitespace compress" would be better.

I agree that $_SERVER['HTTP_ACCEPT_ENCODING'] is plain incorrect and should be removed or replaced by a http request and header check (gzipping could also be being done by a reverse proxy, so there is really no way to check this without a network request).

This is, in it's current form a bug IMHO, and a bad one at that.

Is normal vs. aggressive caching now selected automatically, as suggested in #26, or was it removed entirely? If the former is true, perhaps there should be some indication of what caching mode has been selected for you (and why). As it stands in HEAD, the behavior seems ambiguous. Of course, this might only be an issue for long-time Drupal users; overall, I'm sure the Performance page makes a lot more sense to new users now.

This replaces the HTTP_ACCEPT_ENCODING check with a check for zlib.output_compression. At least this catches one common configuration with transparent compression.

As mentioned, a more thorough check would require a HTTP callback. I'm not sure it's worth it, though. Users who installed mod_gzip or a compressing proxy are more likely to know what they are doing.

Hiding the checkbox does not turn off the setting but just prevents the user from changing it, even when it has been set to off. We need to do the same check when the variable is accessed.

Actually the page cache works fine with zlib.output_compression enabled, and no double encoding takes place. zlib.output_compression only compresses the pages that are not served from the cache.

Good hit, no need to check this ($_SERVER['HTTP_ACCEPT_ENCODING'], 'gzip') in admin settings because it depends on visitor settings from browser so this is a job for the page serving system. This was fixed in #43462: cache_set and cache_get base_url brokenosity but still not backported to 6

Nice clean-up. Committed to HEAD, thanks!

Hi!
I am new to Drupal. And I have to create single file for ALL CSS and JS file for caching purpose. Can Anyone help me.
And I want o know how to use this patch.

Waiting for quick response.
Thank You.

@mrj0909: Welcome to Drupal.

This isn't really the place for your support request, though. You can create a new support request for Drupal 6, and find out about other support options at http://drupal.org/support.

The Drupal 6 admin/settings/performance does aggregate your CSS files into a single file, likewise with your javascript files, with some exceptions. Just go to admin/settings/performance.