Bug 12904 - Force browser to load new JavaScript and CSS files after upgrade
Summary: Force browser to load new JavaScript and CSS files after upgrade
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Josef Moravec
URL:
Keywords:
Depends on:
Blocks: 20187 20188 20189 20190 20193 20420 20480 20880
  Show dependency treegraph
 
Reported: 2014-09-10 16:17 UTC by Kyle M Hall
Modified: 2019-10-14 19:57 UTC (History)
16 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
For non-package installations, the following rewrite rules will need to be added to the apache config file: RewriteRule ^(.*)_[0-9][0-9]\.[0-9][0-9][0-9][0-9][0-9][0-9][0-9].js$ $1.js [L] RewriteRule ^(.*)_[0-9][0-9]\.[0-9][0-9][0-9][0-9][0-9][0-9][0-9].css$ $1.css [L]
Version(s) released in:


Attachments
Bug 12904 - Force browser to load new javascript files after upgrade (20.02 KB, patch)
2014-09-10 16:19 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 12904 - Force browser to load new javascript files after upgrade (22.39 KB, patch)
2014-09-11 16:22 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (21.86 KB, patch)
2014-11-06 07:34 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase JS Cache Timeout (1.13 KB, patch)
2014-11-06 08:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Give same treatment to CSS (110.83 KB, patch)
2014-11-06 09:36 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Give same treatment to CSS (111.98 KB, patch)
2014-11-06 09:38 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase CSS Cache Timeout (1.10 KB, patch)
2014-11-06 09:39 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Catch remaining JS files (94.58 KB, patch)
2014-11-06 15:46 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (21.86 KB, patch)
2014-11-06 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase JS Cache Timeout (1.13 KB, patch)
2014-11-06 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Give same treatment to CSS (112.03 KB, patch)
2014-11-06 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase CSS Cache Timeout (1.10 KB, patch)
2014-11-06 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Catch remaining JS files (94.85 KB, patch)
2014-11-06 16:04 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (21.86 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase JS Cache Timeout (1.13 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Give same treatment to CSS (112.03 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase CSS Cache Timeout (1.10 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Catch remaining JS files (94.85 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 [Followup] - Add to Intranet for Dev Install (1.12 KB, patch)
2014-11-06 16:44 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (21.66 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase JS Cache Timeout (1.13 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904 [Followup] - Give same treatment to CSS (109.66 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904 [Followup] - Increase CSS Cache Timeout (1.10 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904 [Followup] - Catch remaining JS files (91.34 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904 [Followup] - Add to Intranet for Dev Install (1.22 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: Update new occurrences of .js|.css (77.19 KB, patch)
2016-03-07 11:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
script to update tt & inc files (1.14 KB, application/x-perl)
2016-03-07 11:57 UTC, Jonathan Druart
Details
screenshot (156.48 KB, image/png)
2016-03-24 22:27 UTC, Mirko Tietgen
Details
Bug 12904 - Force browser to load new javascript files after upgrade (5.71 KB, patch)
2016-08-15 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (218.00 KB, patch)
2016-08-15 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (5.71 KB, patch)
2016-08-15 17:32 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (236.16 KB, patch)
2016-08-15 17:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (5.71 KB, patch)
2016-12-22 12:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (234.43 KB, patch)
2016-12-22 12:20 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 [QA Followup] - Don't send 301 status (2.93 KB, patch)
2016-12-22 15:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (5.76 KB, patch)
2016-12-22 15:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (234.48 KB, patch)
2016-12-22 15:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 12904 [QA Followup] - Don't send 301 status (2.99 KB, patch)
2016-12-22 15:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (5.83 KB, patch)
2017-03-06 10:28 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (236.11 KB, patch)
2017-03-06 10:28 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12904 [QA Followup] - Don't send 301 status (3.05 KB, patch)
2017-03-06 10:29 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12904 - Add mod_expire to etc/koha-httpd.conf (1.44 KB, patch)
2017-03-06 10:33 UTC, Olli-Antti Kivilahti
Details | Diff | Splinter Review
Bug 12904 - Force browser to load new javascript files after upgrade (5.88 KB, patch)
2017-05-24 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Update template css and js file references (236.13 KB, patch)
2017-05-24 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 [QA Followup] - Don't send 301 status (3.11 KB, patch)
2017-05-24 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 - Add mod_expire to etc/koha-httpd.conf (1.40 KB, patch)
2017-05-24 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904 [QA Followup] - Look up the Koha version once (121.30 KB, patch)
2017-05-24 14:07 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 12904: Force browser to load new javascript files after upgrade (5.91 KB, patch)
2018-01-23 13:30 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: (QA follow-up) Don't send 301 status (3.12 KB, patch)
2018-01-23 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: Add mod_expire to etc/koha-httpd.conf (1.41 KB, patch)
2018-01-23 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: Force browser to load new javascript files after upgrade (263.90 KB, patch)
2018-01-23 13:44 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: Pass KOHA_VERSION in C4::Auth (1.77 KB, patch)
2018-01-23 13:45 UTC, Jonathan Druart
Details | Diff | Splinter Review
script to update tt & inc files (721 bytes, application/x-perl)
2018-01-23 13:46 UTC, Jonathan Druart
Details
Bug 12904: Fix occurrences not caught by script (2.84 KB, patch)
2018-02-06 13:41 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 12904: Apache configuration (2.70 KB, patch)
2018-02-06 13:58 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 12904: Add mod_expire to etc/koha-httpd.conf (1.45 KB, patch)
2018-02-06 13:58 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 12904: Pass KOHA_VERSION in C4::Auth (1.81 KB, patch)
2018-02-06 13:58 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 12904: Force browser to load new javascript files after upgrade (261.96 KB, patch)
2018-02-06 13:58 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 12904: Fix occurrences not caught by script (2.88 KB, patch)
2018-02-06 13:58 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 12904: Add KOHA_VERSION to few more .css (12.45 KB, patch)
2018-02-16 16:00 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2014-09-10 16:17:24 UTC
A common issue with Koha seems to be that the web browsers continue to use cached js files after an upgrade. If we append the version as a parameter, we can force the browser to update the javascript files.
Comment 1 Kyle M Hall 2014-09-10 16:19:29 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2014-09-11 16:22:09 UTC Comment hidden (obsolete)
Comment 3 Owen Leonard 2014-09-11 16:48:15 UTC
I'm not sure how relevant this is, but running a performance audit from Chrome Developer Tools gives this warning: "Resources with a "?" in the URL are not cached by most proxy caching servers."

Digging into that issue led me to this:

http://www.stevesouders.com/blog/2008/08/23/revving-filenames-dont-use-querystring/

Sounds like there are Apache configuration options, for instance, to rewrite file names for this purpose. I have no idea whether that fits into Koha's upgrade process.

I think we should do a general audit on how we handle compression and caching at the server level--preferably by someone who knows more about it than me.
Comment 4 Martin Renvoize 2014-09-11 19:39:51 UTC
I've been thinking about this problem for a few yers now.. though never gotten around to doing much about it.

I did at some point start using mod_pagespeed (by google) on a number of our server, but this unfortunately led to other problems which I could not narrow down to anything but this apache module.

However, the real piece I wanted was this caching advantage enabled using the following filter:

https://developers.google.com/speed/pagespeed/module/filter-cache-extend

I'm wondering if we could do somthing similar, either in the Apache config itself, or in perl pre apache.

My thought's were to append a version string within the templates (similar to Kyle's approach but within the filename as opposed to a query string) and then add some re-write rules in apache to remove the appended string again. (I got stuck at the apache re-write rules level.. I've never been especially good at them) I did wonder whether we should be appending a hash of the file content as opposed to a versions for our version string.. to enable the detection of customised changes, but this would obviously add to server load too.
Comment 5 Kyle M Hall 2014-11-05 13:13:34 UTC
I think the biggest issue with this approach is it's tied to using Apache, right? This solution should work with apache, nginx, or whatever crazy way someone may have Koha up and running. Am I correct?

(In reply to Martin Renvoize from comment #4)
> I've been thinking about this problem for a few yers now.. though never
> gotten around to doing much about it.
> 
> I did at some point start using mod_pagespeed (by google) on a number of our
> server, but this unfortunately led to other problems which I could not
> narrow down to anything but this apache module.
> 
> However, the real piece I wanted was this caching advantage enabled using
> the following filter:
> 
> https://developers.google.com/speed/pagespeed/module/filter-cache-extend
> 
> I'm wondering if we could do somthing similar, either in the Apache config
> itself, or in perl pre apache.
> 
> My thought's were to append a version string within the templates (similar
> to Kyle's approach but within the filename as opposed to a query string) and
> then add some re-write rules in apache to remove the appended string again.
> (I got stuck at the apache re-write rules level.. I've never been especially
> good at them) I did wonder whether we should be appending a hash of the file
> content as opposed to a versions for our version string.. to enable the
> detection of customised changes, but this would obviously add to server load
> too.
Comment 6 Owen Leonard 2014-11-05 13:32:22 UTC
I set this bug to "In Discussion" when I submitted comment 3, so I would like to keep it "In Discussion" until that point is discussed. I think this is a worthwhile issue to pursue but I would like to hear more comments.
Comment 7 Mason James 2014-11-06 00:00:28 UTC
(In reply to Kyle M Hall from comment #5)
> I think the biggest issue with this approach is it's tied to using Apache,
> right? 

not quite...

mod_pagespeed has modules for both apache and ngnix
Comment 8 Martin Renvoize 2014-11-06 06:48:50 UTC
I wasn't actually suggesting the use of mod_pagespeed itself, but reimplementing one of its features as best we could.

In effect kyles patch does indeed do this, only it utilises a query parameter as a posed to appending to the filename itslef; which as Owen quite rightly suggests could mean that our 'static' resources end up never being chached on ocassions which would adversely affect performance.

Kyle, your right that my approach of using rewrite rules would be dependant upon some apache\nginx\whatever server specific rules being written.  We are doing this already though for some other cases I think so I don't see that as such a binding reason not to implement it in this way.

I'll see if I can find some time today to submit an alternative patch just to give people a clue of what I'm talking about.

As for upgrade paths.. I have no idea how the packages handle Apache config updates at current... But I imagine the shared config in /etc/koha to be where such rules should exist and as such believe its perfectly possible to update them.
Comment 9 Martin Renvoize 2014-11-06 07:34:07 UTC Comment hidden (obsolete)
Comment 10 Martin Renvoize 2014-11-06 07:34:50 UTC
Alternative patch including Apache rewrite rules attached.
Comment 11 Martin Renvoize 2014-11-06 07:38:12 UTC
Hold off on testing.. my RewriteRule needs some more work ;)
Comment 12 Martin Renvoize 2014-11-06 07:41:10 UTC
Scratch that.. it was just my gitified setup confusing me ;)..

Go forth and test ;)
Comment 13 Martin Renvoize 2014-11-06 07:59:35 UTC
Of course.. we'll need a followup for other static files too (at least css).

I'll get to work on that using my method.. and will include upping the cache time for .js and .css files as this patch basically means we're introducing a method for enforced replacement of caches when, and only when, we want them to get updated :)
Comment 14 Martin Renvoize 2014-11-06 08:35:31 UTC Comment hidden (obsolete)
Comment 15 Martin Renvoize 2014-11-06 08:56:55 UTC
Kyle, how did you choose which files to do this for.. it feels like there are instances of js files referred to which you've not accounted for?

I discovered this whilst attempting to find all css files fro a similar treatment.

I've used the following command to regex all css imports within file in the templates directory and note that many of my results also contained a JS import that hadn't had your treatment applied?

grep -r -l "<link rel=\"stylesheet\" type=\"text\/css\" href=\"" * | xargs sed -i 's/\.css"/_[% KOHA_VERSION %].css"/g'
Comment 16 Martin Renvoize 2014-11-06 09:00:12 UTC
Admittedly.. my regex catches a few file we don't wish to update.. but I'm now going through the file manually to remove those..
Comment 17 Martin Renvoize 2014-11-06 09:36:16 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2014-11-06 09:38:19 UTC Comment hidden (obsolete)
Comment 19 Martin Renvoize 2014-11-06 09:39:55 UTC Comment hidden (obsolete)
Comment 20 Martin Renvoize 2014-11-06 09:52:23 UTC
(In reply to Martin Renvoize from comment #15)
> Kyle, how did you choose which files to do this for.. it feels like there
> are instances of js files referred to which you've not accounted for?
> 
> I discovered this whilst attempting to find all css files fro a similar
> treatment.
> 
> I've used the following command to regex all css imports within file in the
> templates directory and note that many of my results also contained a JS
> import that hadn't had your treatment applied?
> 
> grep -r -l "<link rel=\"stylesheet\" type=\"text\/css\" href=\"" * | xargs
> sed -i 's/\.css"/_[% KOHA_VERSION %].css"/g'

Regarding this comment.. I'm even more sure we've missed a few places for the JS's..  We'll definitely need to handle these before applying my follow-up patch that increases JS cache time.
Comment 21 Martin Renvoize 2014-11-06 15:46:41 UTC Comment hidden (obsolete)
Comment 22 Martin Renvoize 2014-11-06 15:49:00 UTC
I'm pretty sure I've caught all css and js now.. lots of regex fun... not.
Comment 23 Martin Renvoize 2014-11-06 16:04:32 UTC Comment hidden (obsolete)
Comment 24 Martin Renvoize 2014-11-06 16:04:35 UTC Comment hidden (obsolete)
Comment 25 Martin Renvoize 2014-11-06 16:04:38 UTC Comment hidden (obsolete)
Comment 26 Martin Renvoize 2014-11-06 16:04:42 UTC Comment hidden (obsolete)
Comment 27 Martin Renvoize 2014-11-06 16:04:45 UTC Comment hidden (obsolete)
Comment 28 Martin Renvoize 2014-11-06 16:44:41 UTC Comment hidden (obsolete)
Comment 29 Martin Renvoize 2014-11-06 16:44:44 UTC Comment hidden (obsolete)
Comment 30 Martin Renvoize 2014-11-06 16:44:48 UTC Comment hidden (obsolete)
Comment 31 Martin Renvoize 2014-11-06 16:44:51 UTC Comment hidden (obsolete)
Comment 32 Martin Renvoize 2014-11-06 16:44:54 UTC Comment hidden (obsolete)
Comment 33 Martin Renvoize 2014-11-06 16:44:57 UTC Comment hidden (obsolete)
Comment 34 Martin Renvoize 2015-01-06 14:35:04 UTC
Hmm.. not sure why this got marked 'In Discussion', setting back to 'Needs Signoff'
Comment 35 Martin Renvoize 2015-01-06 14:37:02 UTC
Comment on attachment 31547 [details] [review]
[SIGNED-OFF] Bug 12904 - Force browser to load new javascript files after upgrade

Obsoleting the original patch to make applying the current series easier... I would still like to hear others thoughts on this alternative approach however.
Comment 36 Olli-Antti Kivilahti 2015-10-22 16:29:57 UTC
What a great idea!
Comment 37 Kyle M Hall 2015-10-26 13:55:48 UTC
Doesn't apply. I'd *really* like to sign off on this. Martin, can you rebase? Thanks!
Comment 38 Jonathan Druart 2016-03-07 11:56:05 UTC Comment hidden (obsolete)
Comment 39 Jonathan Druart 2016-03-07 11:56:08 UTC Comment hidden (obsolete)
Comment 40 Jonathan Druart 2016-03-07 11:56:14 UTC Comment hidden (obsolete)
Comment 41 Jonathan Druart 2016-03-07 11:56:17 UTC Comment hidden (obsolete)
Comment 42 Jonathan Druart 2016-03-07 11:56:22 UTC Comment hidden (obsolete)
Comment 43 Jonathan Druart 2016-03-07 11:56:26 UTC Comment hidden (obsolete)
Comment 44 Jonathan Druart 2016-03-07 11:56:31 UTC Comment hidden (obsolete)
Comment 45 Jonathan Druart 2016-03-07 11:57:04 UTC
Patches rebased (it was a nightmare...)
Comment 46 Jonathan Druart 2016-03-07 11:57:51 UTC
Created attachment 48742 [details]
script to update tt & inc files
Comment 47 Jonathan Druart 2016-03-07 11:58:10 UTC
(In reply to Jonathan Druart from comment #46)
> Created attachment 48742 [details]
> script to update tt & inc files

Usage: perl kv.pl **/*.tt **/*.inc
Comment 48 Mirko Tietgen 2016-03-24 21:14:15 UTC
I tried this, and I see the scripts having version numbers, but they are part of the file name and so they all throw 404s.
Comment 49 Jonathan Druart 2016-03-24 21:31:50 UTC
Could you confirm you have updated your apache config?
Comment 50 Mirko Tietgen 2016-03-24 22:27:16 UTC
Created attachment 49572 [details]
screenshot

Tried again with updated Apache config. No 404 anymore, but it does not seem like the files are reloaded. It looks like a redirect to me and the files used are the ones in the cache, see attached screenshot.
Comment 51 Kyle M Hall 2016-08-15 16:03:26 UTC Comment hidden (obsolete)
Comment 52 Kyle M Hall 2016-08-15 16:03:37 UTC Comment hidden (obsolete)
Comment 53 Kyle M Hall 2016-08-15 17:32:03 UTC
Created attachment 54488 [details] [review]
Bug 12904 - Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences
Comment 54 Kyle M Hall 2016-08-15 17:36:24 UTC
I'm at a loss. I just rebased and squashed/split all these patches for readability. Everything is working, but the browser continues to indicate it's grabbing cached files even when the filename has changed!

At first I thought it was because the rewrite was doing a 301 redirect, so I removed that, and still had the same thing. Next I tried a 307, but again, the same results ( though chrome decided not to cache some files at all with a 307 ).

Here is a video demonstration: http://screencast.com/t/JfFonhkWax

Martin, Jonathan, any ideas on what is going on?

Kyle
Comment 55 Kyle M Hall 2016-08-15 17:39:10 UTC
Created attachment 54489 [details] [review]
Bug 12904 - Update template css and js file references
Comment 56 Jonathan Druart 2016-08-16 07:47:14 UTC
(In reply to Kyle M Hall from comment #54)
> I'm at a loss. I just rebased and squashed/split all these patches for
> readability. Everything is working, but the browser continues to indicate
> it's grabbing cached files even when the filename has changed!
> 
> At first I thought it was because the rewrite was doing a 301 redirect, so I
> removed that, and still had the same thing. Next I tried a 307, but again,
> the same results ( though chrome decided not to cache some files at all with
> a 307 ).
> 
> Here is a video demonstration: http://screencast.com/t/JfFonhkWax
> 
> Martin, Jonathan, any ideas on what is going on?
> 
> Kyle

Using firebug (network tab) that sounds ok for me:
(reload mainpage)
mainpage_16.0600014.css 301 (moved permanentely)
mainpage.css 200

(click on mainpage)
nothing # from cached

(change version using the interface, so the page reload and will hit some .css and .js files, that's why we are focusing on mainpage.css)
(click on mainpage)
mainpage_16.0600014.css 301 (moved permanentely)
mainpage.css 200
Comment 57 Jonathan Druart 2016-09-14 13:35:29 UTC
Back to NSO...
Comment 58 Jonathan Druart 2016-12-20 17:19:38 UTC
Nobody to signoff on this one?
Comment 59 Kyle M Hall 2016-12-22 12:20:16 UTC
Created attachment 58378 [details] [review]
Bug 12904 - Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences
Comment 60 Kyle M Hall 2016-12-22 12:20:29 UTC
Created attachment 58379 [details] [review]
Bug 12904 - Update template css and js file references
Comment 61 Kyle M Hall 2016-12-22 15:42:44 UTC
Created attachment 58392 [details] [review]
Bug 12904 [QA Followup] - Don't send 301 status
Comment 62 Nick Clemens 2016-12-22 15:54:41 UTC
Created attachment 58393 [details] [review]
Bug 12904 - Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 63 Nick Clemens 2016-12-22 15:54:47 UTC
Created attachment 58394 [details] [review]
Bug 12904 - Update template css and js file references

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 64 Nick Clemens 2016-12-22 15:54:54 UTC
Created attachment 58395 [details] [review]
Bug 12904 [QA Followup] - Don't send 301 status

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 65 Jonathan Druart 2016-12-27 16:21:31 UTC
(In reply to Nick Clemens from comment #64)
> Created attachment 58395 [details] [review] [review]
> Bug 12904 [QA Followup] - Don't send 301 status
> 
> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Can you explain this patch?
Comment 66 Olli-Antti Kivilahti 2017-03-06 10:28:05 UTC
Created attachment 60826 [details] [review]
Bug 12904 - Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 67 Olli-Antti Kivilahti 2017-03-06 10:28:50 UTC
Created attachment 60827 [details] [review]
Bug 12904 - Update template css and js file references

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 68 Olli-Antti Kivilahti 2017-03-06 10:29:45 UTC
Created attachment 60830 [details] [review]
Bug 12904 [QA Followup] - Don't send 301 status

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
Comment 69 Olli-Antti Kivilahti 2017-03-06 10:33:44 UTC
Created attachment 60831 [details] [review]
Bug 12904 - Add mod_expire to etc/koha-httpd.conf

Has a positive effect on page loads for crazy dev-installs as well.
Comment 70 Olli-Antti Kivilahti 2017-03-06 10:38:43 UTC
Rebased to master.

(In reply to Jonathan Druart from comment #65)
> Can you explain this patch?

Yes.
Without this bad things will happen when caching is used.

Our page load times drop at best 2-4 seconds depending on the env we run Koha.
Better than plack :)


When the Koha-version changes, the urls for all static web-assets dynamically change to point at the new Koha-version.
This forces the browser to reload new versions of javascript-code.

You cannot really cache static web-content without a way of invalidating the user's cache, because then after version upgrades, some browsers would use old javascript code and bad things would happen.
Comment 71 Martin Renvoize 2017-04-28 15:23:15 UTC
I came here to QA this, then realised I wrote a bunch of the original code that was eventually rebased.

This is still a great thing to have and we've been using it in production in a few places so I'm happy it works.

I'll try to find a friendly QAer.
Comment 72 Olli-Antti Kivilahti 2017-05-24 13:11:42 UTC
(In reply to Martin Renvoize from comment #71)
> I came here to QA this, then realised I wrote a bunch of the original code
> that was eventually rebased.
> 
> This is still a great thing to have and we've been using it in production in
> a few places so I'm happy it works.
> 
> I'll try to find a friendly QAer.

Just QA this plix?
PM when you plan to do so.
This is a big moving target and it is hard to keep it applying to master. We have a rebased version from two-three weeks ago I guess. No point in wasting time applying it here because it will just get stale.
While doing that I can rebase our fork on top of master.
Comment 73 Kyle M Hall 2017-05-24 14:07:10 UTC
Created attachment 63692 [details] [review]
Bug 12904 - Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 74 Kyle M Hall 2017-05-24 14:07:30 UTC
Created attachment 63693 [details] [review]
Bug 12904 - Update template css and js file references

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 75 Kyle M Hall 2017-05-24 14:07:35 UTC
Created attachment 63694 [details] [review]
Bug 12904 [QA Followup] - Don't send 301 status

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 76 Kyle M Hall 2017-05-24 14:07:40 UTC
Created attachment 63695 [details] [review]
Bug 12904 - Add mod_expire to etc/koha-httpd.conf

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 77 Kyle M Hall 2017-05-24 14:07:46 UTC
Created attachment 63696 [details] [review]
Bug 12904 [QA Followup] - Look up the Koha version once

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 78 Katrin Fischer 2017-05-25 09:54:22 UTC

>For non-package installations, the following rewrite rules will need to be >added to the apache config file:
>RewriteRule ^(.*)_(.*).js$ $1.js [R=301,N,L]
>RewriteRule ^(.*)_(.*).css$ $1.css [R=301,N,L]

Wonder if this might not get lost - would there be a way to automate it? What happens if you don't?
Comment 79 Fridolin Somers 2017-05-31 10:44:08 UTC
> RewriteRule ^(.*)_(.*).js$ $1.js [R=301,N,L]
This rewrite rules are quite large, what about files that already contain an underscore ?
I think we should use a more explicit separator like "_kohav" with rewrite :
RewriteRule ^(.*)_kohav(.*).js.

> +<script type="text/javascript" src="[% interface %]/[% theme %]/js/datatables_[% KOHA_VERSION %].js"></script>
The syspref "Version" contains a dot (ie 16.1200037) meaning that files will have 2 dots, ie : 
../datatables_16.1200037.js
This may be a problem for some web-browsers like IE ?
Comment 80 Jonathan Druart 2018-01-23 13:30:24 UTC
Created attachment 70830 [details] [review]
Bug 12904: Force browser to load new javascript files after upgrade

A common issue with Koha seems to be that the web browsers continue to
use cached js files after an upgrade. If we append the version as a
parameter, we can force the browser to update the javascript files.

1) Apply this patch on a new branch based on master.
2) In a separate branch check out a previous version of Koha and go
   through the installation process on an empty database.
3) After logging into Koha, turn on Firebug's Net panel (or Chrome's
   Network panel) and turn on the filter for JavaScript files.
4) Clear the browser cache and navigate to a page, for instance
   Circulation. In Firebug the Net panel should show a status of "200
   OK" for all files, indicating that the file was downloaded, not
   loaded from the cache. In Chrome the "size" column will show a file
   size if the file was downloaded.
5) Navigate to another page and confirm that scripts are being loaded
   from the cache: Scripts like jquery.js and jquery-ui.js which are
   loaded on every page should have a status in the Firebug Net panel of
   "304 Not Modified." In Chrome the "size" columns should show "from
   cache."
6) Check out the branch you created for testing this patch.
7) In order to preserve the state of your browser cache for accurate
   testing, open a different browser from the one you were previously
   testing with, load Koha, and run through the database update.
8) Return to your original testing browser and navigate to another Koha
   page. Firebug's Net panel should show that script files are now being
   downloaded with a "koha_version" query string. These files should
   have a status of "200 OK" indicating that they are being downloaded,
   not loaded from the cache.
9) Navigate to another page and conifrm that the new versions of the
   scripts are being loaded from the cache.

To confirm that all affected JavaScripts links are working correctly,
test these pages by checking the Firebug Net panel for errors:

- Administration -> Currencies and exchange rates
  (jquery.dataTables.colvis.js)
- Tools -> Stage MARC records for import (ajaxfileupload.js) or another
  page which performs a file upload.
- Catalog -> Detail page -> MARC Preview (greybox JS)
- Help -> Any help page
- Any page which loads DataTables (Circulation in the staff client, for
  instance, and a title's detail page in the OPAC)
- Bonus points if you check the various conditional JS includes
  controlled by the many OPAC feature preferences

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 81 Jonathan Druart 2018-01-23 13:33:35 UTC
(In reply to Katrin Fischer from comment #78)
> 
> >For non-package installations, the following rewrite rules will need to be >added to the apache config file:
> >RewriteRule ^(.*)_(.*).js$ $1.js [R=301,N,L]
> >RewriteRule ^(.*)_(.*).css$ $1.css [R=301,N,L]
> 
> Wonder if this might not get lost - would there be a way to automate it?
> What happens if you don't?

We will have to highlight that in the release notes.
People will quickly now that something is wrong if all JS and CSS files return 404

(In reply to Fridolin SOMERS from comment #79)
> > RewriteRule ^(.*)_(.*).js$ $1.js [R=301,N,L]
> This rewrite rules are quite large, what about files that already contain an
> underscore ?
> I think we should use a more explicit separator like "_kohav" with rewrite :
> RewriteRule ^(.*)_kohav(.*).js.
> 
> > +<script type="text/javascript" src="[% interface %]/[% theme %]/js/datatables_[% KOHA_VERSION %].js"></script>
> The syspref "Version" contains a dot (ie 16.1200037) meaning that files will
> have 2 dots, ie : 
> ../datatables_16.1200037.js
> This may be a problem for some web-browsers like IE ?

I am going to push these patches soon. Developers will have time to catch the issues before 18.05.


If someone disagrees with these patches, tell us why.
Comment 82 Jonathan Druart 2018-01-23 13:44:34 UTC
Created attachment 70831 [details] [review]
Bug 12904: (QA follow-up) Don't send 301 status

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 83 Jonathan Druart 2018-01-23 13:44:48 UTC
Created attachment 70832 [details] [review]
Bug 12904: Add mod_expire to etc/koha-httpd.conf

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 84 Jonathan Druart 2018-01-23 13:44:58 UTC
Created attachment 70833 [details] [review]
Bug 12904: Force browser to load new javascript files after upgrade

This patch has been automatically generated using:
  perl kv.pl **/*.tt **/*.inc
Comment 85 Jonathan Druart 2018-01-23 13:45:08 UTC
Created attachment 70834 [details] [review]
Bug 12904: Pass KOHA_VERSION in C4::Auth
Comment 86 Jonathan Druart 2018-01-23 13:46:00 UTC
Created attachment 70835 [details]
script to update tt & inc files
Comment 87 Josef Moravec 2018-02-06 09:26:24 UTC
Looks good, just small issues:

the staff-global.css and reset-fonts-grids.css in intranet are not versioned

The staff-global.css needs to be adressed in file
koha-tmpl/intranet-tmpl/prog/en/includes/intranetstylesheet.inc

reset-fonts-grids.css is using through @import in staff-global.css, but could be leaved as is, it probably never change and I believe it will be removed in the future as it is part of yui library.

in opac the opac.css is needs to be addressed in file koha-tmpl/opac-tmpl/bootstrap/en/includes/doc-head-close.inc

script.js and enquiry.min.js needs to be addressed in file koha-tmpl/opac-tmpl/bootstrap/en/includes/opac-bottom.inc
Comment 88 Owen Leonard 2018-02-06 12:53:15 UTC
(In reply to Josef Moravec from comment #87)

> reset-fonts-grids.css is using through @import in staff-global.css, but
> could be leaved as is, it probably never change and I believe it will be
> removed in the future as it is part of yui library.

That is my goal.
Comment 89 Josef Moravec 2018-02-06 13:04:50 UTC
(In reply to Owen Leonard from comment #88)
> (In reply to Josef Moravec from comment #87)
> 
> > reset-fonts-grids.css is using through @import in staff-global.css, but
> > could be leaved as is, it probably never change and I believe it will be
> > removed in the future as it is part of yui library.
> 
> That is my goal.

Great!
Comment 90 Jonathan Druart 2018-02-06 13:41:46 UTC
Created attachment 71250 [details] [review]
Bug 12904: Fix occurrences not caught by script
Comment 91 Jonathan Druart 2018-02-06 13:42:24 UTC
Great catches Josef, thanks!
Comment 92 Josef Moravec 2018-02-06 13:58:22 UTC
Created attachment 71251 [details] [review]
Bug 12904: Apache configuration

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 93 Josef Moravec 2018-02-06 13:58:26 UTC
Created attachment 71252 [details] [review]
Bug 12904: Add mod_expire to etc/koha-httpd.conf

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 94 Josef Moravec 2018-02-06 13:58:31 UTC
Created attachment 71253 [details] [review]
Bug 12904: Pass KOHA_VERSION in C4::Auth

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 95 Josef Moravec 2018-02-06 13:58:36 UTC
Created attachment 71254 [details] [review]
Bug 12904: Force browser to load new javascript files after upgrade

This patch has been automatically generated using:
  perl kv.pl **/*.tt **/*.inc

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 96 Josef Moravec 2018-02-06 13:58:42 UTC
Created attachment 71255 [details] [review]
Bug 12904: Fix occurrences not caught by script

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 97 Josef Moravec 2018-02-06 14:00:54 UTC
(In reply to Jonathan Druart from comment #90)
> Created attachment 71250 [details] [review] [review]
> Bug 12904: Fix occurrences not caught by script

Thanks for quick reaction!

Passed QA. I just fixed the first patch, which was made as fix of apache configuration, which was part of original patch. Now should apply cleanly on current master.
Comment 98 Jonathan Druart 2018-02-08 19:00:45 UTC
1. Must misc4dev deal with apache *-git.conf files?

If so, how? I think we will have to ask devs to add these 2 lines to /etc/koha/apache-shared-*-git.conf

2. 
<Joubu>  20 RewriteRule ^(.*)_(.*).js$ $1.js [N,L].
<Joubu> [N,L]?
<tcohen> it looks like L shouldn't be there? or N?
<tcohen> L = Last
<tcohen> N = Next
<tcohen> using N is very tricky                                                                                                                                                                     
<tcohen> and need to have a perfect understanding of how all defined rules make sense together

Does someone know?
Comment 99 Jonathan Druart 2018-02-09 17:08:57 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 100 Jonathan Druart 2018-02-09 21:37:15 UTC
We should improve the rewrite rule, it fails for few files (for instance editor_plugin.js on admin/branches.pl?op=add_form&branchcode=CPL)

Could anyone take a look?
Comment 101 Katrin Fischer 2018-02-13 07:14:07 UTC
It seems to break other functionality too, like the modal on translating itemtype descriptions: Loading failed for the <script> with source “http://localhost:8081/intranet-tmpl/lib/greybox/gb_scripts.js”.

Should we file a separate bug? This is quite a blocker and hinders testing of other functionalities.
Comment 102 Jonathan Druart 2018-02-13 13:40:13 UTC
(In reply to Katrin Fischer from comment #101)
> It seems to break other functionality too, like the modal on translating
> itemtype descriptions: Loading failed for the <script> with source
> “http://localhost:8081/intranet-tmpl/lib/greybox/gb_scripts.js”.
> 
> Should we file a separate bug? This is quite a blocker and hinders testing
> of other functionalities.

I confirm this is fixed by bug 20187.
Comment 103 Marcel de Rooy 2018-02-13 13:44:20 UTC
(In reply to Jonathan Druart from comment #98)
> <Joubu> [N,L]?
> <tcohen> it looks like L shouldn't be there? or N?
> <tcohen> L = Last
> <tcohen> N = Next
> <tcohen> using N is very tricky                                             

N,L is wrong imo
N would give other rules a chance to make further rewrites
looks to me that normally L would be fine
Comment 104 Nick Clemens 2018-02-16 01:27:29 UTC
Awesome work all! Enhancement, not backported to stable.
Comment 105 Victor Grousset/tuxayo 2018-02-16 08:22:23 UTC
Thanks all, less cache cleaning for users! :D


(In reply to Nick Clemens from comment #104)
> [...] Enhancement, not backported to stable.

Doesn't this fix bugs actually? Anyway, this would be a pain to backport right?
Comment 106 Katrin Fischer 2018-02-16 08:43:03 UTC
As we've found quite a few issues with it so far and it requires changing your Apache configuration I think it's not a good candidate for backporting right now.
Comment 107 Victor Grousset/tuxayo 2018-02-16 10:28:53 UTC
Indeed, that's kind of a non-backward compatible change. So that would be a bad surprise for a minor upgrade.


(In reply to Katrin Fischer from comment #106)
> [...] it requires changing
> your Apache configuration

By the way, is this the right place to ask/post an updated Nginx configuration?
(or any other web server)
Comment 108 Victor Grousset/tuxayo 2018-02-16 10:30:32 UTC
Updated «Text to go in the release notes» with bug 20187
Comment 109 Katrin Fischer 2018-02-16 12:04:33 UTC
> By the way, is this the right place to ask/post an updated Nginx
> configuration?
> (or any other web server)

I think we don't officially support Nginx and not a lot of people read here apart from the Bug CC, so probably better on the mailing list or in IRC.
Comment 110 Jonathan Druart 2018-02-16 16:00:17 UTC
Created attachment 71822 [details] [review]
Bug 12904: Add KOHA_VERSION to few more .css
Comment 111 Jonathan Druart 2018-02-16 16:01:55 UTC
(In reply to Jonathan Druart from comment #110)
> Created attachment 71822 [details] [review] [review]
> Bug 12904: Add KOHA_VERSION to few more .css

Pushed to master.
Comment 112 Jonathan Druart 2018-02-16 21:27:55 UTC
QA tests added:
  commit 42ce839b1a61e5fef1836b4525bf942c6b53d878
    Bug 12904: Add [% KOHA_VERSION %] for .js and .css
Comment 113 Jonathan Druart 2018-02-19 15:28:05 UTC
Could anyone explain (again) why we choose to append the version to the filename and not in parameter?

So basically why:
  /my/file_[% KOHA_VERSION %].js
and not:
  /my/file.js?version=[%  KOHA_VERSION %]

Kyle and Martin talked about the two approaches earlier in comments (from comment 4) but I do not understand. Please clarify.

It could avoid the rewrite rules.
Comment 114 Marcel de Rooy 2018-02-20 13:16:36 UTC
(In reply to Jonathan Druart from comment #113)
> Could anyone explain (again) why we choose to append the version to the
> filename and not in parameter?
> 
> So basically why:
>   /my/file_[% KOHA_VERSION %].js
> and not:
>   /my/file.js?version=[%  KOHA_VERSION %]
> 
> Kyle and Martin talked about the two approaches earlier in comments (from
> comment 4) but I do not understand. Please clarify.
> 
> It could avoid the rewrite rules.

From one of the linked sources:
===
But which is better: mylogo.1.2.gif or mylogo.gif?v=1.2? To gain the benefit of caching by popular proxies, avoid revving with a querystring and instead rev the filename itself.
===
I am not sure if passing a version as parameter in a script URL would be good practice. It seems quite uncommon, but I may be wrong?
Comment 115 Kyle M Hall 2018-02-20 14:05:31 UTC
AFAIK, the reason the former is better then the latter is fairly simple. With the latter ( that is, /my/file.js?version=[%  KOHA_VERSION %] ), the file will *never* be cached. It will always be fetched on each page load. Since we would be passing a GET parameter, a modern web browser would assume the contents of the URL may vary based on the parameter, and will assume it shouldn't cache the file.

Kyle

(In reply to Marcel de Rooy from comment #114)
> (In reply to Jonathan Druart from comment #113)
> > Could anyone explain (again) why we choose to append the version to the
> > filename and not in parameter?
> > 
> > So basically why:
> >   /my/file_[% KOHA_VERSION %].js
> > and not:
> >   /my/file.js?version=[%  KOHA_VERSION %]
> > 
> > Kyle and Martin talked about the two approaches earlier in comments (from
> > comment 4) but I do not understand. Please clarify.
> > 
> > It could avoid the rewrite rules.
> 
> From one of the linked sources:
> ===
> But which is better: mylogo.1.2.gif or mylogo.gif?v=1.2? To gain the benefit
> of caching by popular proxies, avoid revving with a querystring and instead
> rev the filename itself.
> ===
> I am not sure if passing a version as parameter in a script URL would be
> good practice. It seems quite uncommon, but I may be wrong?
Comment 116 Jonathan Druart 2018-02-20 14:30:52 UTC
Ok, make sense! Thanks for the answers.
Comment 117 Francesco Rivetti 2018-02-21 15:17:08 UTC
query parameters don't necessarily bypass the cache.

open the network inspector of your browser and check http://b.oha.it/s/test.html

the cache is 10 seconds, so if you are fast enough you will see it fetched from the cache (tested on chrome a firefox on linux)