[TYPO3-core] RFC #13998: Introduce Automatic versioning of CSS and JS files

Ernesto Baschny [cron IT] ernst at cron-it.de
Fri Apr 9 10:18:09 CEST 2010


Lars Houmark schrieb am 08.04.2010 05:24:
>> Please provide three sentences about this feature (including the
>> configuration option) of what it does. The committer will then take
>> care of updating NEWS.txt as well.

> As follows:
(...)
> Good enough? Feel free to correct / beautify / shorten / elaborate :)

You forgot to mention that this works only with apache's .htaccess
support and how to turn the setting on/off. Turning that switch "on"
without a proper rewrite rule will turn the backend unusable. So
mentioning that and also adding the exact line needed in .htaccess in
the NEWS.txt will make it more used.

Now to the patch:

Lars Houmark schrieb am 09.04.2010 01:37:

> Since RFC# 13693 changed behavior to use the pageRenderer in backend.php
> the patch no longer applies cleanly to the current trunk.
> 
> It does now in version 5, and at the same time I changed '&' to be
> '&' when adding the timestamp to an existing query-string that has
> parameters already.

1) I don't like the "&" as this assumes that the rest of the
parameters also have already been htmlspecialchared. Since this by
"convention" is meant to be done by the last instance just right before
generating a "html-tag", I prefer to keep the "&" and add some comment
in the functions header.

Also a comment about passing potential URL parameters to the function
(and not only the filename) is important, because otherwise people might
use it like this:

   createVersionNumberedFilename('filename.css').'?var=value'

which could generate filename.css?timestamp?var=value. The phpdocs are
very important to cover all aspects, as the API user might not read the
code.


2) For the backend, your patch breaks all Effects from scriptaculous.
Test the "clear cache" menu above: It wont open. This is because of this
line in the scriptaculous.js:

   var js = /scriptaculous\.js(\?.*)?$/;

So changing the filename to "scriptaculous.<timestamp>.js" will make it
unable to load the ?load=... modules (that is of course stupid, but it
is not our source-code).

So I suggest adding a parameter to the createVersionNumberedFilename to
force the use of query string for that specific purpose (scriptaculous.js).


3) Then there are some "if (TYPO3_MODE == 'BE') {" in the
class.t3lib_pagerenderer.php which I don't understand. So if we want to
allow this functionality to be used in the frontend (as the setting in
the install tool suggests), then this "if's" are in the way.

To allow it also in the FE I would propose to make the setting ternary:

- default (empty setting) is to skip versioning of files (backwards
compatible)
- "embed" means embedding version in filename
- "querystring" means appending version to filename


So attached is my patch modifying these points, adding some text to
NEWS.txt. Tests in backend and frontend worked as expected.

Please retest and re-review. Thanks!

Cheers,
Ernesto
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: 13998_v6.diff
URL: <http://lists.typo3.org/pipermail/typo3-team-core/attachments/20100409/d2e2a908/attachment-0001.txt>


More information about the TYPO3-team-core mailing list