ad module generates block content even if no ads are to be displayed

Confirmed bug.

I have a similar issue. Just downloaded the HTML code for an Amazon.com affiliate ad. It is an iFrame. Width of the frame is less than my sidebar. Ad entered as HTML using "Full HTML" input mode. Set block to left sidebar. Displays top portion of block with block title, but no ad content (no iFrame) from Amazon. Non-iFrame HTML ads from remote source display without problem.

Do I need to set <iFrame> as an allowed tag somewhere?

Suggestions?

The latest DEV release doesn't fix the problem. Still an empty block shown when no ads are available for the group.

@tetramentis

Thank you. You hit it on the head. I do find that after setting the ad as 'Full HTML' and saving it, when I return to edit it, the input format is set back to 'Basic'. I am using Advertisement 6.x-2.1 (Aug 05, 2009). I will try the DEV version and see if it solves the problem.

UPDATE: Installed 6.x-2.x DEV. Problem solved.

Thanks for the patch, I appreciate your contributions! However, I'm not going to commit it as it doesn't fully fix the bug.

Here's a patch that I believe fixes the bug for all display types. It could use more testing to be sure it doesn't cause any problems I didn't think of. The solution this patch uses is CSS, hiding the block if there was an error.

Testing and feedback welcome!

> I believe your patch doesn't fully fix the problem either, as output is still
> produced - just hidden by JS. What if someone has disabled JS for increased
> security? (I'm not talking about text browsers, as they wouldn't be visually affected anyway.)

There is no fix for that, really. The reason being, when using any display method except raw there is _always_ going to be block output. If using the Js or jQuery display methods, then the output is a little javascript. That script then loads the appropriate ad(s) -- if there are none, the only solution we have at this point is to hide the block with CSS.

Anyone that is disabling JS on their website will be seeing modified pages anyway, so they've already made this choice.

I agree though, for the raw method we can prevent the display of the block altogether, and thus I will do so.

Otherwise, I'm just looking for testers to confirm that my patch successfully hides the block when no ads are displayed in all browsers on all operating systems. As you have noted, it won't work on browsers that don't have JS enabled, but it should work on the rest.

Subscribing to this because it is also critical for my site design that *no output* be generated in the block when there is no ad to be displayed. Seems like the module should be able to determine if no ad is being displayed and then prevent the output of the html and javascript.

In my case, I'm using Image Ads. I have different Ad Groups that are displayed on different pages. If a particular Ad Group doesn't contain any ads, then I don't want any HTML output for that block on that page.

Subscribing as well since having the same issues as above. Will try the patch out!

Edit: tried the patch but didn't get it to work, the output is parent.document.getElementById('block-ad-t25').style.display="none"; but my block is called block-ad-25 is that the problem?

Looks like this isn't working as intended, so I'm not going to merge the patch for the next release. If someone wants to build on what I've attached above and contribute back something that works better, that would be much appreciated.

Subscribing to this very, very important topic.

+1

Looks like this isn't working as intended, so I'm not going to merge the patch for the next release. If someone wants to build on what I've attached above and contribute back something that works better, that would be much appreciated.

What exactly isn't working as intended ?

Subscribe.

Subscribing...

Subscribe

Subscribing...

Subscribing

Subscribing

fixed patch from #8.

@#14 - you're right, it was targeting the wrong element id. I'd added a preg_replace to strip out the 't'

Another thing you could do is copy theme_ad_display() from ad.module (should be line 202) and add it to your template.php file. Then modify the function to something like this:

function yourthemenamehere_ad_display($group, $display, $method = 'javascript') {
  static $id = -1;
  $output = '';
  
  // Increment counter for displaying multiple advertisements on the page.
  $id++;

  // The naming convention for the id attribute doesn't allow commas.
  $group = preg_replace('/[,]/', '+', $group);
  $tid = str_replace('tids-','',$group);
  $q = db_query("SELECT * FROM term_node WHERE tid = ".$tid);
	if(db_affected_rows($q)){
	  if ($method == 'jquery') {
		$output .= "\n<div class=\"advertisement group-$group\" id=\"group-id-$id\">\n <script type=\"text/javascript\">\n//<![CDATA[\n  $(document).ready(function(){ jQuery(\"div#group-id-$id\").load(\"$display\"); });\n //]]>\n </script>\n</div>\n";
	  }
	  else if ($method == 'raw') {
		$output .= $display;
	  }
	  else {
		$output .= "\n<div class=\"advertisement group-$group\" id=\"group-id-$group\">$display</div>\n";
	  }
	}
	
	return $output;
}

Since all of my ad blocks only use a single tid that works great for me. If you're using multiple then just get the tids from $group and put them in an "IN()" clause in the query.

For those of you who don't know, what this does is just check if there are any ads for that term id. Since every ad group is tracked as a taxonomy term, searching term_node for any values with a specific ad group tid will tell you if any ad nodes exist. If not, it outputs an empty string which will keep the block from being displayed. Otherwise it'll show the ads as usual.

A slight variation to the code snippet above which checks for ACTIVE ads.

Again, insert this code into your template.php

function phptemplate_ad_display($group, $display, $method = 'javascript') {
  static $id = -1;

  // Increment counter for displaying multiple advertisements on the page.
  $id++;

  // The naming convention for the id attribute doesn't allow commas.
  $group = preg_replace('/[,]/', '+', $group);

  // The naming convention for the id attribute doesn't allow commas.
  $tid = str_replace('tids-','',$group);
  $q = db_query("SELECT tn.nid FROM {term_node} tn INNER JOIN {ads} a ON tn.nid=a.aid WHERE tid = ".$tid . " AND a.adstatus='active'");

if(db_affected_rows($q)){
  if ($method == 'jquery') {
    return "\n<div class=\"advertisement group-$group\" id=\"group-id-$id\">\n <script type=\"text/javascript\">\n//<![CDATA[\n  $(document).ready(function(){ jQuery(\"div#group$
  }
  else if ($method == 'raw') {
    return $display;
  }
  else {
    return "\n<div class=\"advertisement group-$group\" id=\"group-id-$group\">$display</div>\n";
  }
 return '';
 }
}

Though a very great module in case of features, but this seems a major drawback when websites are now working in CSS powered multi-platform environments. Hope no one minds the priority change.

We are working to find the solution.

Drupal 6 is EOL and no longer supported. Closing this as outdated for that reason. Thanks for your contribution!