Hello,
As a practice for better performance, ads should be loaded only when everything else is rendered. This patch move GA_googleFillSlot() to just before closing tag. Performance is improve, because banners are loaded after other elements.
Two problems, still:
- No admin option yet, but there is a define('GAM_LAZY', true); at the beginning so that you can easily turn it on/off.
- Move other Javascript to the bottom. I don't have time to do it now, but just the fillslot that already makes a big performance gain (particularly when Google CDN is far from you).
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | gam-765802.patch | 3.03 KB | jcisio |
| #2 | gam-765802-3.patch | 2.96 KB | jcisio |
| gam-lazy.patch | 2.21 KB | jcisio |
Comments
Comment #1
toemaz commentedThanks for the patch. Just a note regarding: $(document).ready(function() ... It has been deprecated. Instead we should use Drupal.behaviors.
If you can update the patch, I will be glad to commit it.
Comment #2
jcisio commentedOK new patch to use Drupal.behaviors. I moved all scripts to the bottom, too.
Still, a few shortcomings:
- When DOM not ready, ads display at the bottom for a very short moment. I don't use a wrapper with display:none or move out of viewport because it seems that iframe will reload when it is turned on/attached to another element. My actual solution is move a displayed banner from the bottom to the correct position.
- Normally, drupal_add_js doesn't work on hook_footer. I don't know why it works here. Maybe move it to hook_init to be sure.
- Last but not least, with this patch, the default behavior is changed: script is inserted at the bottom by default. If it is good or not, I leave it for the maintainer to decide ;)
Comment #3
jcisio commentedJust a notice, there is incoherent usage of ga_ and gam_ prefixes in the patch. I prefer gam_ because that what Google is using, and ga_ is for "Google Adsense".
Comment #4
iamer commentedWhen I tried this patch, all the ads were clumped up together at the top of the page.
Comment #5
jcisio commentedI'm not sure, but it works well here http://www.thongtincongnghe.com/
I don't have time to test now, but maybe it's a css position problem in theme.
Comment #6
iamer commentedYou're probably right, I'll look at it and report back.
Comment #7
jcisio commentedIt seems that I've found the problem. If the ad divs is positionned by CSS, and the CSS has not completed loaded on domready, those positions are wrong and cause the problem.
Solution could be fix the position again after onload event.
Comment #8
jcisio commentedNew patch. Attach on onload event, too. Fixed problem with admin_menu, and may fix #4. Please test ;)
Comment #9
kmontyWhy have the
return ' ';line? Why not just do areturn;?Comment #10
jcisio commentedIf a block is empty, Drupal won't render it and we won't have the placeholder that we want.
Comment #11
Funkymoses commentedsubscribing; apologies for not having anything useful to say.
Comment #12
jcisio commentedAt least there is a patch to test. Changing the status.
Comment #13
Exploratus commentedsubscribe.... This is very important for page loadtime. ill test.
Comment #14
jcisio commentedYes please test and report. As I'm not sure if it works for most websites.
Comment #15
Exploratus commentedDoesnt seem to work. Dont know why. it moves the ads all around...
Comment #16
jcisio commentedProvide more info with any detail. If not I can't get it done.
Comment #17
Exploratus commentedThe ads appeared in the bottom left corner of my screen, the css settings I had setup for Google Ad Manager where no longer working when I used the patch.
Comment #18
jcisio commentedThat behavior means that there is some conflict with the other Javascript in the webpage. Did you see any error?
About the css, that's because you set some rule on code generated by GAM. I advise you set stylesheet only on the block container (the
<div class="block">...</div>), because its content is expected to have no stylesheet at all, with the size that is exactly the same as banner size.Comment #19
jcisio commentedTested in two sites with different themes without problem. Committed a modified patch for broader testing. This feature is disabled by default.
Reopen if have any problem.