开发者

My Javascript works on IE FF & Safari, but not Chrome

开发者 https://www.devze.com 2023-03-02 00:02 出处:网络
I have a simple rotating banner javascript on the site I\'m developing. It works perfectly locally, on IE, Firefox and Safari but not on Chrome. Is there something wrong with my code? I\'m new to Java

I have a simple rotating banner javascript on the site I'm developing. It works perfectly locally, on IE, Firefox and Safari but not on Chrome. Is there something wrong with my code? I'm new to Javascript so this problem is really baffling me. Here's the relevant code:

<script language="Javascript" type="text/javascript">
    adImages = new Array
    ("images/AMSite18.png","images/AMSite19.png","images/AMSite09b.png")
    thisAd = 0
    imgCt = adImages.length

    function rotate() {
        if (document.images) {
            thisAd++
            if (thisAd == imgCt) {
                thisAd = 0
            }

            document.adBanner.src=adImages[thisAd]
            setTimeout("rotate()", 3 * 1000)
        }
    }
    </script>
</head>

<body onload="rotate()">
    <div id="banner">
        <img src="images/banner1.gif" name="adBanner" alt="Ad Banner" />
    <开发者_StackOverflow社区/div><!-- end banner -->
</body>


It looks like the reason it isn't working in Chrome is this line:

document.adBanner.src=adImages[thisAd]

You're referring to the element with the name "adBanner" via document.adBanner, but Chrome doesn't support that. You'll have to use something like this:

document.getElementsByName('adBanner')[0].src = adImages[thisAd];

Some others things that could improve code quality:

  1. Don't use the language attribute. Not necessary.

  2. Use the format var x = [...]; to create a new array. 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 new Array(...) instead. No one.

  3. Use the var keyword to create variables, and the semi-colon to end your statements. Although it isn't hurting anything here, if you don't use var, then JavaScript assumes you're creating/changing a global 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.)

  4. Why are you checking for document.images? It's unnecessary. You don't refer to it anywhere else.

  5. Crockford suggests using x += 1 instead of x++. Not a big deal, and a lot of people disagree, just something I noticed.

  6. Always use strict equality (===). The kind you're using (==) doesn't take into account types; 2 == "2" will return true, but 2 === "2" 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.

  7. Never pass strings to setTimeout. The browser just evals the string, and nobody hangs out with people who eval stuff. You don't even need to pass a string, because you're using a function that doesn't need any arguments! Just use this:

    setTimeout(rotate, 3 * 1000);
    
  8. Try to put script tags at the bottom of the body. There are two reasons for this. First, performance. When the browser gets to your script, it stops everything 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:

  9. Try to avoid using onload. 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 onload at all:

    <body>
        <div id="banner">
            <img ... />
        </div>
    
        <script>
        // Copy all of your code exactly the same,
        // and then:
    
        rotate();
        </script>
    </body>
    
  10. For the love of god, don't use the name attribute. For forms, who cares? But when you're manipulating elements with JavaScript, use the id. It's immediately obvious what you're doing, and document.getElementById('adBanner') is way faster than document.getElementsByName('adBanner')[0].

My Javascript works on IE FF & Safari, but not Chrome


You should start by fixing the syntax problems.

Lines should end with a semi-colon ;, variable should be declared with var and you should use [] rather than new array, pass a function to setTimeout rather than a string

var adImages = ['images/AMSite18.png','images/AMSite19.png','images/AMSite09b.png'];
var thisAd = 0;
var imgCt = adImages.length;

function rotate() {
    if (document.images) {
        thisAd++;
        if (thisAd == imgCt) {
            thisAd = 0;
        }

        document.adBanner.src=adImages[thisAd];
        setTimeout(function(){
           rotate();   
        }, 3 * 1000);
    }
}

This may not fix it, but you you should do it anyway :)


I just ran you code on Chrome 11 on a Mac and it worked. Even with the syntax errors. But Paul is right you should always write valid JavaScript.

Also this is a better way of passing functions

setTimeout(rotate, 3 * 1000);
0

精彩评论

暂无评论...
验证码 换一张
取 消