Page MenuHomePhabricator

Bugzilla: Add a query parameter to CSS URL to force browsers to reload cached CSS files
Closed, ResolvedPublic

Description

Screenshot of front page in Chrome.

Started recently. Could be cache related.


Version: wmf-deployment
Severity: enhancement
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=49474

Attached:

Screen_Shot_2013-06-17_at_4.07.37_PM.png (856×2 px, 191 KB)

Details

Reference
bz49720

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:00 AM
bzimport set Reference to bz49720.
bzimport added a subscriber: Unknown Object (MLST).

code-update-regression is somewhat useless in this case - it's not a mediawiki or related bug.

It seems to happen on every n requests. Usually goes away on a refresh or so

A hard refresh should resolve this. I can't find the other bug to dupe this one....

I'm pretty sure this is just bug 22170 comment 20 and a hard refresh should resolve the issue. But I'll leave it to you to mark this resolved as appropriate.

Other Bugzilla redesign fallout: bug 49474.

I could reproduce this now in Chrome 27.
skins/contrib/Wikimedia/index.css has these ancient entries:

#enter_bug { background: url(index/bug.gif)     no-repeat; }
#query     { background: url(index/search.gif)  no-repeat; }
#account   { background: url(index/account.gif) no-repeat; }

while in Firefox the delivered CSS file is correct:

#enter_bug { background: url(../../standard/index/file-a-bug.png)     no-repeat; }
#query     { background: url(../../standard/index/search.png)  no-repeat; }
#help      { background: url(../../standard/index/help.png)     no-repeat; }
#report    { background: url(index/report.png)  no-repeat; }
#account   {
  background: url(../../standard/index/new-account.png) no-repeat;
  margin-right: 0;
}

Not cache related. Can be consistently reproduced in at least Chrome.

The change is broken (incomplete CSS and/or not compatible with latest HTML). Please revert ASAP. Author to re-submit and work on it further.

Looks like it is cache after all, but our caching headers don't match between content and styles. We need a cache buster and linker between content and stylesheet (e.g. like a manual version of the old wgStyleVersion). Otherwise this will go wrong again in the future.

So:

  • [high] HTML needs to reference stylesheet with query parameter so that if you see an old version, the browsers also caches the old version. And when the html changes (incl. query parameter) a new version is forced to be fetched for css as well.
  • [low] To deploy faster to all users, needs a Last-Modified header. Right now browsers are told to basically cache it unconditionally so nobody will get the new content until their cache is manually purged.

So it looks like we talk about http://stackoverflow.com/questions/118884/what-is-an-elegant-way-to-force-browsers-to-reload-cached-css-js-files

I don't plan to revert the change as the functionality improvement outweighs the rendering issues.

(In reply to comment #7)

Looks like it is cache after all, but our caching headers don't match between
content and styles. We need a cache buster and linker between content and
stylesheet (e.g. like a manual version of the old wgStyleVersion). Otherwise
this will go wrong again in the future.

This is basically what I said in bug 22170 comment 23.

  • [high] HTML needs to reference stylesheet with query parameter so that if

you see an old version, the browsers also caches the old version. And when the
html changes (incl. query parameter) a new version is forced to be fetched for
css as well.

Let's make this bug about this issue.

  • [low] To deploy faster to all users, needs a Last-Modified header. Right

now browsers are told to basically cache it unconditionally so nobody will get
the new content until their cache is manually purged.

I've filed this as bug 49792.

(In reply to comment #8)

I don't plan to revert the change as the functionality improvement outweighs
the rendering issues.

The page certainly looks silly, but seems to be completely functional and usable. I don't think this is a high priority issue (lowering this field on the bug) and I agree with your assessment.

I have basically no clue about this, and if I interpret http://stackoverflow.com/questions/118884/what-is-an-elegant-way-to-force-browsers-to-reload-cached-css-js-files correctly and if I consider the information trustworthy, then adding a query parameter is not a great idea.

(In reply to comment #11)

I have basically no clue about this, and if I interpret
http://stackoverflow.com/questions/118884/what-is-an-elegant-way-to-force-
browsers-to-reload-cached-css-js-files
correctly and if I consider the information trustworthy, then adding a query
parameter is not a great idea.

Which answer are you referring to?

There's lots of ways to do this, some elaborate, some simple. A simple way is to include a querystring containing the file's modification time or hash in the static (CSS/JS) references. Then, whenever the HTML is generated, it will generate URLs uniquely identifying the current static content. This allows the static content to be cached for a long time (since changing the querystring changes the URL).

MW's ResourceLoader takes the modification time approach (though it has a lot of other functionality)

If you mean http://stackoverflow.com/a/119326/ ("According the letter of the HTTP caching specification, user agents should never cache URLs with query strings. While Internet Explorer and Firefox ignore this, Opera and Safari don’t"), that apparently isn't accurate (http://stackoverflow.com/a/85386/). However, if we wanted to be on the safe side we could use directories and a rewrite rule. Something like:

/static/mtime/file.css

rewritten to:

/static/file.css

I don't plan to work on this -> Setting lowest priority.

Looking at boogs.wmflabs.org I can see

<link href="skins/contrib/Wikimedia/global.css?1377539412" rel="stylesheet"
    type="text/css" title="Wikimedia"><!--[if lte IE 7]>

Looking at bugzilla.wikimedia.org I get

<link href="skins/contrib/Wikimedia/global.css" rel="stylesheet"
    type="text/css" title="Wikimedia"><!--[if lte IE 7]>

Both run Bugzilla 4.2. Wondering what's creating the difference here.

The cache-busting timestamp is affixed to the URL by the 'mtime' filter:

http://git.wikimedia.org/blob/wikimedia%2Fbugzilla%2Fmodifications.git/eee3bdc9f86d8a7dc8dad5ef907f1818f1fe71ac/template%2Fen%2Fcustom%2Fglobal%2Fheader.html.tmpl#L120

If you look up the implementation of 'mtime', it appears to check for the existence of a 'BZ_CACHE_CONTROL' environment variable. When it is set, the timestamp is affixed; when it is not set, 'mtime' does nothing.

Thanks for the help! Looking at .htaccess was also my guess after reading http://bzr.mozilla.org/bugzilla/trunk/revision/7391

So much for that theory. :(
Revision 8748 of http://bzr.mozilla.org/bugzilla/trunk/view/head:/.htaccess is nearly the same as our .htaccess and we have

SetEnv BZ_CACHE_CONTROL 1

set. Only difference in upstream is an additional

<IfModule mod_rewrite.c>
  RewriteEngine On
  RewriteRule ^rest/(.*)$ rest.cgi/$1 [NE]
</IfModule>

at the end.

(In reply to comment #20)

So much for that theory. :(
Revision 8748 of http://bzr.mozilla.org/bugzilla/trunk/view/head:/.htaccess
is
nearly the same as our .htaccess and we have

SetEnv BZ_CACHE_CONTROL 1

set.

What do you mean by "our .htaccess"? Most Wikimedia Apache servers are configured to ignore .htaccess files, I believe, as enabling .htaccess support is a performance hit. Are you sure this file is being used?

Ahem, while digesting the 4.4 release notes, I ran into http://www.bugzilla.org/releases/4.0/release-notes.html#v40_feat_js_css_update :

mod_headers
mod_expires
mod_env

need to be installed.

Running /srv/org/wikimedia/bugzilla/checksetup.pl on kaulen should tell us.

Still same problem on zirconium (new server that will replace kaulen).

Change 127254 had a related patch set uploaded by Krinkle:
bugzilla apache: Enable required modules for caching

https://gerrit.wikimedia.org/r/127254

Change 127254 merged by Dzahn:
bugzilla apache: Enable required modules for caching

https://gerrit.wikimedia.org/r/127254

we are now loading the required Apache modules:

expires.load -> ../mods-available/expires.load
headers.load -> ../mods-available/headers.load
env.load -> ../mods-available/env.load