Banner Rotator - IE7 shows "undefined" on page

Hi guys.

I have a banner rotation script from the pcman website which works perfect in Safari and FF, but not IE7.

IE7 will sometimes show the images and sometimes show "undefined".

Below is the code.

<script type="text/javascript"><!--
/*
 Copyright 2009 The PCman Website Rotating Banner Code
 http://www.thepcmanwebsite.com
 This code is free to use provided this notice is not removed.
 Create your own custom banner code with banners
*/
mybanners=
[
"<a href=\"http://www.url1.com\" target=\"_blank\"><img src=\"http://www.url1.com/banners/sbg-125x125.gif\" width=\"125\" height=\"125\" alt=\"name1\" border=\"0\"></a>",
"<a href=\"http://www.url2.com\" target=\"_blank\"><img src=\"http://www.url2.com/banners/sbg-125x125.gif\" width=\"125\" height=\"125\" alt=\"name2\" border=\"0\"></a>",
]
randomNumber = Math.random()
var show_mybanners = mybanners[Math.floor(randomNumber * mybanners.length)]
document.write(show_mybanners);
// --></script>
<noscript>
<a href=\"http://www.url1.com\" target=\"_blank\"><img src=\"http://www.url1.com/images/adverts/adhere1_125x125.png\" width=\"125\" height=\"125\" alt=\"name1\" border=\"0\"></a>
</noscript>

Code (markup):

Any help to resolve this would be appreciated.

 

The method for building the arrays is sloppy, there's no reason to be embedding the markup in the array like that, making it more complicated. The method for choosing a random number is semi-broken, and you seem to be slash escaping values in your noscript for no good reason... oh, and I'd axe that target attribute since there's no good reason for that ******* - it was deprecated in STRICT doctypes for a reason. (don't shove opening links in a new window down the user's throat - GOD that **** pisses me off every time I encounter it)

This is untested, but I'd write that like this:

<script type="text/javascript"><!--

myBanners=new Array(
	new Array(
		'http://www.url1.com',
		'http://www.url1.com/banners/sbg-125x125.gif',
		125,125,
		'name1'
	),
	new Array(
		'http://www.url2.com',
		'http://www.url2.com/banners/sbg-125x125.gif',
		125,125,
		'name2'
	)
);

banner=Math.floor(Math.random()*(myBanners.length-1));

document.write('<a href="'+myBanners[banner][0]+'"><img src="'+myBanners[banner][1]+'" width="'+myBanners[banner][2]+'" height="'+myBanners[banner][2]+'" alt="'+myBanners[banner][4]+'" /></a>');

// --></script>
<noscript>

	<a href="http://www.url1.com">
		<img
			src="http://www.url1.com/images/adverts/adhere1_125x125.png"
			width="125" height="125"
			alt="name1"
		/>
	</a>

</noscript>

Code (markup):

The likely culprit to the error you saw though is the handling of math.Random, which is returning a floating point value, hence my using a math.floor. I also trap the size of the array length so as to be sure it doesn't try to access past the end of the array.

NOT that I'd be running this client side as javascript, and I CERTAINLY wouldn't be embedding it in the markup if I were to do that. Wrap it in a function and get the lions share of that the hell out of there