In the Current adserve.php file on line 171 when this if statement occurs :

    if (empty($ads)) {
      adserve_variable('error', TRUE);
      $output = 'No active ads were found in the '. (empty($nids) ? 'tids' : 'nids') ." '$id'.";
    }

This produces the following to be displayed in the JavaScript

document.write('<!-- No active ads were found in the tids '0'. -->');

So because " ' " is being used inside a " ' " this is causing a Javascript error on the document.write

CommentFileSizeAuthor
#2 fix_multiline_ads.diff714 bytesdasil003

Comments

jeremy’s picture

jeremy commented
Assigned: Unassigned » jeremy

Thanks, I'll be sure this is fixed in the next release.

dasil003’s picture

dasil003 commented
StatusFileSize
new fix_multiline_ads.diff714 bytes

Here's a patch that fixes this issue (hopefully, untested) in addition to a problem if the output is multiple lines. Javascript doesn't like multiple line strings, so I'm splitting them. I HAVE tested the line splitting functionality, and am using it in production to make the ad_flash module work, where previously it was not displaying the flash ads in the blocks.

My patch is from the subversion of my project. Sorry I don't have time to do the CVS checkout and create a patch in the proper context.

jeremy’s picture

jeremy commented
Status: Active » Needs work

Yes, the "'" needs to be escaped, this portion will be merged soon.

My gut feeling is that the splitting functionality should be moved out of adserve.php into ad_text.module's ad_text_display_prepare() function. Why are there \n's being generated by the ad_flash module? This could also be implemented as a one line preg_replace_all().

jeremy’s picture

jeremy commented
Status: Needs work » Fixed

The primary problem of quotes breaking the output has been fixed by added a call to addslashes() in in adserve.php. The issue of displaying multi-line ads is not addressed as your implementation didn't work in my testing. If you want to pursue that issue, please open a new issue (and reference this one). I'd prefer to see it added to the function ad_text_display_prepare() in the ad_text module.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)