Why does this require editing the template?

yeah, I mentioned this as well. The trick is the way the logic in this module works, see http://drupal.org/node/351214#comment-1712192 and my follow ups. #351214 is pretty much resolved, so we should deal with this issue here I think. feel free to keep digging :)

if i remember correctly, the google_add_js() function has dynamic output. as each block is rendered, it "adds" the necessary script output for each block, and ONLY if that particular block appears on that page. This is actually good because it only adds the scripts to pages that actually have google ads on them. The alternative is to just output ALL the scripts for EVERY block on EVERY page. This on the other hand might not be so bad, especially if you only have a few ad blocks, and the blocks show on most every page. Solutions welcome :)

As mentioned by frankcarey, I didn't use drupal_add_js to avoid that the necessary admanager scripts were called for each request. So purely for performance reasons. If someone thinks this is incorrect or can be done better, don't hesitate to share your thoughts.

Any idea if it is really slower. I'm using boost cache (which, BTW, works wonderfully with Admanager), so I don't care so much about a few milliseconds. Of course, it would be great to have theme modification as an option, for those who want to improve performance later. But I think you're currently making trying out this module harder than necessary.

I tend to agree. If the performance hit is causing additional js downloads, I might reconsider, but I don't think the calls really do anything bad other than making the size of the html slightly larger, right? And if the js to load these things can be at the end of the page, then there would be little lag i think. @toemaz are you open to trying it out where everything is loaded? might take some semi-serious reworking of the logic.

Yes, I'm open to try it out where everything is loaded. However, I can't work on this in the coming weeks, so if anyone think he can come up with a patch, knock yourself out.

Here's my patch which removes the need to alter your theme's template.php.

The patch makes use of drupal_add_js() and drupal_set_html_head() where possible. To avoid duplicate inclusion of the JS and/or avoid inclusion of JS when the blocks are not present, I made use of a static variable in google_admanager_add_js() to track whether the one-time Google Ad Manager specific JS is added as well as an optional parameter that toggles whether or not to add in the closing Google Ad Manager JS (GA_googleFetchAds()) which must be added after all the GA_googleAddSlot() calls for each ad slot block have already been added via drupal_add_js().

Implemented hook_preprocess_page() to add in this final 'closing' JS in GA_googleFetchAds()

Feedback welcome.

I just added #631262: Load Google JS in footer, not head using additional javascript. as alternative idea as well.

this patch makes sense to me, but i still have to test it.

looks like it's working fine (i still see my ads), but the google_service.js is loading before the css, which is bad.

Also, make sure you clear your cache after you patch!

We're using this line to add external js because drupal_add_js can't do it in 6, but it will in 7.

We can still use drupal_add_js to add external js with a little hack using document.write() and then we don't have to put it in head (and before all other css and js). I think we can also use google_admanager_js_add() to actually cache all the js to add and then just print it at the end to make it a little more neat, and have a little more control over the js placement, but here is the patch redo with just this in place..

OK, i rewrote this a little bit more elegantly than my last one, using a recursive google_admanager_add_js() that caches all of the js output in an array split into 4 sections that mirror the way google outputs their example scripts. [init, service, slot, and close]. I had at first just tried to output them all at once, but i guess they need to be in their own script tags. Output is handled with google_admanager_get_js() which combines all scripts for each section and outputs them to the page with drupal_add_js(). The new format should make it pretty easy for us to easily output this to the footer instead as a more speedy option (see comment #8).

deleted - drupal.org messing up

deleted - drupal.org messing up

deleted - drupal.org messing up

Frankcarey-

I'm still using patch 2 you provided and it is working fien on my site. However patch 3 throws me an error when trying to patch. Does it matter what release i'm using and if i've already use the 2nd patch?

Thanks.

@mustang You'll probably have to unpatch number 2, and then apply the 3 patch. Unpatch you can use the "-R" path option. If it doesn't work for you, you could just install a fresh copy and then add the patch there. let me know if there more more issues.

example to unpatch 2 and then patch in 3:

Thanks for the reply. just unpatched #2 and tried to apply #3 again.

patching file google_admanager.module
Hunk #1 FAILED at 100.
1 out of 1 hunk FAILED -- saving rejects to file google_admanager.module.rej

This was with the .dev version, then i installed the latest release and i still get the same error when i'm patching.

Idea?

Thanks!

you should attach your google_admanager.module.rej file

Here you go!

Any update to this?

I didn't found the time yet to reroll one of the attached patches since the 6.x branch has changed a lot already. If there is a ready to apply patch available, I'll commit it without hesitation.

patch still works fine for me. I added this patch on a new site to the latest dev version using the "patch" command.

note: i used the patch in #15

I just found a small bug in the logic of my patch. If you don't have an ad block enabled, then you'll get 4 errors from line 201. This now makes sure that google_admanager_add_js() doesn't return an empty $ga_js.

Small change keeping it marked as RTBC

frankcarey-

The patch you just posted up works for me with the latest dev release. However when I just put the ad_manager code for the ad block straight into my HTML(so it spits my ad block out anywhere i want it on my page, rather than using the block system), it will display the ad just one time. Then on a page refresh it goes away. If i clear cache on the performance page the ad will appear again, but only for one time...again on refresh it disappears. Anything im doing wrong here?

Thanks.

"However when I just put the ad_manager code for the ad block straight into my HTML(so it spits my ad block out anywhere i want it on my page, rather than using the block system)"

It sounds like the code is being cached. Can't you just make a new region in your theme instead, and place the block using the block admin?

I have everything on the performance page disabled.

I suppose I could make a new region..