Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    text
    copied!<p>It looks like the reason it isn't working in Chrome is this line:</p> <pre><code>document.adBanner.src=adImages[thisAd] </code></pre> <p>You're referring to the element with the name "adBanner" via <code>document.adBanner</code>, but <a href="http://jsfiddle.net/makqj/" rel="nofollow noreferrer">Chrome doesn't support that</a>. You'll have to use something like this:</p> <pre><code>document.getElementsByName('adBanner')[0].src = adImages[thisAd]; </code></pre> <p>Some others things that could improve code quality:</p> <ol> <li><p><strong>Don't use the <code>language</code> attribute.</strong> Not necessary.</p></li> <li><p><strong>Use the format <code>var x = [...];</code> to create a new array.</strong> There's no reason to use the constructor format. None at all. Zippo. No one could possibly comment on this answer with a reason you'd use <code>new Array(...)</code> instead. No one.</p></li> <li><p><strong>Use the <code>var</code> keyword to create variables, and the semi-colon to end your statements.</strong> Although it isn't hurting anything here, if you don't use <code>var</code>, then JavaScript assumes you're creating/changing a <em>global</em> variable, when that may not necessarily be the case. (Also, the semi-colon rules may be a little convoluted, but it really helps with readability.)</p></li> <li><p><strong>Why are you checking for <code>document.images</code>?</strong> It's unnecessary. You don't refer to it anywhere else.</p></li> <li><p><strong>Crockford suggests using <code>x += 1</code> instead of <code>x++</code>.</strong> Not a big deal, and a lot of people disagree, just something I noticed.</p></li> <li><p><strong>Always use strict equality (===).</strong> The kind you're using (==) doesn't take into account types; <code>2 == "2"</code> will return true, but <code>2 === "2"</code> will not. Again, not a big deal, and some people don't really care, but it could bite you later on, in a different project.</p></li> <li><p><strong>Never pass strings to <code>setTimeout</code>.</strong> The browser just evals the string, and nobody hangs out with people who <code>eval</code> stuff. You don't even <em>need</em> to pass a string, because you're using a function that doesn't need any arguments! Just use this:</p> <pre><code>setTimeout(rotate, 3 * 1000); </code></pre></li> <li><p><strong>Try to put script tags at the bottom of the body.</strong> There are two reasons for this. First, performance. When the browser gets to your script, it stops <em>everything</em> to parse and execute the code. If you put it at the bottom of the body instead of the head, the page will at least appear to load faster. The second point is addressed next:</p></li> <li><p><strong>Try to avoid using <code>onload</code>.</strong> It's just gauche. The reason you need to is because your script is in the head, and has no access to the DOM just yet. But if that script was moved to the bottom of the body (which, for some reason, you might not be able to; no big deal), you wouldn't have to mess with <code>onload</code> at all:</p> <pre><code>&lt;body&gt; &lt;div id="banner"&gt; &lt;img ... /&gt; &lt;/div&gt; &lt;script&gt; // Copy all of your code exactly the same, // and then: rotate(); &lt;/script&gt; &lt;/body&gt; </code></pre></li> <li><p><strong>For the love of god, don't use the <code>name</code> attribute.</strong> For forms, who cares? But when you're manipulating elements with JavaScript, use the id. It's immediately obvious what you're doing, and <code>document.getElementById('adBanner')</code> is way faster than <code>document.getElementsByName('adBanner')[0]</code>.</p></li> </ol> <p><img src="https://i.stack.imgur.com/9ZGas.jpg" alt="enter image description here"></p>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload