Status

  • This has been fixed on 6.x-4.x-dev (which has no stable release at the time of writing), but will not be fixed on 6.x-3.x due to the risk of conflicts with other modules
  • #31 is the latest and many of us are using this in production since late 2011.
  • Google Webmaster Tools does complain that the tracking scripts should be placed in HEAD.
  • We were able to successfully move the scripts to head and still have knowledge of whether the page was a 404 or 403 by using the theme layer.
  • @hass wants additional dev testing before RTBC - please review!




Original Issue

Google recommends putting all the async tracking code in head. I realize we can't do that because there's no easy way in Drupal to put the tracking code in head AND know whether the page was 404 or 403. (I personally think tracking 404s/403s should be an option, but that's a different issue)

However, I don't see any reason not to put the call to ga.js in the head so it loads earlier. In my (admittedly limited) test, this shaved a few tenths of a second off the time it takes to register a page view for a page with a couple of ad calls and widgets on it. You can measure this yourself in Firefox or Chrome by seeing how long it takes to call __utm.gif -- that's how GA registers a page view.

Curious what you think. Let me know if there are any issues with the patch; it's my first time really working with git.

CommentFileSizeAuthor
#103 google_analytics-6.x-3.x-code-position-patched1.patch4.17 KBgnindl
#68 error.jpg203.07 KBfenda
#43 1023384-5.png266.96 KBseandunaway
#31 google_analytics-6.x-3.x-1023384-5.patch2.74 KBseandunaway
#29 google_analytics-6.x-3.x-1023384-3.patch2.25 KBseandunaway
#23 google_analytics-6.x-3.x-1023384-2.patch1.91 KBseandunaway
#20 google_analytics-6.x-3.x-1023384-1.patch1.6 KBseandunaway
#8 googleanalytics_6_gajs_header2.patch2.86 KBhass
#6 googleanalytics_6_gajs_header.patch2.86 KBhass
googleanalytics_6_gajs_head.patch2.18 KBeli

Comments

hass’s picture

hass commented
Status: Active » Closed (duplicate)

#921308: Document that module using asynchronous javascript but not able to correctly load code in header

eli’s picture

eli commented
Status: Closed (duplicate) » Active

Sorry, I've read the related issues, but I don't understand why splitting the code is bad. Could you give me a hint?

I see that you considered the idea already http://drupal.org/node/648284#comment-3025044 but I can't figure out why it wasn't committed.

hass’s picture

hass commented

I don't know if this works reliable. If there are no docs if splitting is allowed and possible you need to upgrade to D7.

hass’s picture

hass commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, googleanalytics_6_gajs_head.patch, failed testing.

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new googleanalytics_6_gajs_header.patch2.86 KB

Fixed patch attached.

Status: Needs review » Needs work

The last submitted patch, googleanalytics_6_gajs_header.patch, failed testing.

hass’s picture

hass commented
Status: Needs work » Needs review
StatusFileSize
new googleanalytics_6_gajs_header2.patch2.86 KB

Nitpicking test robot :-)

YK85’s picture

YK85 commented

Everything looks to be working great with the patch.
@hass - where you able to develop the feature any further? Or is this RTBC for its current state?

hass’s picture

hass commented

I have no plans to commit this patch to D6 soon as it is not documented anywhere by google if this is supported or not. Therefore - see this patch "as is" or upgrade to D7.

pjeutr’s picture

pjeutr commented

It is needed in the head by google webmaster central to verify website ownership through the Google Analytics tracking code, as stated in this page (yellow warning block).

https://www.google.com/support/webmasters/bin/answer.py?answer=185871&hl=en

EvanDonovan’s picture

EvanDonovan commented

Does http://code.google.com/apis/analytics/docs/tracking/asyncUsageGuide.html... answer this question?

hass’s picture

hass commented

Do you mean Split Snippet? It shows an example that do not help us...

EvanDonovan’s picture

EvanDonovan commented

The Split Snippet is not the same as what we are talking about here?

My apologies; it was hard for me to tell from the patch what was happening.

hass’s picture

hass commented

No, flip bottom/header...

seandunaway’s picture

seandunaway commented

Google Webmaster Tools just recently started complaining about my tracking code not being in also.

seandunaway’s picture

seandunaway commented

Is the only thing that requires changing...

from:

drupal_add_js($script, 'inline', 'footer');

to:

drupal_add_js($script, 'inline', 'header);

?

wiifm’s picture

wiifm
Primary language English
Location Wellington, NZ
commented

Is #17 correct? Is this all that is needed to be done? Was there any reason why the code is currently hard coded to be in the footer?

According to Google:

http://www.google.com/support/analytics/bin/answer.py?answer=74932

Once you find the code snippet, copy and paste it into your web page, just before the closing </head> tag*. If your website uses templates to generate pages, enter it just before the closing </head> tag in the file that contains the <head> section. (Most websites re-use one file for common content, so it's likely that you won't have to place the code snippet on every single page of your website.) 

*One of the main advantages of the asynchronous snippet is that you can position it at the top of the HTML document. This increases the likelihood that the tracking beacon will be sent before the user leaves the page. It is customary to place JavaScript code in the <head> section, and we recommend placing the snippet at the bottom of the <head> section for best performance.
Anonymous’s picture

Anonymous (not verified) commented

I get a white screen when I apply #17

seandunaway’s picture

seandunaway commented
StatusFileSize
new google_analytics-6.x-3.x-1023384-1.patch1.6 KB

Here is a patch which nixes hook_footer and instead uses hook_init to place the js in the header.

I did a quick run though but with so many options in google_analytics this will need some proper testing.

Status: Needs review » Needs work

The last submitted patch, google_analytics-6.x-3.x-1023384-1.patch, failed testing.

seandunaway’s picture

seandunaway commented

Hmm, seems drupal_get_headers() doesn't catch the 404 header until after hook_init().

seandunaway’s picture

seandunaway commented
StatusFileSize
new google_analytics-6.x-3.x-1023384-2.patch1.91 KB

Maybe we can move this to preprocess_page()?

Attached.

seandunaway’s picture

seandunaway commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, google_analytics-6.x-3.x-1023384-2.patch, failed testing.

seandunaway’s picture

seandunaway commented
Status: Needs work » Needs review

#23: google_analytics-6.x-3.x-1023384-2.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, google_analytics-6.x-3.x-1023384-2.patch, failed testing.

seandunaway’s picture

seandunaway commented

Hmm, so I think hook_init() is out because apparently that isn't run sometimes when serving cached content to anonymous users and also it's too early for 404 information in drupal_set_headers().

hook_preprocess_page() is out because apparently its too late in the execution order to use drupal_add_js().

Gonna try a custom preprocess function next.

seandunaway’s picture

seandunaway commented
StatusFileSize
new google_analytics-6.x-3.x-1023384-3.patch2.25 KB

Okay, this one should pass.

seandunaway’s picture

seandunaway commented
Status: Needs work » Needs review
seandunaway’s picture

seandunaway commented
StatusFileSize
new google_analytics-6.x-3.x-1023384-5.patch2.74 KB

Cleaned up and added comments.

izmeez’s picture

izmeez commented

subscribing

tfo’s picture

tfo commented

+1

fenda’s picture

fenda commented

subscribing

what's the roadmap on this? we need to load this in head!

seandunaway’s picture

seandunaway commented
Status: Needs review » Reviewed & tested by the community

I've been running it fine. I think it's ready for commit.

hass’s picture

hass commented
Status: Reviewed & tested by the community » Needs review

There is NO need for this patch. Roadmap is WON'T FIX for D6.

fenda’s picture

fenda commented

@hass - Why is this? Sorry I can't figure out the reason behind not upgrade. Google recommends putting the asynchronous code in the head for optimal tracking: http://www.google.com/support/analytics/bin/answer.py?answer=174090

hass’s picture

hass commented

There is no technical requirement to move code to head. It's a nice to have and it's where it is for technical reasons. The patch would break other modules integrating with ga. I will not risk compatibility for something that is nice to have. Upgrade to D7 if you need the code in head, please. Or use the patch at your own risk, but do not request for any support here.

seandunaway’s picture

seandunaway commented
Category: feature » bug

Google complains about the code not being in head. This is a bug per Google. I am not experiencing any incompatibility. Please tell me where and I will work to correctly integrate.

izmeez’s picture

izmeez commented

So, if i understand correctly,

the patch in #5 #31 is reportedly working,
there is concern that if this is committed it may cause conflicts with other modules,
therefore the recommendation is the patch should be used at your own risk.

Does anyone know of modules that have issue with this patch?

I can run some tests on the patch and report my findings.

Thanks.

seandunaway’s picture

seandunaway commented

izmeez, Try #31, its the latest.

izmeez’s picture

izmeez commented

Yes, I did mean the one in #31, that's the one i'm testing right now.
Patch applies fine to google-analytics-6.x-3.3

The script CDATA still appears at the bottom of the /body
And webmaster tools is failing to verify the site, indicating code needs to be in head.

Flushing caches does not help.

Am I missing something else?

seandunaway’s picture

seandunaway commented
StatusFileSize
new 1023384-5.png266.96 KB

It's moved to head for me... just tested.

Image attached.

seandunaway’s picture

seandunaway commented

By the way, I applied against 6.x-3.x (dev).

seandunaway’s picture

seandunaway commented

@hass, the latest patch is dramatically different then the one originally posted. It's no longer applying this in hook_footer or hook_init but using a preprocess in the theme layer.

I'm just trying to assess what you meant by break compatibility and now I think you might have been referring to the original copy/paste patch. Have you reviewed the latest?

Thank you for your consideration.

izmeez’s picture

izmeez commented

@rump Thanks. I updated to google_analytics-6.x-3.x-dev (2011-06-15), used the patch in #31 and ran update.php

Sure enough the script is moved to the head and google webmaster tools verifies the site without problems. So far there does not appear to be a conflict with other modules on the site and there are quite a few. I will see how it goes and report any problems.

seandunaway’s picture

seandunaway commented
Status: Needs review » Reviewed & tested by the community

Marking rtbc for hass's eyes. @hass, please see #45 and #31.

izmeez’s picture

izmeez commented

I probably should have changed the status to RTBC.

Another advantage of this patch appears to be that Google webmaster tools can then be used fully not just Google analytics.

hass’s picture

hass commented
Status: Reviewed & tested by the community » Needs review

@rump: NEVER set Your own patches RTBC. See Ubercart checkout.

hass’s picture

hass commented
Category: bug » feature

Nice to have is not a bug.

izmeez’s picture

izmeez commented
Status: Needs review » Reviewed & tested by the community

@hass With respect for all the work you have done on this very useful module.

I am changing the status to RTBC as I should have considered doing before.

Fortunately, for sites who wish to make use of this functionality this patch is available.

The larger question is one we have had before in another issue related to this module and that is: "In an open source community such as Drupal, how can individuals make their case to the module maintainer such that they can be heard and such that modules can be improved rather than individuals feeling that the maintainer is simply refusing to add functionality that is useful and possibly needed?

Maybe others can add comments here on the value of adding this "feature" to the 6.x-3.x branch.

Thanks,

hass’s picture

hass commented
Status: Reviewed & tested by the community » Needs review

I'm sorry, but I'm very sure You have not reviewed the code and I do not trust You. You are not a coder if I remember correctly. This code cannot work with the other modules. There is NO NEED for this patch. The module works flawless without this patch.

izmeez’s picture

izmeez commented

@hass Thanks, you are right I am not a coder. I can only vouch for testing the patch. Sorry, there is no status just for that, tested. Because of this at first I just provided my report and did not change the status. And I appreciated that @rump was asking you to review the patch.

I wish I had a better understanding of the code to be able to understand your comment

This code cannot work with the other modules.

. Hopefully, I will learn more and better understand.

seandunaway’s picture

seandunaway commented
Assigned: Unassigned » seandunaway
Category: feature » bug
Status: Needs review » Closed (won't fix)

I wouldn't say flawlessly considering you have a lot of people watching this thread wanting this functionality and google throws an error. This module is designed to work with Google Analytics and this is a bug report per Google Analytics documentation. Oh well.

seandunaway’s picture

seandunaway commented

I'm still hoping you would elaborate on why this code will not be compatible with other modules. I have this on two sites running fine. I'd love to address your concerns but will require more feedback then what you've repeatedly provided. I disagree with your assertion that this patch is not needed. The GA/Google Webmasters errors keep reminding me.

hass’s picture

hass commented
Category: bug » feature
Status: Closed (won't fix) » Needs review

Please keep this case open, at least to stop other from opening new cases.

@rump: other modules rely on hook footer too. They jump in and extend the code. As said you need to test other modules integrating with ga. With 70k installs we cannot change this easily. Previous ga docs said put into footer... some months ago they changed the docs but in past 3 years footer was ok.. Now they recommend the header, but again this does not break anything. It's annoying that webmaster tools are stupid... ~1 year they complained not beeing able to find the code, but docs still said it could be in the footer, too. I don't care that much as long as it works... The tracking works fine and in D7 it's in the header as recommended. In one year 90% will run D7...

seandunaway’s picture

seandunaway commented

What if this patch is re-rolled with an option on the settings page to move the ga code to head with a note, eg: may break modules which extend ga? The setting would default to the footer. Is it worth considering?

seandunaway’s picture

seandunaway commented

I guess another option for your consideration would be a new version branch if compatibility with other modules is a concern.

While I agree that the d7 installs are always increasing... Drupal experienced most of its growth during version 6, with an explosion of contrib and "big" sites/client now running on d6. I think d6 will be unique from other version of Drupal including d7, in that it will have extended life and usage.

I'm always surprised to hear from well known drupal developers (typically in #drupal-contribute) that they are beginning their next project on d6. Yes, even after d7 has been released for almost a year. I myself am beginning work on a new site and have chosen d6 for this particular project.

The gist: Please let's not forget about d6 just yet. However, I do understand the importance of serious testing and not leaving 70k+ installs high and dry.

izmeez’s picture

izmeez commented

+1 for the suggestion in comment #57 to add option to settings page.

fenda’s picture

fenda commented

+1 for the suggestion in comment #57 to add option to settings page.

By default we can stick with the current implementation and then we can also keep everyone in this issue happy by letting them decide if they want to move the code to the head.

@hass - would this be acceptable?

EvanDonovan’s picture

EvanDonovan commented

I think moving to the head via a setting on the settings page would be a good solution, as long as people were aware of the potential compatibility issues.

fenda’s picture

fenda commented

bump :(

simon.fryer’s picture

simon.fryer commented

Any update on this?

GA wont validate cross-domain tracking if it isn't in the head for me.

Verification failed for xx using the Google Analytics method (5 minutes ago). The Google Analytics tracking code on your site is in the wrong location. You can verify site ownership only with the asynchronous snippet placed in the section of your home page. Learn more.

Urgency appreciated as it's a live site that isn't being tracked properly. :(

fenda’s picture

fenda commented

I tried the patch in #31 but my site isn't working with Google webmaster tools.. Odd..

seandunaway’s picture

seandunaway commented

@drupaljoe, is the tracking script in the head? which checkout did you apply the patch against? i think @izmeez ran into a similar issue around #42 with the solution in #46. or, perhaps this needs a re-roll if there have been any changes to dev. please advise.

fenda’s picture

fenda commented

@rump: Yeah, the script is in the head and I just checked out 6.x-3.x and applied the patch again but still webmaster tools doesn't let me in.
This is the site, you can see the script in the head:http://www.cyac.jp/

seandunaway’s picture

seandunaway commented

Hmm, weird! Can you provide any details or screenshots with Webmaster Tools regarding any errors?

If it's just not verifying, does it verify properly if you paste the code into the template, just as a test?

fenda’s picture

fenda commented
StatusFileSize
new error.jpg203.07 KB

Screenshot attached for the error.

seandunaway’s picture

seandunaway commented

@drupaljoe, the main difference in your code that i'm seeing is you're caching a local copy of ga.js. can you try disabling that feature to use the script hosted by google while we troubleshoot?

additionally, see if removing the cross domain tracking resolves it.

the only difference after that is the cdata comment craziness that might be throwing off webmaster tools.

seandunaway’s picture

seandunaway commented

@simon.fryer re: #63,
are you having the same issue with webmaster tools not verifying when using cross domain tracking? did you apply #31 to head?

izmeez’s picture

izmeez commented

@rump Yes, my problems were resolved as described in comment #46

agoel@axelerant.com’s picture

agoel@axelerant.com commented

#31: google_analytics-6.x-3.x-1023384-5.patch queued for re-testing.

yingtho’s picture

yingtho commented

#20: google_analytics-6.x-3.x-1023384-1.patch queued for re-testing.

seandunaway’s picture

seandunaway commented

For those just in, #31 is the latest and still passes.

Alex Andrascu’s picture

Alex Andrascu commented

@hass...one can never stress enough but you mate...you are quite special. Is the patch going to break your own custom modules or why is it that you deny this harmless patch so badly ?

Alex Andrascu’s picture

Alex Andrascu commented

@rump can you please explain me why have you removed this check:

-  // 1. Check if the GA account number has a value.
-  // 2. Track page views based on visibility value.
-  // 3. Check if we should track the currently active user's role.
-  if (!empty($id) && _googleanalytics_visibility_pages() && _googleanalytics_visibility_user($user)) {

and why have you replaced

-function googleanalytics_init() {

with

+function googleanalytics_add_js(&$vars, $hook) {

Thanks

seandunaway’s picture

seandunaway commented

Hey Alex, please follow the thread as I documented each step that was necessary to get this to work properly, specifically around #28.

That check was in the code twice, and so now it is only required once. It is still there.

Also regarding the custom preprocess function... for one, it just makes more sense I believe, but more importantly:

Hmm, so I think hook_init() is out because ... ... ... it's too early for 404 information in drupal_set_headers().

Mainly, the 404 tests were breaking.

Thanks,

Alex Andrascu’s picture

Alex Andrascu commented

Hi rump,

Thanks alot for explaining. It makes perfect sense to me.

sammyframson’s picture

sammyframson commented

Thank you for your efforts on this @rump. I utilize a 3rd party online booking engine with my Drupal site and it has built in cross-domain tracking capability using GA. However, the booking engine will not load on the page unless the GA script loads first - so the OBE wouldn't load on any page which rendered my booking engine un-usable without your patch. Moving the script to the head has been very helpful and useful and it was certainly required for my implementation.

Thx.

tribe_of_dan’s picture

tribe_of_dan commented

+1 for the suggestion in comment #57 to add option to settings page.

A list of which modules may be incompatible with this feature would also be desirable if this is added in. If its only Ubercart, then I would happily use this feature as I'm not using that module.

rwohleb’s picture

rwohleb
he/him
Primary language English
commented

Ugh, coming back to a D6 project from a D7 project, I'm really missing the scope option. I could have sworn it used to be a D6 option back in the day, but maybe I'm on crack. There is some commented code in googleanalytics.admin.inc for setting the 'googleanalytics_js_scope' variable (as of 6.x-3.3), so...?

+1 for giving people the option in the 'advanced' section with the caveat that if 'you break it you buy it' in other modules.

rjbrown99’s picture

rjbrown99 commented
Status: Needs review » Needs work

Seems like additional effort is required with this to add settings. I just started following, but I'm going to be bold and rotate the status back to needs work.

And count me in as another +1 to add it to the status page with a compatibility warning to scare away folks who may not have the coding skills to debug if/when it breaks something elsewhere.

seandunaway’s picture

seandunaway commented
Status: Needs work » Needs review

rjbrown99, we're waiting for word from @hass if he will even consider the ui config options. as of right now, i don't think he'll consider this in any form or fashion. he did say he wants to keep this open to catch new comers. i don't think we should put "work" into it until the patch, issue, compatibility, gets some direction from him.

compatibility warning? i think we should first find the incompatibilities. :) it seems several of us are running this patch in production, without issues. i'd like to keep this as need review in hope of even more people (developers) incorporate this patch and show there aren't any issues (or fix them) and mark as RTBC.

mikew3’s picture

mikew3 commented

If you put the tracking code in the footer, then some analytics features won't work, such as cross domain linking.

For example, see the yellow box here:
http://support.google.com/googleanalytics/bin/answer.py?hl=en&answer=55532

If your pages include a call to _trackPageview(), _link(), _trackTrans(), or _linkByPost(), your Analytics tracking code must be placed in your HTML code above any of these calls.

So it seems pretty important that we have the code in the HEAD block. Would it be better to just paste the script into the theme template and avoid using this module altogether?

(I sent hass an email but I wanted to post here too)

betz’s picture

betz commented

any news on this?

seandunaway’s picture

seandunaway commented

@betz,

Latest is #31 which apparently a few of us are using successfully in production.

@hass would still like for it to have proper review and his concern seems to be about possible conflicts with other modules? I'm not sure which he means.

betz’s picture

betz commented

yes, i am also running it on production, but i want this to move on, as i dont want to patch forever.
On drupal 5 and 7 you can choose to put this in the head, why this problem for 6?

budda’s picture

budda
Primary language English
Location Cheshire
commented
Assigned: seandunaway » budda
Status: Needs review » Fixed

I've tested the #31 patch on a clean D6 install and confirmed the jscript is appearing in the right place.

With so many people having this already in production for several months it can be considered stable enough for inclusion in the dev branch for now.

I attempted to search out some Drupal modules which would be affected by this change, according to the earlier comments, but haven't found anything obvious. Open a new issue if one is found.

hass’s picture

hass commented
Status: Fixed » Needs work

Rollback required

hass’s picture

hass commented
Status: Needs work » Needs review

@budda: Don't commit this patch to 3.x branch, please. The "testers" have mostly; if not all - confirmed they are end users only and not developers. This case has a status of "won't fix" for compatibility reasons with other modules and is only kept open to *stop* others from creating duplicates.

This patch adds several troubles and breaks existing features as I know. It has never been tested with Ubercart and I'm not sure about tracking of 403/404 pages. This patch will also break all customization's that people made by using hook_footer() to customize the tracking code. Additional to this issue it is not required at all to keep the tracking up an running properly. The code was correct in past and is still correct. Google cannot deprecate their API until D6 goes end of live. This patch is only a nice to have and all who like to patch can do this at own risk, but it's not ok to risk and support the other 120.000 D6 installations just for a few people who think they must follow a nit-picking dumb Google validation result from today that passed in past. At the time when you left the project the installation base was 1-2% of today. The impact of bugs is therefore extreme and cannot compared to the many complains from past. I had several complains when I marked 3.x as default as the integration's have all been broken. This is a TOP10 module, don't risk it's reputation, please.

I'm open minded to get #231451: Add hook to alter data before sending it to browser done to make hook_footer() integration clean. Until this task is completed and this patch does not work as before this is a no-go for this heavy used project. We can consider creating a new 4.x version branch with this patch, but only if Ubercart and others can integrate and are fully tested and documented. I had no time to review and/or test the last patch and it is much more important to work on the critical issues in this queue than these useless ones.

hass’s picture

hass commented
Issue summary: View changes

Updated issue summary.

budda’s picture

budda
Primary language English
Location Cheshire
commented

@hass your lack of response to the questions about what other modules are to be affected, and no other information as to what you personally know could be broken hasn't been transparent at all. I couldn't find any evidence of modules which would be affected by this change from searching either.

So the change was committed to the 3.x dev, maintainers of other modules have a chance to resolve any integration hacks they might be going to the js code.
The same sort of knock on effects happened with modules like Webform which suddenly changed things and broken 3rd party modules assuming how it works.

Please list examples of the modules which would also need modifying to be compatible with the change to head for ga.js rather than wasting your time repeating the same claim.

This is not a useless issue either. Aside from the posters in this issue we've got clients who have temporarily removed the GA drupal module in favour of hard coding the GA script snippet in to their page tpl to move it to the head to ensure that pages are tracked properly even if the full page assets are never loaded and the visitor clicks back. Ensuring bounce rate is correctly tracked, and improving Googles scoring of pages for clients which are more obsessed about SEO on-page performance and loading speeds.

Maybe a 4.x branch is the solution then to avoid problems with legacy modules hacking in to code.

sebsebseb123’s picture

sebsebseb123 commented

Just so people know... the dev version WAS updated so that the GA code would appear in
Thanks @budda
:)

BUT, the very next day, the dev code was reverted back to putting the GA code at the bottom :(
Sorry @hass but, I'm also having trouble understanding why you're so reluctant to let this happen.
Certain features of GA can only be used with the Async GA code in the

Anyway, the #31 patch no longer works with the latest dev. So, for anyone trying to get this to work... the best option is to get a git clone of it. (Instructions here: http://drupal.org/node/49388/git-instructions/6.x-3.x). Then do a "git log" to see when the last time issue #1023384 was fixed... and do a "git reset --hard #hash". Currently the hash is: 45891c

Hope this helps.

izmeez’s picture

izmeez commented

+1 for a 4.x branch to resolve this impasse

hass’s picture

hass commented
Version: 6.x-3.x-dev » 6.x-4.x-dev
hass’s picture

hass commented
Status: Needs review » Fixed
izmeez’s picture

izmeez commented

@hass Thank you for creating the 6.x-4.x-dev branch and the release which can be found at http://drupal.org/node/49388/release and for the comments with the release as to what may be broken.

In consideration of comment #92 I am wondering if the status should be "needs review"? I will do my own testing and provide comments.

Thanks,

Izzy

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Alex Andrascu’s picture

Alex Andrascu commented

@hass " The "testers" have mostly; if not all - confirmed they are end users only and not developers "

I think you might want to speak for yourself on this kind of matters. I haven't confirmed anything with you.

I've been testing it with Ubercart and it doesn't throw any issues.

Thanks.
Alex

hass’s picture

hass commented

Google fixed their validator for footer and have broken the head validator. #1788586: Google analytics Javascript is not always added at the very end of the head section. So you guys can go back to 6.x-3.x and we can discontinue 4.x.

tribe_of_dan’s picture

tribe_of_dan commented

Thank God for that. Thanks hass.

seandunaway’s picture

seandunaway commented
Status: Closed (fixed) » Fixed

@hass,

If 4.x is discontinued and the only option is to have the ga tracking code in the footer, doesn't that mean that we can't track events and use many of the critical ga features?

hass’s picture

hass commented
Status: Fixed » Closed (fixed)

Nope

gnindl’s picture

gnindl commented
Version: 6.x-4.x-dev » 6.x-3.3
Status: Closed (fixed) » Needs review
StatusFileSize
new google_analytics-6.x-3.x-code-position-patched1.patch4.17 KB

I don't know if it's still an issue, but I created a patch for 3.3. It lets you configure the position of the GA code: either in the header or footer.

Status: Needs review » Needs work

The last submitted patch, google_analytics-6.x-3.x-code-position-patched1.patch, failed testing.

hass’s picture

hass commented
Version: 6.x-3.3 » 6.x-4.x-dev
Status: Needs work » Closed (fixed)
gnuget’s picture

gnuget
Primary language Spanish
commented
Version: 6.x-4.x-dev » 6.x-3.x-dev
Status: Closed (fixed) » Active

This issue is marked as fixed but here:

http://drupalcode.org/project/google_analytics.git/commit/253e4572928f12...

Seems to this commit was reverted. So this is still a bug.

gnuget’s picture

gnuget
Primary language Spanish
commented

Ahh, seems my mistake, in the branch 6.x-4-dev this bug is fixed but in hte 6.x-3.x the commit was reverted.

After to do a diff, seems to the unique diference between this two branches is this particular patch.

diff -x .git  -r google_analytics-4 google_analytics-3.x
diff -x .git -r google_analytics-4/googleanalytics.module google_analytics-3.x/googleanalytics.module
39,47d38
<  * Implementation of hook_theme_registry_alter().
<  */
< function googleanalytics_theme_registry_alter(&$registry) {
<   // Add custom preprocess function to the beginning of the stack.
<   // This allows use of the drupal_add_js() function before $vars is populated.
<   array_unshift($registry['page']['preprocess functions'], 'googleanalytics_add_js');
< }
< 
< /**
72c63
<  * Adds Google Analytics tracking scripts.
---
>  * Implementation of hook_init().
74c65
< function googleanalytics_add_js(&$vars, $hook) {
---
> function googleanalytics_init() {
104a96,110
>   }
> }
> 
> /**
>  * Implementation of hook_footer() to insert JavaScript at the end of the page.
>  */
> function googleanalytics_footer($main = 0) {
>   global $user;
> 
>   $id = variable_get('googleanalytics_account', '');
> 
>   // 1. Check if the GA account number has a value.
>   // 2. Track page views based on visibility value.
>   // 3. Check if we should track the currently active user's role.
>   if (!empty($id) && _googleanalytics_visibility_pages() && _googleanalytics_visibility_user($user)) {
237c243
<     drupal_add_js($script, 'inline', 'header');
---
>     drupal_add_js($script, 'inline', 'footer');
460c466
<   // Cache visibility setting in googleanalytics_add_js.
---
>   // Cache visibility setting in hook_init for hook_footer.

Who is the good one? i should use the branch 6.x-4? the 6.x-4 will become the new branch where all the work for this version will be added?

hass’s picture

hass commented
Version: 6.x-3.x-dev » 6.x-4.x-dev
Status: Active » Closed (fixed)
tribe_of_dan’s picture

tribe_of_dan commented

I think what Haas was trying to say was... "6.x-4-dev is no longer needed."

tribe_of_dan’s picture

tribe_of_dan commented
Issue summary: View changes

Added fixed message.

ianthomas_uk’s picture

ianthomas_uk
he/him
Primary language English
Location Brighton, UK
commented
Issue summary: View changes

Removing "fixed" note from issue summary, as the fix was reverted the following day

UMG’s picture

UMG commented

I have recently been trouble shooting some GA issues on my site, and a consultant pointed out that the code is not the head. I've read this thread, and I'm genuinely curious as to why so many think this is not an important distinction, since from what I've read, it does improve tracking.

What is the downside to putting the code in the head, as recommended?

escoles’s picture

escoles commented

The code has traditionally been put near the bottom of the page to improve loading performance.

It should now be a moot point, as Google Analytics tracking will now not work reliably and completely if the code's not in the head container.

escoles’s picture

escoles commented
Issue summary: View changes

Add note about 6.x-4.x branch and that this won't be fixed on 6.x-3.x

hass’s picture

hass commented

This commit introduced bug #2335391: Chameleon theme fails to add JS code to page.