1

I am writing a code to add a div depending on a set of 'object' found in document on click of button.

When Document.getElementsByTagName('object'); there is only one object, but it goes in to an infinite loop! giving me incremented value of swf.length incremented by one infinitely in an alert. can you help whats wrong am i doing!

<script>
 function get2(){
    var swf = document.getElementsByTagName('object');
    for (j=0;j<swf.length ;j++ )
    {
        alert(swf.length);
        var objs = swf[j].getElementsByTagName('param');
        for(i=0;i<objs.length;i++){
            id = objs[i].getAttribute('name');
            if (id == 'src'){
                source = objs[i].getAttribute('value')
                dv = document.createElement('div');
                dv.setAttribute('id','myContent');
                dv.setAttribute('border','2');

               document.getElementsByTagName('body')[0].appendChild(dv);
               /* code to embed a new swf object in new Div*/

            }/*if*/
        }/*for*/
    }/*outer for*/
 }/*function*/

here is the HTML part:

<body>


<div id="old">
</br><h1> Hello </h1>

<object id="myId1" width="250" height="250" name="movie1" classid="clsid:d27cdb6e-ae6d-    11cf-96b8-444553540000" codebase="http://fpdownload.macromedia.com/pub/shockwave/cabs/flash/swflash.cab#version=8,0,0,0"><param name="src" value="whitebird.swf">
</object>
<h1> welcome to flash</h1>

</div>
</br>
<input type="button"  onclick="get2()" value="Insert"/>
</body>
Anna
  • 23
  • 4
  • The variables id, i, and j here are global. You probably don't want that. – dyoo Aug 20 '11 at 18:54
  • I tried making i and j local, it didnt work. – Anna Aug 20 '11 at 19:01
  • Your param tag isn't closed, and your br tags should have the slash at the end:
    – Jason Goemaat Aug 20 '11 at 19:01
  • Here what is happening is, Document.getElementsByTagName('object') is identifying one more object at each loop of For. Though the statement is outside the loop, its fetching the newly created object also. Hence its leading me in to an infinite loop of embedding the swf object.Can any one please help me why the stmt outside forloop is getting me the count of new object! – Anna Aug 20 '11 at 19:05
  • You want to fix that problem anyway. Even though I suspect it's not immediately related to the problem you're seeing, it still needs to be fixed. If you ever write another function that also coincidentally uses global variable i and j, you will not be happy. – dyoo Aug 20 '11 at 19:08
  • If you are adding object tags in your loop that would be the case. Also element ids should be unique. – Jason Goemaat Aug 20 '11 at 19:09
  • Yup, I understood your suggestion, i will keep the variables local and set div id along with loop index. I am experimenting with js, really learning good skills now! – Anna Aug 20 '11 at 19:23

3 Answers3

2

Put a breakpoint in your inner for loop and inspect the values of j, i, and getElementsByTagName('object').length as well as swf[j].getElementsByTagName('param').length after the first iteration and post the results. Try getting your length outside the loop (which is good practice anyway). Also use the var keyword in your loops:

var objectLength = swf.length;
for (var j = 0; j < objectLength; j++)
{
     ...
}

You aren't adding more object tag in your div and just omitted that code are you?

Jason Goemaat
  • 28,692
  • 15
  • 86
  • 113
  • yes I did omitted the embed code! Thanks a ton Jason, you saved my day! It works, no infinite loop now! I appreciate your help! have a great weekend :) – Anna Aug 20 '11 at 19:12
2

I think the issue is that you're looping on the length of a dynamic array returned from getElementsByTagName('object') and then inside that loop you're adding a new object tag which will increase the array which will cause the loop to extend again which will add another object which will extend the loop again - forever. You can fix that by not looping on .length, but by getting the length of the initial array and only looping on that. Here are other things I suggest you fix too.

  1. Use local variables, not global variables.
  2. When you use getElementsByTagName, they return dynamic arrays that can change in length as you manipulate things. It's safer to loop on the initial length and thus never have infinite loop risk.
  3. You can use document.body instead of document.getElementsByTagName('body')[0].
  4. Some semi-colons missing.

Here's fixed up code that has these fixed/protections in it.

function get2(){
    var swf = document.getElementsByTagName('object');
    for (var j = 0, len = swf.length; j < len ; j++ )
    {
        var objs = swf[j].getElementsByTagName('param');
        for(var i=0, oLen = objs.length; i < oLen; i++){
            var id = objs[i].getAttribute('name');
            if (id == 'src'){
                var source = objs[i].getAttribute('value');
                var dv = document.createElement('div');
                dv.setAttribute('id','myContent');
                dv.setAttribute('border','2');

               document.body.appendChild(dv);
               /* code to embed a new swf object in new Div*/

            }/*if*/
        }/*for*/
    }/*outer for*/
 }/*function*/

When using any of these dynamic arrays, I always prefetch the length of the array into a local variable and loop on that to avoid every having this situation.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
1

There are two things here that make me suspicious.

  • The only thing that looks really suspicious to me is the following line:

    document.getElementsByTagName('body')[0].appendChild(dv);
    

    You're modifying the DOM here. What's this element that you're modifying here?

  • The structure of your outer loop:

    for (j=0; j<swf.length; j++) { ... }
    

    Its termination condition presumes that swf.length doesn't change.

There is a particular situation I can imagine where these two things can clash. What if you end up appending each new div right into the SWF element?

dyoo
  • 11,795
  • 1
  • 34
  • 44
  • I am adding a div under and embedding a swf with in it. It works, only thing was it was looping infinitely, now With Jason's help its fixed. Thank you for your views! – Anna Aug 20 '11 at 19:16
  • Ah. Yes, that makes sense. So that's what "/* code to embed a new swf object in new Div*/" is. Yeah, getElementsByTagName is live. So every SWF that you added in automatically became part of the collection you were iterating over. See: https://developer.mozilla.org/en/DOM/element.getElementsByTagName – dyoo Aug 20 '11 at 19:19