CRM-20254: add cache buster string for custom CSS #9980
Conversation
…tring generation (duplicated from original PR which was to the wrong branch)
CRM/Core/Resources.php
Outdated
| * @return string | ||
| */ | ||
| public function addCacheCode($url) { | ||
| parse_str(parse_url($url, PHP_URL_QUERY), $queryParts); |
CRM/Core/Resources.php
Outdated
| return $url; // no need to update | ||
| } | ||
| else { | ||
| return str_replace('r=' . $existing, 'r=' . $latest, $url); |
There was a problem hiding this comment.
OK, so this leads to my main confusion in the PR: I don't understand the scenario where (a) the URL already has r=XYZ and (b) we need to change it. Do you have an example?
There was a problem hiding this comment.
@totten It's my unfamiliarity with the system that led me to putting that there to cover all cases. I thought maybe it might be possible that the cache buster string could change. If you can be sure it'll never happen I can rewrite it without it to just cover
- $url has a query string and we need to append cache buster with '&'
- $url has no query string and we need to append cache buster with '?'
Which should simplify things a lot
There was a problem hiding this comment.
Yeah, that rewrite/simplification sounds good.
The cache busting string does, of course, change from time-to-time, but I look at it this way:
- The signature for
Resources::getUrl()says that it takes an extension-name and file-name (not a URL fragment). The intended usage is this: someone callsaddScriptFile(...)oraddStyleFile(...), then it automatically composes the URL and outputs the markup. There's no opportunity in that flow for someone to append their own query-parameters or re-append the cache-code. - As a general matter, the code is and should be stable because we want caching to work. The cache code should only change in a few limited cases: (a) the sysadmin does some kind of system update (upgrade core; install a new extension) or (b) the sysadmin/developer explicitly flushes the caches.
There was a problem hiding this comment.
@totten it's certainly a lot simpler with that change!
I did a quick comparison on the performance of parse_url vs. str_pos which I mentioned on Stack Overflow, also in the hope that if I'm wrong about the check someone will point it out.
|
This works for me. It also improves test-coverage. :) |
…-custom-css CRM-20254: add cache buster string for custom CSS
As mentioned in Jira this adds the cache buster string for custom CSS files.
It also improves on the appending of the cache buster string to URLs by detecting for an existing query string or an existing cache buster string.
I originally opened a PR on the 4.6 branch, this is a replacement for that