Last modified: 2014-07-10 12:11:01 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T51720, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 49720 - Bugzilla: Add a query parameter to CSS URL to force browsers to reload cached CSS files
Bugzilla: Add a query parameter to CSS URL to force browsers to reload cached...
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Bugzilla (Other open bugs)
wmf-deployment
All All
: Normal enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on: 49792
Blocks:
  Show dependency treegraph
 
Reported: 2013-06-17 23:08 UTC by Krinkle
Modified: 2014-07-10 12:11 UTC (History)
7 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Screenshot of front page in Chrome. (191.70 KB, image/png)
2013-06-17 23:08 UTC, Krinkle
Details

Description Krinkle 2013-06-17 23:08:39 UTC
Created attachment 12568 [details]
Screenshot of front page in Chrome.

Started recently. Could be cache related.
Comment 1 Sam Reed (reedy) 2013-06-17 23:10:38 UTC
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
Comment 2 MZMcBride 2013-06-17 23:11:22 UTC
A hard refresh should resolve this. I can't find the other bug to dupe this one....
Comment 3 MZMcBride 2013-06-17 23:13:19 UTC
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.
Comment 4 MZMcBride 2013-06-17 23:14:29 UTC
Other Bugzilla redesign fallout: bug 49474.
Comment 5 Andre Klapper 2013-06-18 03:37:33 UTC
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;
    }
Comment 6 Krinkle 2013-06-18 08:45:06 UTC
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.
Comment 7 Krinkle 2013-06-18 08:48:53 UTC
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.
Comment 8 Andre Klapper 2013-06-18 17:13:25 UTC
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.
Comment 9 MZMcBride 2013-06-19 02:32:46 UTC
(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.
Comment 10 MZMcBride 2013-06-19 02:34:26 UTC
(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.
Comment 11 Andre Klapper 2013-06-19 04:21:37 UTC
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.
Comment 12 Matthew Flaschen 2013-06-19 04:31:53 UTC
(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)
Comment 13 Matthew Flaschen 2013-06-19 04:39:59 UTC
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
Comment 14 Ori Livneh 2013-06-19 05:32:01 UTC
Upstream: https://bugzilla.mozilla.org/show_bug.cgi?id=428313
Comment 15 Andre Klapper 2013-08-30 21:19:57 UTC
I don't plan to work on this -> Setting lowest priority.
Comment 16 Andre Klapper 2013-09-10 19:04:34 UTC
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.
Comment 17 Ori Livneh 2013-09-11 05:07:51 UTC
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.
Comment 18 Ori Livneh 2013-09-11 05:08:22 UTC
I meant to link to the mtime implementation, too:
http://lxr.mozilla.org/bugzilla/source/Bugzilla/Template.pm#412
Comment 19 Andre Klapper 2013-09-11 16:39:26 UTC
Thanks for the help! Looking at .htaccess was also my guess after reading http://bzr.mozilla.org/bugzilla/trunk/revision/7391
Comment 20 Andre Klapper 2013-09-19 18:29:57 UTC
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.
Comment 21 MZMcBride 2013-09-19 23:47:44 UTC
(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?
Comment 23 Andre Klapper 2013-10-26 13:48:49 UTC
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.
Comment 24 Andre Klapper 2014-01-23 15:59:15 UTC
Still same problem on zirconium (new server that will replace kaulen).
Comment 25 Gerrit Notification Bot 2014-04-23 09:08:22 UTC
Change 127254 had a related patch set uploaded by Krinkle:
bugzilla apache: Enable required modules for caching

https://gerrit.wikimedia.org/r/127254
Comment 26 Gerrit Notification Bot 2014-07-08 23:22:34 UTC
Change 127254 merged by Dzahn:
bugzilla apache: Enable required modules for caching

https://gerrit.wikimedia.org/r/127254
Comment 27 Daniel Zahn 2014-07-08 23:29:32 UTC
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
Comment 29 Andre Klapper 2014-07-10 12:11:01 UTC
Thank you!

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links