Update for Drupal 6.x need testing

It would be great if you would port it.
It is one of the most important modules I use on my sites.

Patch via DRUPAL-5, able to work with Google Adsense (http://drupal.org/project/adsense, http://drupal.org/node/211584). Include coding style clean up and update with coder. Please test with complete uninstall and reinstall.

Bug fix update: fix node preview, and so preview will show up correctly.

I couldn't get this to work... I've tried two ways: patching adsense injector v5 with patch 5.01, then with 5.02 and patching adsense injector v5 straight to 5.02. The patching gives errors. Can you help?

All,

(I apologize for the delay in responding to these -- I haven't been receiving email notifications on my issues, for some reason.)

Thanks for helping with this. I can't help with this right now, because I haven't had time to set up a Drupal 6 installation. I'll be happy to commit this under an appropriate tag as soon as we think it's working, even thought I can't test it.

So, if someone decides it's working, set the status to "Reviewed & tested by the community" and I'll work on committing it.

Just starting to play with Drupal 6 - since this module makes use of the Adsense module, I'd like to know what version of the Adsense module is compatible with D6.

@inactivist: http://drupal.org/node/265666

subs

subscribing

any news?

Subscribe

subscribe

Status? I can test if that helps...

inactivist, please commit a 6.x-x.x-dev version. This will make it easier for those individuals that don't have the technical expertise or desire to apply patches and will likely increase the number of testers for the Drupal 6 version.

@TallDavid: I will commit a 6.x dev branch when I have time to do so. I appreciate everyone's patience.

sub

excellent news!

/me orders a beer for inactivist ;)

still waiting

I've created a DRUPAL-6--1 branch from the most recent D5 release code for AdSense Injector. The most recent patch does not work against the current code; please re-roll your last modifications against the DRUPAL-6--1 branch, resubmit it under a new issue, and I'll commit it to the D6 branch.

I'm closing this issue; please create new port issues against the DRUPAL-6--1 branch.

Edit: added reason for requesting new patch

Updating branch/version

When will this be released as a link download or a dev link download for 6.x?

As soon as someone re-rolls the patch for the Drupal-6 branch. Sadly, that won't be me -- I hope that the patch originator can help out. Otherwise, anyone else who wants to lend a hand.

The most recent patches don't apply successfully, so the 6.x-1.x-dev branch is nothing more than a copy of the most recent Drupal 5 release code.

This is important. It prevents me from going from d 5.13 to 6.

I'm a very good PHP learner, if someone can teach me the basic differences between Drupal 6 modules and drupal 5 modules, I'll port it easily. But I would need a guide of every single possible difference (I can't find one on this site).

Latest patch for DRUPAL-6--1. Update include:

• Renew hook_menu(). D6 coming with new implementation.
• watchdog() no longer require for t().
• hook_nodeapi() coming with new implementation for D6.
• Split admin related hook_settings() to adsense_injector.admin.inc, as D6 style.
• Retouch adsense_injector.info so group this module with Adsense.
• Better coding style.
• Better comment.
• Code validate with D6 coder module.
• Completely tested with adsense 6.x-1.0-beta3.

IMHO, if this module is not actively maintained, we should remove the paypal banner from admin setting page :S

@hswong3i:

Thanks for the patch re-roll. I'll try it out as soon as possible. I'm giving you CVS access. Feel free to commit the patches, I'll create a release for it. Please tread lightly.

IMHO, if this module is not actively maintained, we should remove the paypal banner from admin setting page :S

Since you mentioned it, yes, we should remove the donation link. I appreciate those who have supported the module over the years.

Let me know if I can help with anything.

@inactivist: thanks for your kindly action. I will tidy up existing code and also commit this patch for CVS, so let's review it afterward :D

Another suggestion: may we activate D5 and D6 -dev version? So before stable release we can test it in advance. On the other hand, D47 should now be obsoleted based on usage statistics, we may disable it, too.

Update: Seems I don't have right to commit file:

cvs commit: Examining .
** Access denied: hswong3i does not have permission to commit files in:
** contributions/modules/adsense_injector
** Please contact the owner of this project and request to be added
** as a CVS maintainer, see http://drupal.org/node/63634.
cvs commit: Pre-commit check failed
cvs [commit aborted]: correct above errors first!
cvs commit: saving log message in /tmp/cvsbGkgQ9

Any idea?

Please try again; it turns out that I had not pressed the proper button to commit the change. Sorry about that.

Hopefully I am not doing too much for this project. A new RC version is now available for D6, where D5 is also update with coding style cleanup and sync programming logic with D6. Some documentation is now tidy up, too. Please feel free to report if there exist any bugs.

Thanks inactivist, thanks for giving chance for contribution. I love this module very much and hope everyone would like this contribution :D

@hswong3i:

Hopefully I am not doing too much for this project. A new RC version is now available for D6, where D5 is also update with coding style cleanup and sync programming logic with D6. Some documentation is now tidy up, too. Please feel free to report if there exist any bugs.

Thanks inactivist, thanks for giving chance for contribution. I love this module very much and hope everyone would like this contribution :D

No, you can't do too much! Thanks for helping.

I won't be able to review hswong3i's work until this weekend at the earliest. So, we need the community to jump in and show just how much this module means. Please check out hswong3i's work and provide feedback ASAP. Generate new issues as appropriate!

Just installed it at 2 website and both seem to be working perfectly.

http://www.PROJEX.com and http://www.DRUMSTHEWORD.com

/me grins like a cat thats got the cream!

Good job fellas!

I've written some instructions here http://www.projex.com/configure-drupal-6-google-adsense-and-injector-module

Thanks to hswong3i... he spent the time to get this up and running. I'll try this out on a test installation this weekend.

You are a star!

@inactivist: Sorry for over-reaction... Since this module seems like a raw diamonds, and as been waiting for almost 8 months, I suddenly get out of control... We should have more review and discussion before stable release. I am too careless and please forgive me :S

Here are the cvs diff for simpler review, between: 1. 5.x HEAD vs. 5.x-2.5-1, and 2. 5.x HEAD vs. 6.x HEAD. Both diff files are obtain with cvs diff -uRpNwbB so we can just focus on string changes but no space changes. I diff 5.x with 6.x as they are now sync in programming logic and coding style, so this can show out the major change of programming logic for version upgrade from D5 to D6. Most likely, additional changes are belongs to documentation :D

P.S. The different between 5.x and 6.x should be very tiny. The diff file is big just because hook_settings() is now split into individual file. The function itself have no different between 5.x and 6.x.

@hswong3i:

@inactivist: Sorry for over-reaction... Since this module seems like a raw diamonds, and as been waiting for almost 8 months, I suddenly get out of control... We should have more review and discussion before stable release. I am too careless and please forgive me :S

As over-reactions go, that wasn't too bad. :D

The diffs look reasonable.

Please re-insert my copyright notices. Exodus Development retains copyright even though they are licensed under the GPL. So, each php module should contain the following line at or near the top:

/* Copyright (c) 2006 - 2008 Exodus Development, Inc. All Rights Reserved. http://exodusdev.com */

Other than that, I think things look OK. Anyone else have any comments? C'mon, everyone! You all are anxious for this module, so jump in and help out! :D

Minor suggestion: when checking adsense module, they insert their own copyright information in front of README.txt as below, and left LICENSE.txt with GPLv2:

Should we clone this style too? It looks elegant :D

Patch for review.

fwiw - I agree. This style looks better.

*AND* I got a problem. When I'm logged into my website I see the ADSENSE marker boxes but when I log out the ads vanish.

This is the generated code when I'm logged out:
!-- google_ad_section_start --

Am I doing something stupid?

@finkpad, @hswong3i:

Go for it. Let's wrap it up. Roll a new release when you have time. Thanks again.

Get it :D

I will double check if still something miss, and roll a new release ASAP. Maybe just RC2 so we have more time for review before stable release?

There are more cleanup and bugfix within latest within 6.x-2.6-rc2. Please check the CHANGELOG.txt for more information.

We may need more test and review before stable release. Please feel free to comment if possible :D

hswong3i - can you confirm that you see the ads when anonymous and what website is it on.

When I am logged in (I see the ad placeholders anyway) but NOTHING when I am logged out ie: an anonymous surfer. I'm possibly suffering from adsense blindness from playing around with the settings too much. If I can see it working on another Drupal6 website that will help.

Feel free to register and test at www.PROJEX.com

Cheers.

Drupal 6.8
adsense module 6.x-1.0-beta3
Adsense Injector 6.x-2.6-rc2

@Finkpad: They are now working within my blog: http://edin.no-ip.com/. I am also using adsense.module 6.x-1.0-beta3 + adsense_injector.module 6.x-2.6-rc2. Just provide you some demo setup as reference:

Activate modules: AdSense core, Managed ads and AdSense Injector
Site Google AdSense Publisher ID: pub-0123456789012345
Body view template: <div style="float: right; margin: 0; padding: 0 1em .25em 0;">[adsense:250x250:0123456789]</div>%body<br class="clear"/>[adsense:728x90:0123456789]
List view template: %teaser<br class="clear"/>[adsense:728x90:0123456789]
Active required content type

P.S. Google adsense now coming with new "slot" setup. You may need to update your google adsense account setting :D

OK - but I dont see any ads on YOUR blogs either....

Have you tried LOGGING OUT and looking at your website? Do the ADSENSE placeholders only apear when you are logged in? I just registered on http://edin.no-ip.com/ and still dont see any adsense.

I swear I am going maaaaaddddddd

/me shakes head

Finkpad: ...... A silly Qs: are you sure your browser is supporting JS? E.g. As I have install FF3 web developer plugin, I can simply disable all JS from the tool bar. I have check my site with 3 different computer and both IE/FF, it is functioning :D

Moreover, may you try to use adsense's helper code directly? E.g. create a new node + php fiflter + call adsense_display() manually. If this also display none of ads, means your adsense module is not configure correctly :S

P.S. Remember to follow up this issue if you can/can't solve this problem, so others can take this as reference ;)

I think it is almost time for a stable release. Any suggestion?

The stable 6.x-2.7 is now released. This issue will now marked as fixed :D

@hswongi:

Thanks for the help! And thanks to those of you who provided useful feedback.

Happy new year!

With the advent of this latest rollout my adsense is displaying just great. Happy New years fellas - this is everything I *love* (in a manly way of course) about the Drupal community.

Many Thanks!

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