Currently, this module is adding the Javascript code via hook_footer(), because then the JS is at the bottom of the page for sure, which results in faster page load times. However, this is not needed when using the locally cached .js file. You can then simply do:
drupal_add_js('files/googleanalytics/urchin.js', 'module', 'header');
The major benefit is that this .js file will now also be aggregated! I noticed this while working on the CDN integration module, in which this file is the only .js file that was not being synchronized to and served from the CDN.
I will create a patch in a couple of minutes.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | drupal_add_js.patch | 3.3 KB | wim leers |
| #6 | drupal_add_js.patch | 3.28 KB | wim leers |
| #5 | drupal_add_js.patch | 3.27 KB | wim leers |
| #1 | drupal_add_js_for_locally_cached_js.patch | 2.64 KB | wim leers |
Comments
Comment #1
wim leersPreliminary patch. The crux is described in the issue. This will require an update of the
googleanalytics_cron()andgoogleanalytics_cache()functions, to add support for caching the new ga.js file.While I was at it, I fixed the coding style. I'm marking this as CNR because I'd like to get some feedback from hass. If he likes where this is going, I may update the aforementioned functions as well.
Comment #2
wim leersI just noticed the example code of this issue is the opposite of what I meant. This is the fixed one:
drupal_add_js('files/googleanalytics/urchin.js', 'module', 'footer');(footer instead of header)Comment #3
hass commentedhttp://drupal.org/node/200681 and fixed in D6
Comment #4
wim leersAny chance you could commit this to the Drupal 5 branch as well?
Comment #5
wim leersIt doesn't hurt to have the JS files in the footer. It increases performance. Arguments about "it may not be completed yet" are only valid if you're caching the file locally and your hosting is super slow. In any other case, it should load fast enough. And in all cases, the JS will be cached locally. So in the worst case, you're not tracking the first page load completely.
For people who'd like to use it in the header, we should provide an option in the UI. That's not yet included in this patch. This patch is solely for standardizing on drupal_add_js() and using 'footer' as the default scope.
This is critical to the page loading performance of a website.
Comment #6
wim leersReroll. drupal_add_js() doesn't play nice with absolute URLs (thus also not with external .js files).
Comment #7
wim leersAnother reroll. The local caching was broken. I didn't notice this right away, because cron isn't working properly on my own site atm.
The latest in the DRUPAL-5 branch, combined with this patch, is being used at wimleers.com. And it just works :) I'm using urchin.js btw.
Comment #8
hass commentedRe-roll the patch, please. I synced D6 with D5 today and fixed many bugs. Maybe this bug is already fixed now. I'd like your code optimizations... so take a look, please.
Comment #9
hass commentedhttp://drupal.org/node/182334