According to RFC 2616, the correct value for the Expires: header is a Unix timestamp, whereas the value of Cache-Control: max-age is relative to now. Not sure if this distinction has any practical value, nevertheless it can be corrected :)

CommentFileSizeAuthor
#1 admin_menu-maxage-129838.patch696 bytessmk-ka

Comments

smk-ka’s picture

Status: Active » Needs review
StatusFileSize
new696 bytes

Attached patch fixes the max-age value.

sun’s picture

+++ b/admin_menu.module
@@ -299,10 +299,10 @@ function admin_menu_js_cache($hash = NULL) {
-  header('Cache-Control: max-age=' . $expires);
+  header('Cache-Control: max-age=' . $max_age . ', private');

The addition of "private" looks suspicious to me - any particular reason for that?

That said, the entire cache output code is still based on D6 core, whereas D7 vastly improved it (including the gzip and whatnot mangling). Was actually playing with the idea of entirely removing the custom code and (ab)using drupal_serve_page_from_cache() with a stub/mock $cache object instead, but didn't have time to look into that yet. What do you think?

Status: Needs review » Needs work

The last submitted patch, admin_menu-maxage-129838.patch, failed testing.

sun’s picture

Status: Needs work » Fixed

Thanks for reporting, reviewing, and testing! Committed to all branches.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Status: Fixed » Closed (fixed)

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