dynamic select element: problem with onchange event

C

Covad

Hi all,

For some reason my change() function is only called when the page loads. I'd
much rather it gets called when the select changes.

Here's the code:

window.onload = init;

function init() {
var new_select = new Selector('tdata','myselect','myid');
var new_select_list = new DataSource("some_list");
new_select_list.addItem(1,"One");
new_select_list.addItem(2,"Two");
new_select_list.addItem(3,"Three");
new_select_list.addItem(4,"Four");
new_select_list.addItem(5,"Five");
new_select.setDataSource(new_select_list);
new_select.formInput("form","input");
}

Selector = function(container_id,name,id) {
var container = document.getElementById(container_id);
this.node = document.createElement("select");
//this.node = new Select();
container.appendChild(this.node);
this.node.name = name;
this.node.id = id;
}

Selector.prototype.setDataSource = function(ds) {
this.dataSource = ds;
for(var i = 0; i < ds.items.length; i++) {
if(ds.items != undefined) {
var option = new Option(ds.items,i,false,false);
this.node.options[this.node.options.length] = option;
}
}
}

Selector.prototype.formInput = function(form,element) {
var myform = document.getElementById(form);
this.input = document.createElement("input");
this.input.name = element;
//this.input.type = "hidden";
myform.insertBefore(this.input,myform.firstChild);
this.node.onchange = change(this);
}

function change(selector) {
alert("hello");
selector.input.value = selector.node.value;
}

DataSource = function(name) {
this.name = name;
this.items = new Array();
}

DataSource.prototype.addItem = function(id,item) {
this.items[id] = item;
}

and the html:

<html>
<head>
<script defer src="/javascript/selectors.js"
type="text/javascript"></script>
</head>
<body>
<form id=form>
<table border=1>
<tr>
<td>
Select:
</td>
<td id=tdata>
</td>
</tr>
</table>
</form>
</body>
</html>
 
R

Richard Cornford

Selector.prototype.formInput = function(form,element) {
var myform = document.getElementById(form);
this.input = document.createElement("input");
this.input.name = element;
//this.input.type = "hidden";
myform.insertBefore(this.input,myform.firstChild);
this.node.onchange = change(this);
}
<snip>

Your specific problem is the line above. The - change - function is
only called once because that line calls the function and assigns the
return value of the change function (undefined) to the onchange property
of the node. To have an onchange event handling function called with a
change event you need to assign a reference to the function to the
onchange property of the SELECT element.

Unfortunately, you want the - change - function to reference its -
selector - parameter, but event handling functions are either passed a
reference to the event object (on browsers that follow the Netscape
pattern of event handling) or they get no parameter at all. There are
two approaches to associating a JavaScript object instance with an event
handling function. The first is to create a global reference to the
object so the event handling function can refer to the object via a
global identifier or property accessor, for example:-

function myObject(){
this.index = myObject.instances.length;
myObject.instances[this.index] = this;
this.selectEl = ...
...
}
myObject.prototype.setOnChange = function(){
this.selectEl.onchange =
new Function('change(myObject.instances['+this.index+']);');
}
myObject.instances = [];

- and many variations on the theme.

The second option is to use a closure to hold a reference to the
JavaScript object instance associated with an inner function assigned as
the event handler:-

function setOnChange(selectEl, objInstance){
selectEl.onchange = function(){
alert("hello");
objInstance.input.value = objInstance.node.value;
};
selectEl = null;
}

- and many variations on that. The closure option is probably easiest to
code but suffers from producing a garbage collection problem on all
(Windows at least) version of IE where the garbage collector cannot free
DOM elements or JavaScript objects if they form points in a circular
chain of references that includes both DOM elements and JavaScript
objects. For example, if a select element's onchange property refers to
a function that is an inner function that results in a closure and the
closure holds a reference to the select element then the chain of
references is circular and IE will not garbage collect any of the
objects involved, tying up memory until the browser is closed. The
example function above should not produce that problem itself as the
select element references the inner function and nulling the direct
reference to the select element leaves the closure only holding a
reference to the JavaScript object instance and that is not circular,
_but_ if the JavaScript object instance itself holds a reference to the
select element the circular reference would exist and memory leaks would
result unless positive action was taken to prevent it (such as using an
onunload event handling function to break the circular references at
some point).

Your code also suffers from making assumptions about the features of
environment in which it will be executing, calling various functions and
method without first verifying that they exist in the browsers. There
does not appear to be any reason for not verifying browser support,
possibly in the init function so that it only has to be done once.
Though it would be normal to also plan some path of clean degradation to
a functional UI in the absence of browser support for the required
features, and it looks like this code design is already to JavaScript
dependent for basic HTML functionality in the absence of browser
support. But at least testing the browser for support of the required
features would avoid confirming the user of an unexpected browser's
impression that the page author was incompetent by showing a series of
avoidable JavaScript error messages.

Richard.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,768
Messages
2,569,574
Members
45,048
Latest member
verona

Latest Threads

Top