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.

Comments

wim leers’s picture

Assigned: Unassigned » wim leers
Status: Active » Needs review
StatusFileSize
new2.64 KB

Preliminary patch. The crux is described in the issue. This will require an update of the googleanalytics_cron() and googleanalytics_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.

wim leers’s picture

I 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)

hass’s picture

Status: Needs review » Closed (duplicate)
wim leers’s picture

Status: Closed (duplicate) » Reviewed & tested by the community

Any chance you could commit this to the Drupal 5 branch as well?

wim leers’s picture

Title: drupal_add_js() should be used instead of manually adding it » [Performance] Use drupal_add_js(), use footer as scope
Status: Reviewed & tested by the community » Needs review
StatusFileSize
new3.27 KB

It 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.

wim leers’s picture

StatusFileSize
new3.28 KB

Reroll. drupal_add_js() doesn't play nice with absolute URLs (thus also not with external .js files).

wim leers’s picture

StatusFileSize
new3.3 KB

Another 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.

hass’s picture

Status: Needs review » Needs work

Re-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.

hass’s picture

Status: Needs work » Closed (duplicate)