Refactor the Adsense module for Drupal 6.x [#265666]

Comment	File	Size	Author
#22	adsense.tgz	20.54 KB	jcnventura
#18	adsense.tgz	16.13 KB	jcnventura
#12	adsense-6.2-alpha.tar_.gz	12.86 KB	kbahey
#11	adsense.tgz	11.99 KB	jcnventura

Comment #1

wayland76 commented 3 June 2008 at 07:47

subscribe

Log in or register to post comments

Comment #2

toovy commented 4 June 2008 at 21:27

Thanks for the work. Looking forward.

Log in or register to post comments

Comment #3

ares069 commented 11 June 2008 at 18:13

Subscribe. If you need any assistance testing, or even developing the re-factor of the module, please let me know.

Log in or register to post comments

Comment #4

mcurry commented 12 June 2008 at 04:01

subscribing. I can now test some aspects (I can't test revenue sharing or click tracking) on D6.

Log in or register to post comments

Comment #5

jcnventura

English

commented 16 June 2008 at 09:11

I know I haven't said anything in a while (I was a bit sick a while ago)..

I have mostly finished what I want out of the first release-able version:

1. Module and sub-module structure
2. Search modules (I found out that search also got re-designed so there will be a CSE search module also)
3. New ad code module.

What am I leaving out at the moment?
1. Click tracking (not working because of Google Javascript), so out.
2. Old ad code (without slot id). This can trivially be added once the main code is verified more or less OK.
3. Groups and Channels (requires and required by 2.)
4. Revenue sharing module (requires 2.)
5. Blocks (still looking into how I can do this). I don't like the automatic creation of 10 adsense blocks.. The other day I was checking through my Drupal DB, and I found out that I had around 300 unused block definitions created by adsense each time that I tried out a theme..

I hope to be able to submit the code here soon.

João

Log in or register to post comments

Comment #6

kbahey commented 16 June 2008 at 23:07

Actually, out of the second list, the most important are:

5 (Blocks) which provides an easy way to add blocks and configure the ad for size, ...etc. We could make the configuration option for the block the google_ad_slot ID that you get from Google, and a drop down of the valid width and height combos. The colors would have to be set in your Google account, but ce la vie.

This block feature is important. The DB pollution is not a real issue since most sites use just one theme, and only people who fiddle with their site would see lots of entries in the database. Not a real problem.

4 (Revenue sharing). I don't use it on my sites, but I know lots of people who do. The problem is that under the new ad code, if we change the google_ad_client just like we always did, then where would the google_ad_slot come from? Maybe we need a list for each ad position where each person need to setup a slot in his/her Google Adsense account and enter it on a configuration page which has placeholders for each ad position with a link to click where they can enter their own slot id? If this is AJAXy, that would be cool too (no need to redirect to another page).

We can leave revenue sharing for the first cut, and take a phased approach.

I would not worry much about the old ad code, since Google may drop it one day.

Here is an example of the new code ...

<script type="text/javascript">
<!--
google_ad_client = "pub-xxxxxxxxxx";
google_ad_slot = "384999998";
google_ad_width = 728;
google_ad_height = 90;
//-->
</script>
<script type="text/javascript"
src="http://pagead2.googlesyndication.com/pagead/show_ads.js">
</script>

Log in or register to post comments

Comment #7

jcnventura

English

commented 18 June 2008 at 14:04

The problem with blocks at the moment is how to configure them.. I am planning to have a simple drop-down selector for the different ad generators types (I count 4: old search, new cse search, old format ad, new slot format ad). If some crazy user installs all four modules, he will be presented with the four choices in the drop-down and he can say: in block 1 I will want a cse search, in block 2, a new format ad, and in block 3 an old format ad. The general code will simply store this info and it will provide a link to define the appropriate settings.

Anyway, I will start with just the filter and the adsense_display call, and I will add the blocks once I finish those. The way I am doing the new adsense_get_client_id call, I believe that the current modules should work with minimum adaptations.

As to the number of blocks, I will try to make them configurable in the setting interface. The default will be zero for people who don't like to use adsense blocks (like me), but it will be modifiable (upto say 99?).

Regarding the adsense_display call, I will leave a wrapper for the current interface, but the code will be using an array of parameters, as none of the ad unit generators uses all of them.

João

Log in or register to post comments

Comment #8

jcnventura

English

commented 18 June 2008 at 14:12

About the revenue sharing... Let's just forget it for the new format code for the time being, it quickly becomes unpractical. I think that revenue sharing with the new code will have to be handled downstream by the site owners and not by Google.

If someone comes up with a solution that doesn't involve having to synchronize ad units across adsense accounts and storing multiple slot IDs per ad unit, I am all ears. Imagine 10 ad units and 10 content authors.. This means someone must input 100 ad slot IDs.. We could ease the pain by creating a profile module for each content author to know which ad units he must create and to insert the slot IDs himself, but this is something that can safely left to be tackled once all other issues in this module are done.

João

Log in or register to post comments

Comment #9

kbahey commented 18 June 2008 at 15:56

@jcnventura

The problem with blocks at the moment is how to configure them.. I am planning to have a simple drop-down selector for the different ad generators types (I count 4: old search, new cse search, old format ad, new slot format ad). If some crazy user installs all four modules, he will be presented with the four choices in the drop-down and he can say: in block 1 I will want a cse search, in block 2, a new format ad, and in block 3 an old format ad. The general code will simply store this info and it will provide a link to define the appropriate settings.

Let us forget the old code. We have an opportunity to get rid of legacy code, and do things The Drupal Way(TM). The 5.x-2.x code is already too complex and ugly, and this is our chance to clean it up and have elegant code once more.

This leaves only two types: CSE Search and New Ad code.

So, if a user enables the Adsense CSE Search module, there will be one block provided by default. If they don't enable it, no such block is provided.

I like the way that there are predefined blocks now. It makes things very easy for people who are not developers to configure things. They don't need to write code. All they need is to select an Ad format (dimensions), and enter a Slot ID from their Google Adsense account. Done!

As to the number of blocks, I will try to make them configurable in the setting interface. The default will be zero for people who don't like to use adsense blocks (like me), but it will be modifiable (upto say 99?).

I strongly disagree. We should need at least one ad defined, so they know it is there. They can increase that if they want in the settings, but there has to be one that is available. If you do not like using blocks, you just ignore that one, and it will not show anywhere, until you assign it to a region and configure it.

Regarding the adsense_display call, I will leave a wrapper for the current interface, but the code will be using an array of parameters, as none of the ad unit generators uses all of them.

Actually, let us use slot IDs for it, so we get rid of the old code entirely.

In the help text, we mention that you have to migrate your ads to the new code and provide a link to Google's help to do so.

About the revenue sharing... Let's just forget it for the new format code for the time being, it quickly becomes unpractical. I think that revenue sharing with the new code will have to be handled downstream by the site owners and not by Google.

If someone comes up with a solution that doesn't involve having to synchronize ad units across adsense accounts and storing multiple slot IDs per ad unit, I am all ears. Imagine 10 ad units and 10 content authors.. This means someone must input 100 ad slot IDs.. We could ease the pain by creating a profile module for each content author to know which ad units he must create and to insert the slot IDs himself, but this is something that can safely left to be tackled once all other issues in this module are done.

We store the slots for the user for the spot in a table. Like this:

Administrator enables say 3 ad spots. One is 468x60, another is 728xwhatever, the third is 336xblah.

So, the revenue sharing module will use two sets of info:

table: adsense_block
block_delta
format (dimensions, this could reference another table with formats in it)
slot_id (Default slot ID, i.e. the site owner's slot, or when no revenue sharing is present)

table: adsense_user
uid
block_delta
slot_id

We can postpone all this for now. We just need the first table (adsense_block) for the part I discussed above.

Log in or register to post comments

Comment #10

carusen commented 20 June 2008 at 18:03

Guys... i don't mean to be pushy or rude but... how long it will take to finish the port to Drupal 6?
We have 6.2 already. :(

Log in or register to post comments

Comment #11

jcnventura

English

commented 21 June 2008 at 01:07

Status:

Active

» Needs work

Status	File	Size
new	adsense.tgz	11.99 KB

OK.

Here is the pre-alpha version :)

What is done:
- Publisher ID
- Old search code
- New ad formats

Still to be done before calling a dev release:
- Add the blocks :)
- Add the text ref ads
- Add the CSE search code
- Finish the mechanism that transparently routes the calls to the old code or the new code (for ads and for search)
- Polish the documentation a bit

Still to be done before making it the official version:
- Add the old code
- Convert the revenue sharing module to Drupal 6 for use with old format code
- Add the publisher ID module selection to the settings

Please test this in your sites, and give me some feedback. I will be on vacation for a week, so I may not be able to reply soon enough.

João Ventura

BTW: It's not a patch.. It's a complete module package

Log in or register to post comments

Comment #12

kbahey commented 21 June 2008 at 21:49

Status	File	Size
new	adsense-6.2-alpha.tar_.gz	12.86 KB

@carusen

Such questions are not helping.

This software is being worked on by volunteers on their own time. It will be done when it is ready. There is no deadline.

Unless people want to jump in and help by contributing code, testing, and documentation, such questions just distract from getting the code out ...

If you want it sooner, you can do one or more of the above, or offer a financial contribution to Joao (no idea if he needs it or will even accept it, but you [and/or others] can offer it).

@jcnventura

I want to put your code under the DRUPAL-6--2 tag, so we can offer dev snapshots. Will wait till you finish the rest of the stuff (at least till -dev is doable). Once we do that, you can get CVS access and others can contribute documentation and patches easily via the issue queue.

Again, I would prefer that we ditch the old code and worry about that last, so the modules' code is not burdened with it, and we lose complexity. All this bridging will just complicate things more and more and we will end up with an unmaintainable and unmodular mess.

Some specifics:

- The file adsense_managed.admin.inc referenced in hook_menu is missing. We need it.
- Rename the title for the settings page from "Web page" to "Settings", it is confusing.

I wrote an adsense_block.module and included it in the attached tar ball (which includes everything jcnventura has done too). It partially works, but needs the channel part still. It is also better if it uses its own table rather than the variable table.

Log in or register to post comments

Comment #13

PGiro commented 22 June 2008 at 20:46

subscribe

Log in or register to post comments

Comment #14

reptilo commented 24 June 2008 at 11:45

Same as WatcherFR !

Log in or register to post comments

Comment #15

Goose4all commented 24 June 2008 at 20:14

guys, there is something wrong with the "pre-alpha"

if i put it on my site, i get this in the html

<div class="blockcontent"><!--adsense: placeholder-->
<div style="text-align: center; border:solid 1px;">Google AdSense </div></div>
</div>      </div>

this seems correct, cause i am logged in..

i've created 1 5link horizontal link block and if i log out, i get this:

<div class="defaultblock">
    <h2></h2><!--block title-->
   <div class="blockcontent"><!--adsense: ad limit reached for type--></div>
</div>      </div>

and this is definitely wrong cause its the only adsense block on the whole site..

i cant contribute code, but i am willed to test it.

thank you!

Andreas

Log in or register to post comments

Comment #16

jcnventura

English

commented 1 July 2008 at 08:31

Hello,

I just got a mail from Google saying that they will phase out the referrals program at the end of August 2008. As such, I will no longer integrate text referrals to the module.

So that feature is out, and I will actually remove the CPA and referral flags for non-text refs also.

João

Log in or register to post comments

Comment #17

vjirasek commented 3 July 2008 at 06:55

I have just installed http://drupal.org/files/issues/adsense-6.2-alpha.tar_.gz and got this error when going to Administre -> AdSense -> Managed Ads.

Fatal error: require_once() [function.require]: Failed opening required 'sites/all/modules/adsense/adsense_managed.admin.inc' (include_path='.:/usr/share/php:/usr/share/pear') in /var/www/project/includes/menu.inc on line 344

The missing file is not part of the archive. I am happy to test and maybe contribute the code but I'm fairly new to Drupal.

Log in or register to post comments

Comment #18

jcnventura

English

commented 6 July 2008 at 17:22

Status	File	Size
new	adsense.tgz	16.13 KB

Hello again.

Here is the latest version. This one is, I think, dev-capable.

It includes:
- New format ads (Managed ads)
- Blocks
- New search (CSE)
- Test mode now uses the adtest variables that are undocumented, but are present in AdSense's JavaScript files
- Removal of all AdSense for Referrals functions.

Still missing:
- Polish the documentation
- Finish the adsense input filter code.
and of course, the old format code and the revenue sharing methods.

One thing I am trying to do is to add farbtastic support to the color selection in the 'old search' module settings. If anybody can help me with the JavaScript for that I would appreciate it a lot.

I have decided to move the code kbahey provided in #12 to each of the ad unit generation modules. This makes for some code repetition, but we get direct access to the stuff that can be configured in each different ad type, which makes it a lot easier to work with and understand.

I will keep working on the above points, but I would appreciate testing and feedback of the existing functions (Managed Ads, CSE and old search via the adsense_display calls and blocks). I think two or three external confirmations that it is working for someone would be better before making this public via the -dev mechanism.

João Ventura

Log in or register to post comments

Comment #19

kbahey commented 7 July 2008 at 19:21

Again, I will not bother with the old search. Let us focus on the new code and not waste our efforts on the old stuff.

Here are some review notes (the old search is excluded of course).

- Move the adsense_search to the contrib directory, unsupported, and those who want to maintain it can do so there. Rename it to adsense_old_search too.

- There is no uninstall yet under the Uninstall tab after you disable the module.

- The first page under admin/settings/adsense has the Visibility and Advanced. Once you navigate to other tabs, the first tab disappears.

- If the module is disabled then enabled again, there is no admin/settings/adsense again (behaves as if it is admin/settings only). However, there are tabs for Managed Ads and Custom Search, but not for publisher ID, nor the settings tab.

- There are two errors:
* notice: Undefined index: adsense_main_settings in ../includes/form.inc on line 345.
* warning: call_user_func_array() [function.call-user-func-array]: First argument is expected to be a valid callback, 'adsense_main_settings' was given in ../includes/form.inc on line 359.

- Let us have each module and its .inc and .install, .info in its own subdirectory, rather than all of them being in the adsense directory.

Log in or register to post comments

Comment #20

jcnventura

English

commented 7 July 2008 at 19:41

Hi,

I will move the modules to specific directories like you suggested. It's a "good idea(TM)". :)

However, I can't duplicate the problems you had with the tabs.. I tried enabling/disabling and navigating through the tabs and what you describe never happenned. I think there may be weird things going on with your Drupal installation.. The two errors you described seem to confirm that there is a problem in the adsense_main_settings, but looking at it, I can't find any problem.. Maybe your theme is having problems with the module?

João

Log in or register to post comments

Comment #21

kbahey commented 7 July 2008 at 21:19

I tested again on a fresh install of the latest Drupal 6.x from CVS. There is nothing from contrib on this installation except the adsense module you attached. Everything else is just the default (garland, ...etc.).

I can't reproduce what happened last time, so for now ignore those errors.

Log in or register to post comments

Comment #22

jcnventura

English

commented 15 July 2008 at 15:03

Status	File	Size
new	adsense.tgz	20.54 KB

Hi,

Here I am again. This one is definitively dev-capable.

Changes to the previous version:

1. Added the old format code
2. Fixed the input filter code (see the the input filter tips for instructions - basically current input filters should still work, but I have added a new filter that is a lot simpler as it is only [adsense:format:slotID])
3. Added the uninstall code
4. Added upgrade code from 5.x to 6.x that moves old blocks definitions into the new blocks in a way that the block input filter still works.. To do this, the input filter now works with the block name. The upgrade function just makes sure that upgraded blocks have the old number as the new name. Since each module creates its own set of blocks, the simple 'delta' number wasn't going to work anyway (works perfectly, but don't give the same name to two different blocks)

What I didn't do from #19:
- Didn't move the 'old' modules to the contrib directory. I think we should support these modules for a while still. I moved them to the 'old' directory.
- Didn't rename adsense_search to adsense_oldsearch. Sorry, but this could introduce bugs in the code and it would break the 'beauty' that all the adsense_search variables upgraded from 5.x are simply used by the adsense_search module. This way there's no need to rename the variables to adsense_oldsearch.

Refinements to be done:
- Documentation... (code and README.txt)
- Port the existing revenue modules (to be called, if enabled, only by old search and old code)
- There's no plug-and-play support for ad unit modules.. The input filter cycles through the 4 known modules looking for the correct block and the adsense_display tries to use them in the applicable case.

The last point is not important as it is simply adding support to something that doesn't exist. The current code works fine with whatever combination of modules is enabled at the time.

Please test these in your sites, and report any bugs here, so that we can make this public to everyone via the dev mechanism.

Log in or register to post comments

Comment #23

chrissearle commented 20 July 2008 at 12:46

Watching

Log in or register to post comments

Comment #24

alexanderpas commented 20 July 2008 at 17:32

watching, subscribing, and waiting to see a 6 branch on cvs

Log in or register to post comments

Comment #25

kbahey commented 21 July 2008 at 00:25

Version:

master

» 6.x-1.x-dev

@jcnventura

I committed this to the repository, under the DRUPAL-6--1 tag.

A development release will appear here shortly http://drupal.org/node/285093

Log in or register to post comments

Comment #26

CompShack commented 21 July 2008 at 18:29

@jcnventura - Thanks for your hard work on this.

Revenue sharing is still not in d6 dev, correct?

Log in or register to post comments

Comment #27

tf5_bassist commented 22 July 2008 at 16:24

subscribing.

oddly enough, i'm getting the white page of doom on all tabs in the Adsense module, other than the Settings tab. weird.

Log in or register to post comments

Comment #28

alexanderpas commented 22 July 2008 at 17:12

Priority:

Normal

» Critical

seeing as we have a 6.x-1.x-dev branch... upping this to critical ;)

and also, i'm for dropping the old-type advertisement code... Google offers a way to change old-type into new slot-type code...

Log in or register to post comments

Comment #29

jcnventura

English

commented 22 July 2008 at 21:15

Status:

Needs work

» Needs review

@tf5_bassist - what you're reporting is very weird. Try disabling all the adsense modules and uninstalling them. Either you have the module installed and the tabs show or you don't and they don't show. I never gota a white page of doom on them. What's your environment?

@alexanderpas - well, the old-format ads are still usable, and if you create an ad block in Blogger today you still get that format.. Also, since it's the only viable solution to get a revenue sharing scheme in the short term, I would think that it still has it's place.

Changing this to code needs review, as that's what's needed...

Log in or register to post comments

Comment #30

kbahey commented 23 July 2008 at 02:01

Priority:

Critical

» Normal

Critical means the module is totally broken. Since this is dev code that needs review, it can't be that critical.

Reviews appreciated please ...

Log in or register to post comments

Comment #31

alexanderpas commented 23 July 2008 at 21:44

uhm... doesn't a white page of doom qualify as totally broken... at least for that part...

Might do that review myself when i have some time, but I can't guarantee anything...

Log in or register to post comments

Comment #32

CompShack commented 24 July 2008 at 02:44

I have installed both AdSense core and Managed ads with no problems. Was able to save my adsense id, create an add, generate 5 total blocks and display an add.

I have noticed two bugs though, when i go under setting and "Show AdSense on specific pages: > Show on every page except the listed pages." I have listed node/* just to make sure it is working but when i navigate to a node content, the ads are still getting displayed. If i change that to admin/* then the ad block doesn't show BUT the ad bock title is still there! (i don't think the title should stay visible).

I'm excited about this conversion to D6. I will be taking a flight for the next few weeks so sorry wont be able to do any more testing, but i will be back for more in about 3 weeks :)

Log in or register to post comments

Comment #33

jcnventura

English

commented 24 July 2008 at 08:14

@CompShack:
Bug 1: Indeed, I only use the current URL and I don't check for path alias, that's why the node/* didn't work for you.
Bug 2: Even though there's a block-specific visibility setting that covers that, I will see what can be done about extending the ad visibility to the whole block. It seems logical that the block title should not be shown alone.

@alexanderpas: the white page of doom would be critical if it were reproducible.. I can't reproduce it at all, so I am taking it to be some kind of transient bug.

Thanks for the review! Keep them coming!

Log in or register to post comments

Comment #34

jcnventura

English

commented 24 July 2008 at 09:18

The above bugs reported by CompShack have been fixed in the latest dev, which should be created soon.

Log in or register to post comments

Comment #35

Artem commented 24 July 2008 at 16:59

Subscribing. An very important patch it would be.

Log in or register to post comments

Comment #36

CompShack commented 25 July 2008 at 00:32

@jcnventura - I have re-tested both AdSense core and Managed ads and all appears to be working fine after your latest bug fix apply to dev.

One thing that i didn't test and that is the "Enable test mode?" under the advanced options. What is this supposed to do? I've enabled it and saved but didn't notice any difference in my ad display. Just confused on how it works so i know how and what to test :)

Later!

Log in or register to post comments

Comment #37

jcnventura

English

commented 25 July 2008 at 00:59

Well, the test mode is something that I need to document better..

IT'S DANGEROUS.

The test mode uses the adtest flag that is still present in the current Google Javascript. This of course goes against the TOS. According to some forums, it will turn on the display of ads, but won't count them at all so it can, in theory, be used during site testing.

However, as Google states, you can't display ads during site testing, and you can't modify the code they give you. Since that never includes this flag, even if they process it in their JavaScript, it's use it's a bit marginal.

So, use it at your own risk.

João

Log in or register to post comments

Comment #38

CompShack commented 25 July 2008 at 02:17

@jcnventura
>> This of course goes against the TOS.
>> So, use it at your own risk.

So why even bother and have it in the module?! If it was up to me, i would just take it off. OK THEN, I will not be doing any testing on that part as I will not be using it.

I have also just finished testing CSE Search part of the module and I'm very impressed jcnventura! Works like a charm and no problems/bugs that i've noticed. I have only tested the search with Site Encoding: Unicode(UTF-8) and country of United States as this is what I will be using. My test included:
- Altering Logo Type to make sure the Search display really changes :)
- Change Background Color - also to make sure this is working as well.
- Change Text Box Length and text box did change :)
- Change Ad Location - I really like this one and it works perfect!
- Change Number of blocks (adding and subtracting) both ways works great.

One note though that I would like to make. When logged in, the search block is disabled. I guess this works the same way as for displaying ads to prevent users from clicking on their ads (for revenue sharing) BUT this is a search functionality and I think it should always be enabled, what do you think?

I will not be testing the last 2 parts of the module ( both related to old code). I wont be using any old code.

Again great work jcnventura, the module setting is very clean and easy to understand. Do you have any time line on when you will start working on the revenue sharing part?

Log in or register to post comments

Comment #39

kbahey commented 25 July 2008 at 16:10

The old test mode on Drupal 5, was totally out of Google's control. All it did was to display the values that would have been used for the ads. So, it would display the slot, the dimensions, ...etc. but no actual javascript.

We can switch back to using this safer test mode instead of the risky one.

Log in or register to post comments

Comment #40

tf5_bassist commented 27 July 2008 at 11:23

Edit: Apparently I'm a complete idiot, or I'm not used to the Ubuntu archive manager. Somehow I had the awesomosity to have extracted the files into one folder. Not the separate folders it should've been. Tree structure is now uploaded properly, and all pages now show properly. Hopefully from here on out any issues I have are real issues, and not PEBCAK-related.

@jcnventura - Just my luck, I get the odd bug out of the bunch hehe. Well, I'm running Drupal 6.3 with a boat-load of modules, pathauto, taxonomy stuff, etc etc. The module's installed, it's there. I wonder if this could possibly be due to something having to do with my .htaccess setup relating to php5. I had to switch over to php5 for a certain module or two, and it doesn't seem to have been made a recursive change into all folders... so...

Talk about timing, I just checked my Admin/Modules page, and this is what I got as an error in the big red box:

# warning: include_once(modules/adsense/help/adsense_id_help.inc) [function.include-once]: failed to open stream: No such file or directory in /homepages/43/d240332804/htdocs/modules/adsense/adsense.admin.inc on line 91.
# warning: include_once() [function.include]: Failed opening 'modules/adsense/help/adsense_id_help.inc' for inclusion (include_path='.:/usr/lib/php5') in /homepages/43/d240332804/htdocs/modules/adsense/adsense.admin.inc on line 91.
# warning: include_once(modules/adsense/help/adsense_managed_help.inc) [function.include-once]: failed to open stream: No such file or directory in /homepages/43/d240332804/htdocs/modules/adsense/adsense_managed.admin.inc on line 18.
# warning: include_once() [function.include]: Failed opening 'modules/adsense/help/adsense_managed_help.inc' for inclusion (include_path='.:/usr/lib/php5') in /homepages/43/d240332804/htdocs/modules/adsense/adsense_managed.admin.inc on line 18.

So I don't know quite what to make of this. Oh, and another thing I noticed, I don't have the Old Code Ads and Search (Old) modules activated. Do those need to be enabled in order to not break the whole thing? Let's test that.

Enabled those two, and i have the tabs for them now, although Search (Old) gives a white page, Old Code Ads works. but now both that page and the Settings page give this red box error:


    * warning: require_once(modules/adsense/includes/adsense.search_options.inc) [function.require-once]: failed to open stream: No such file or directory in /homepages/43/d240332804/htdocs/modules/adsense/adsense_cse.admin.inc on line 19.
    * warning: include_once(modules/adsense/help/adsense_id_help.inc) [function.include-once]: failed to open stream: No such file or directory in /homepages/43/d240332804/htdocs/modules/adsense/adsense.admin.inc on line 91.
    * warning: include_once() [function.include]: Failed opening 'modules/adsense/help/adsense_id_help.inc' for inclusion (include_path='.:/usr/lib/php5') in /homepages/43/d240332804/htdocs/modules/adsense/adsense.admin.inc on line 91.
    * warning: require_once(modules/adsense/includes/adsense.search_options.inc) [function.require-once]: failed to open stream: No such file or directory in /homepages/43/d240332804/htdocs/modules/adsense/adsense_search.admin.inc on line 19.

Hope this helps.

Log in or register to post comments

Comment #41

jcnventura

English

commented 27 July 2008 at 11:43

@tf5_bassist :
It helps, indeed. Now I know why it doesn't work for you.. It can't work for you if it doesn't find those files. Once it manages to find them, it will start working for you.

However, it's a mystery for me why it can't can find them. Are those .inc files there? Are they readable?

@CompShack + kbahey
Good point indeed. The reason for having it using this dangerous setting is that the old test mode in the 5.x module seemed to be a bit like the disabled mode + placeholder. One thing I liked about Google's dangerous test mode is that it actually let's you see the ads as they would look on your site. However, since it can get you banned, if you're indexed while using it, I think that providing a less 'extreme' solution may be a good idea. What do you think of displaying the images from here?

João

Log in or register to post comments

Comment #42

ares069 commented 29 July 2008 at 14:31

@jcnventura:

Thanks for the module! i've just installed it and am checking it out, and its looking good so far!

One thing I've noticed is that in FireFox, a wide ad is left-justified, but in IE, the same block is centered. I'm guessing this is a CSS issue between the two browsers, since the div is set to text-align:center, and the "Google Banner Ad" text is centered within the block. The block itself is just not centered.

Log in or register to post comments

Comment #43

tf5_bassist commented 4 August 2008 at 12:28

@jcnventura - I wasn't working because, as I stated in my edit at the top of that original post, I'm an idiot and somehow managed to extract the archive sans filestructure lol... Wtf, seriously.

Everything works now, and all functions, however, there's some issue with the way the code's being rendered when I put it in via tags in a post. the code is being outputted as such:

<script type="text/javascript">&lt;!--
google_ad_client = "ca-pub-xxxxxxxxxxxxxxxx";
/* 200x200 */
google_ad_slot = "xxxxxxxxxxx";
google_ad_width = 200;
google_ad_height = 200;
//-->
</script>
<script type="text/javascript"
src="http://pagead2.googlesyndication.com/pagead/show_ads.js">
</script>
</div></div>
<!-- google_ad_section_end -->

Note the ampersand code in the first line... Is that a known issue? Workarounds? I searched for that code and also for "ampersand" here, but couldn't find anything.

Also, if I remember correctly, I'm using the Full HTML option.

Okay, scratch that all... I switched back to my custom adsense input method that also handles PHP and it works. wow, I'm making some really lame mistakes here. Okay, that's fixed.

Log in or register to post comments

Comment #44

jcnventura

English

commented 4 August 2008 at 12:15

@tf5_assist: I am glad you managed to find the problem.. We all make stupid mistakes from time to time, that doesn't make you an idiot (using Joomla however... :) )

As to the ampersand, it's a known Drupal core problem (see #222926: HTML Corrector filter escapes HTML comments). I have code to filter out more troublesome HTML corrections, but the one you saw should never show up in modern browsers.

Log in or register to post comments

Comment #45

tf5_bassist commented 4 August 2008 at 12:34

@jcnventura - "tf5_assist"? Well, that's not too nice now, is it? haha...

Okay, well, the ampersand issue makes sense now. I'm not going to patch it though, considering as i'm all on wintel and have horrible luck with Cygwin. Maybe when I'm back home with Ubuntu I'll be able to patch. Switching to an input filter that allowed for PHP fixed it though, so it's a stopgap. My custom input filter is:

Full HTML/PHP/Javascript

* Use the special tag [adsense:format:slot] or [adsense:format:[group]:[channel][:slot]] or [adsense:block:location] to display Google AdSense ads.
* Lines and paragraphs break automatically.
* Web page addresses and e-mail addresses turn into links automatically.
* You may post PHP code. You should include tags.

It seems to work so far. :D

And yeah... that's why I switched from Joomla hehe... Now, just gotta' wait for the modules to catch up lol.

Log in or register to post comments

Comment #46

jcnventura

English

commented 4 August 2008 at 12:39

@tf5_bassist: Sorry, the b got lost somewhere :) A better solution instead of using the PHP filter is to disable the HTML corrector filter in both the Full HTML and the Filtered HTML input formats..

Anyway, it would help a lot of people patched their Drupal with the code in #222926: HTML Corrector filter escapes HTML comments and reviewed it in that issue. It might help in making the patch be available in Drupal 6.4.

João

Log in or register to post comments

Comment #47

Artem commented 12 August 2008 at 13:04

Subscribing

Log in or register to post comments

Comment #48

jcnventura

English

commented 12 August 2008 at 13:52

Status:

Needs review

» Reviewed & tested by the community

Looking back at the reviews, we have working good confirmations from:

- CompShack (#38)
- ares069 (#42)
- tf5_bassist (#45)

In the early part of the thread two users reported problems, but never announced their 'cure' or that they're still around:
- Goose4all (#15)
- vjirasek (#17) <- this one is so similar to tf5_bassist's problem that it must be fixed by now..

This is now in dev, and bug/problems are coming in their own issues, so I am actually moving to close this.. Does anyone object?

Log in or register to post comments

Comment #49

ares069 commented 12 August 2008 at 15:15

@jcnventura

no objection here.

has the firefox/ie6 centering issue i was mentioning been filed as a bug?

Log in or register to post comments

Comment #50

jcnventura

English

commented 12 August 2008 at 23:48

@ares069:

No, that's only used for ads placeholders, which should not usually be enabled AND it's an IE6 bug. The XHTML is quite clear: text-align:center, so why is IE6 centering the div? There may be some a known CSS workaround magic, but again, it's not that important.

I will close this in a few days, if nobody objects.

João