onchange attribute won't call function

StackOverflow https://stackoverflow.com/questions/1799936

  •  22-09-2019
  •  | 
  •  

Question

I have an HTML document (here), which creates an iframe-based media player for a collection of songs within albums (I just used letters to define these albums and songs in the mymusic array, for simplicity).

Focusing on the top 3 iframes, the way I have set out the user interaction is to generate the HTML for forms of available albums and songs using Javascript, and write them to the iframes in the body. If you run it and make a selection in the Albums menu, you will see that the options in the Songs menu correspond with the mymusic array, so this works.

However, when I choose a song, the function nowplaying(trackindex,albumindex) should be called using an onchange event in the Songs form, the same way as in the form generated using showinitial() ... but the function does not get called.

I have ruled out the coding of nowplaying itself as a cause, because even when I change nowplaying to alert("hello"), it does not get called. So this leads me to think the problem is with the onchange attribute in "anything", but I can't see the problem. The way I coded it is no different to before, and that worked fine, so why won't this work?

Any help would be much appreciated!

Was it helpful?

Solution

Firebug is your friend....

i is not defined

function onchange(event) { parent.nowplaying(this.SelectedIndex, i); }(change )

onchange is getting called, but i is not defined when calling nowplaing.

This is the result of this line:

p+="<html><head></head><body><form><select onchange='parent.nowplaying(this.SelectedIndex,i);' size='";

which is using "i" in the string, when it should append it as a variable:

p+="<html><head></head><body><form><select onchange='parent.nowplaying(this.SelectedIndex," + i + ");' size='";

To clarify, i is defined when anything(i) is called, but you aren't writing i into the code, just the letter i. When nowplaying(this.SelectedIndex,i) is called, i is no longer defined, because you aren't inside of the anything() function anymore. You need to expand i when you append the html to p, so that the value is there and not the variable i.

OTHER TIPS

function anything(i){
    p+="...<select onchange='parent.nowplaying(this.SelectedIndex,i);'...";

Your onchange event handler is set from a string. When run, it will not have access to i, which is a local variable from the anything function that has long since gone away.

The simple fix would be:

    p+="...<select onchange='parent.nowplaying(this.SelectedIndex,'+i+');'...";

which turns the current value of i at string-making time into an integer literal inside the string.

However, it's not generally a good idea to be creating code from strings. It's normally better to write the event handler as a normal function object:

// You will need the below workaround to get the iframe document in IE too
//
var iframe= document.getElementById('songs');
var idoc= 'contentDocument' in iframe? iframe.contentDocument : iframe.contentWindow.document;

idoc.open();
idoc.write(s);
idoc.close();

idoc.getElementsByTagName('select')[0].onchange= function() {
    // This is a closure. The 'i' variable from the parent 'anything' function is
    // still visible in here
    //
    parent.nowplaying(this.selectedIndex, i);
};

However you would generally want to avoid setting handlers from one frame on a different one. I'm not really sure what the iframes are gaining you here other than headaches. Why not just simply use positioned divs with overflow? You can still rewrite their content through innerHTML if you need to... though I would prefer to populate them using DOM methods, to avoid all the HTML-injection problems your current script has.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top