I posted one here a few days ago and no one replied.
You posted it yesterday. At a quick glance, I couldn't see what was wrong,
but after reformatting your code, it's quite clear, so in future please
indent code blocks. It's much easier to follow code that way. In addition,
post *all* the relevant HTML. You reference three elements by id, and many
more through the childNodes collection. How can we tell what's being
manipulated without the document?
[snip]
The code below is overwriting itself.
With very good reason. However, you might also like to know that it will
probably fail in most browsers, if you're doing what I think you're doing.
What it is doing is creating the fields correctly the first time through.
Then it writes over those same fields again and again. I can tell because
the value of the first field changes every loop.
That may be so, but you always insert the same node, which results in it
being removed, then being reinserted.
var ex_counter = 0;
var ex = "";
Why is 'ex' global?
function ex_morefields() {
var x=document.getElementById("choices");
ex_counter++;
var newfields = document.getElementById('ex_fields').cloneNode(true);
newfields.id = '';
newfields.style.display = 'block';
var newfield = newfields.childNodes;
for (y = 0; y<(x.length); y++) { //loop through all item in the list
What list?
This is why I said this code will fail. A conforming browser will only
return one node, or nothing at all, with a getElementById call. This means
that the length property will be undefined and the loop will never be
entered. You seem to be under the impression that it will return a
collection which implies a few things:
1) You've only tested on IE, which is far from a compliant browser.
2) You're using the same id on several elements.
3) You haven't validated you page before trying to script it.
The DOM Specification doesn't define specific behaviour with a document
contains elements with identical id attributes and a getElementById call.
This means that only the first element in document order might be
returned. It's even reasonable to assume that null can be returned. What
IE does - return a collection - is certainly not correct as the interface
for the method clearly defines what can be returned, and a NodeList is not
one of the options.
It's advisable to test on a decent browser before checking with IE. You're
more likely to find bugs that way. Opera and Mozilla are good options, and
are available on many platforms. Also validate:
ex = x[y].text;
for (var i=0;i<newfield.length;i++) {
var theName = newfield.name;
if (theName) {
newfield.name = theName;// + ex_counter;
Rather redundant, isn't it?
}
if (theName == "exercise") {
newfield.setAttribute('value', ex);
Use of setAttribute is usually unnecessary. If the element defines the
attribute as a property, as in the case of INPUT elements and the value
attribute, then
<object>.<property> = <value>;
is all that's required.
}
}
var insertHere = document.getElementById('write_EX');
Why do you repeatedly look up this reference?
insertHere.parentNode.insertBefore(newfields,insertHere);
This is the other issue. The newfields variable is only defined once
outside the outer loop, so it never changes. You probably meant
newfield, but the first problem is the most pressing as correcting it
might require you to re-examine all of this code, and probably the HTML,
too.
}
} // end add more software script
One final point: In all of the code above, you fail to check if the user
agent supports the methods and objects you use. Always feature test
(4.26).
<URL:http://jibbering.com/faq/>
[snip]
I'm sorry if that's a lot of information to throw at you, but it's all
important. If you need more help, you'll have to post your HTML and what
you're trying to accomplish. This would be best done by providing a link
to a demonstration.
Mike
