[#9691] - Allow to add script/stylesheet version with JHtml
- Closed
- 31 Mar 2016
- Medium
- Build: staging
- # 9691
- Diff
- andrepereiradasilva:patch-8
- Failure continuous-integration/travis-ci/pr The Travis CI build failed Details
User tests: Successful: Unsuccessful:
Summary of Changes
This PR is for allowing to add script and stylesheet versioning with JHtml.
Testing Instructions
- Apply this patch in latest staging
- Add the following code to isis template index.php (in https://github.com/joomla/joomla-cms/blob/staging/administrator/templates/isis/index.php#L22)
// Script with no version
JHtml::_('script', 'system/jquery.Jcrop.min.js', false, true);
// Script with auto version
JHtml::_('script', 'system/jquery.Jcrop.min.js', false, true, false, true, true, null);
// Script with custom version
JHtml::_('script', 'system/jquery.Jcrop.min.js', false, true, false, true, true, 'custom-version');
// Stylesheet with no version
JHtml::_('stylesheet', 'system/jquery.Jcrop.min.css', array(), true);
// Stylesheet with auto version
JHtml::_('stylesheet', 'system/jquery.Jcrop.min.css', array(), true, false, true, true, null);
// Stylesheet with custom version
JHtml::_('stylesheet', 'system/jquery.Jcrop.min.css', array(), true, false, true, true, 'custom-version');- Now go to any page in the admin panel
- View the html source and check if this lines are in the html head section
<head>
[...]
<link rel="stylesheet" href="/media/system/css/jquery.Jcrop.min.css" type="text/css" />
<link rel="stylesheet" href="/media/system/css/jquery.Jcrop.min.css?<some-hash>" type="text/css" />
<link rel="stylesheet" href="/media/system/css/jquery.Jcrop.min.css?custom-version" type="text/css" />
[...]
<script src="/media/system/js/jquery.Jcrop.min.js" type="text/javascript"></script>
<script src="/media/system/js/jquery.Jcrop.min.js?<some-hash>" type="text/javascript"></script>
<script src="/media/system/js/jquery.Jcrop.min.js?custom-version" type="text/javascript"></script>
[...]
</head>| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Labels |
Added:
?
|
||
ok, will close this until i better understand how this whole thing works.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-03-31 22:43:51 |
| Closed_By | ⇒ | andrepereiradasilva |
It'd be better if there was only one way to add versions. So if you're going to add support for the JDocument way to JHtml, the pre-existing way needs to be deprecated. And as long as the pre-existing way exists, it needs to not cause issues with the new way. Just by looking at this, I can tell right now I can put an MD5SUM file in a directory and end up with something like media/jui/js/bootstrap.min.js?md5sumfilecontentsstring?jdocumentgeneratedstring.
i closed this one. i will investigate further and when i have a more complete solution i will make a PR.
thanks for the help
Conceptually this isn't bad. Well, aside from the fact that both methods have excessively long signatures so specifying a version is somewhat inconvenient. But it has to account for the current API's behavior too (even if I might be the only person who knows what that behavior actually is because almost nothing in JHtml is actually documented) and as is it'll break.
Deprecate MD5SUM if you're going to do this. Also, if you're going to do this, this needs to work in a way where both ways to add a query/version string don't collide.