469,646 Members | 1,693 Online
Bytes | Developer Community
New Post

Home Posts Topics Members FAQ

Post your question to a community of 469,646 developers. It's quick & easy.

removeChild skips on odd nodes, why?

VK
I must be missing something very obvious, but my nightly head doesn't
work anymore.

Press "Insert" button to add <insnodes after each <br>.
Now press "Delete" - only even <insare being removed.
ins.length is reported properly, each <inshas "insert" class name.
What a...?

<html
><head
<title>Demo</title
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1"
><style>
..insert {
background-color: #FFFF00;
color: #FF0000;
text-decoration: none;
padding-left: 2px;
padding-right: 2px;
}
</style
><script>
function ins() {
var br = document.body.getElementsByTagName('br');
var ms = document.createElement('ins');
ms.appendChild(document.createTextNode('Message')) ;
ms.className = 'insert';
for (var i=0; i<br.length; i++) {
br[i].parentNode.insertBefore(ms.cloneNode(true),
br[i].nextSibling);
}
}
function del() {
var ins = document.body.getElementsByTagName('ins');
for (var i=0; i<ins.length; i++) {
if (ins[i].className == 'insert') {
alert( ins[i].parentNode.removeChild(ins[i]) );
}
}
}
</script
></head
><body
><p>Demo <brtext</p
><p>Demo <brtext</p
><p>Demo <brtext</p
><p>Demo <brtext</p
><p><button type="button" onclick="ins()">Insert</button>
<button type="button" onclick="del()">Delete</button></p
></body
</html>
Feb 8 '07 #1
6 1961
VK wrote:

Hi VK,
Press "Insert" button to add <insnodes after each <br>.
Now press "Delete" - only even <insare being removed.
The DOM tree is live, when removing some elements from it you have to
make sure that your iterator is updated as well.
alert( ins[i].parentNode.removeChild(ins[i]) );
alert( ins[i].parentNode.removeChild(ins[i--]) );
Regards,
Elegie.
Feb 8 '07 #2
VK
alert( ins[i].parentNode.removeChild(ins[i]) );
>
alert( ins[i].parentNode.removeChild(ins[i--]) );
Thanks for the hint, this brought the code to life:

<html
><head
<title>Demo</title
<meta http-equiv="Content-Type"
content="text/html; charset=iso-8859-1"
><style>
..insert {
background-color: #FFFF00;
color: #FF0000;
text-decoration: none;
padding-left: 2px;
padding-right: 2px;
}
</style
><script>
function ins() {
var br = document.body.getElementsByTagName('br');
var ms = document.createElement('ins');
ms.appendChild(document.createTextNode('Message')) ;
ms.className = 'insert';
for (var i=0; i<br.length; i++) {
br[i].parentNode.insertBefore(ms.cloneNode(true),
br[i].nextSibling);
}
}
function del() {
var ins = document.body.getElementsByTagName('ins');
for (var i=ins.length-1; i>=0; i--) {
if (ins[i].className == 'insert') {
ins[i].parentNode.removeChild(ins[i]);
}
}
}
</script
></head
><body
><p>Demo <brtext</p
><p>Demo <brtext</p
><p>Demo <brtext</p
><p>Demo <brtext</p
><p><button type="button" onclick="ins()">Insert</button>
<button type="button" onclick="del()">Delete</button></p
></body
</html>
I'm still missing I'm affraid how switching iteration direction would
solve the problem - but it obviously did. Maybe I'll get some enlight
after a good sleep... :-)

Feb 8 '07 #3
VK wrote:
I'm still missing I'm affraid how switching iteration direction would
solve the problem - but it obviously did. Maybe I'll get some enlight
after a good sleep... :-)
Rest, sport and good meals are three of the key success factors that,
IMHO, help a person get through intense stress situations that tend to
last for many days. Cheers up, things will clear up :)

Consider what removeChild does: it removes one of the object you're
iterating on. Your index, which refers to this object, now points to the
next object. Then, your "for loop" continues and increases the index by
one *while it is already pointing to the next node*, resulting in
skipping the node it had been referring to after the removal.

Ex.:

Given the following nodes: [A] [b] [C] [D]
Given an ii index, with ii==1, i.e. points to [b]
Removing [b] gives: [A] [C] [D]
Your index still equal to 1, ii==1, means it now points to [C]
The "for" statement increases ii to ii==2, i.e. points to [D]
[C] has been skipped in the process.
HTH,
Elegie.

PS : BTW VK your code is not displayed correctly when you post, ">"
marks appear at the beginning of lines, making my newsreader think
you're quoting stuff.
Feb 8 '07 #4
On Feb 8, 9:47 pm, "VK" <schools_r...@yahoo.comwrote:
[...]
function del() {
var ins = document.body.getElementsByTagName('ins');
for (var i=ins.length-1; i>=0; i--) {
if (ins[i].className == 'insert') {
ins[i].parentNode.removeChild(ins[i]);
}
}
You might find it easier to iterate backwards using a while loop:

var n, i = ins.length;
while (n = ins[--i]) {
if (n.className == 'insert') {
n.parentNode.removeChild(n);
}
}
If you don't like assignment inside the while test:

while ( i-- ) {
n = ins[i];
if ...
}

There are many ways to skin a cat...

[...]
I'm still missing I'm affraid how switching iteration direction would
solve the problem - but it obviously did. Maybe I'll get some enlight
after a good sleep... :-)
The NodeList returned by getElementsByTagName is live. When you
remove ins[i] then the node that was ins[i+1] is now ins[i].
Incrementing the counter on then next loop skips that node to the one
that was ins[i+2].

When you work backwards (i.e. from highest to lowest index), inserting
or removing items at or beyond the current index list doesn't affect
iteration over all the original nodes.
--
Rob
Feb 8 '07 #5
VK
On Feb 8, 4:01 pm, "RobG" <r...@iinet.net.auwrote:
You might find it easier to iterate backwards using a while loop:

var n, i = ins.length;
while (n = ins[--i]) {
if (n.className == 'insert') {
n.parentNode.removeChild(n);
}
}
Agreed.
The NodeList returned by getElementsByTagName is live. When you
remove ins[i] then the node that was ins[i+1] is now ins[i].
Incrementing the counter on then next loop skips that node to the one
that was ins[i+2].

When you work backwards (i.e. from highest to lowest index), inserting
or removing items at or beyond the current index list doesn't affect
iteration over all the original nodes.
Thanks to everyone, now I am clear on it.

getElementsByName / getelementsByTagName returns NodeList object (also
called HTMLCollection in older docs). In my coding I wrongly assumed
that NodeList is a kind of stripped down Array - without Array methods
and ability to store proprietary data, only with ability to iterate
using positive integer index.

In fact NodeList is Vector data type, so it automatically "shrinks" on
element removal with corresponding index renumbering of remaining
elements.

This way the safe way to remove elements is by going from the top to
bottom: in this case there is no index shift of remaining elements.

What wonders me now: one cannot directly remove elements from
NodeList, and a statement like
elm[i].parentNode.removeChild(elm[i])
changes the current NodeList in an indirect way (as a reflection of
DOM Tree change).

Does it mean that NodeList collection re-retrieved over and over
before any involving operation?

Feb 8 '07 #6
VK
On Feb 8, 3:17 pm, Elegie <ele...@invalid.comwrote:
Rest, sport and good meals are three of the key success factors that,
IMHO, help a person get through intense stress situations that tend to
last for many days. Cheers up, things will clear up :)
Thanks :-)
PS : BTW VK your code is not displayed correctly when you post, ">"
marks appear at the beginning of lines, making my newsreader think
you're quoting stuff.
Yeah. Normally I shape up my code before "going on public" (Usenet
posting): type="", conventional pretty-print and stuff. Last night I
was too tired for that I guess. This is anti-phantom pretty-print my
authoring software makes. Gives about 10% time gain on client-side
parsing stage for Gecko engines - because it doesn't have to allocate
bogus text nodes for line breaks - and up to 40% for intensive tree
traversal scripting - because of using native DOM methods without
extra checks for phantom nodes. Once I compared the productivity
impact - since then I'm using just that.
But I promise do not confuse in the future news agents and news
readers :-)

Feb 8 '07 #7

This discussion thread is closed

Replies have been disabled for this discussion.

Similar topics

2 posts views Thread by bulk88 | last post: by
5 posts views Thread by Markus Stehle | last post: by
1 post views Thread by Philipp Schumann | last post: by
2 posts views Thread by Pete | last post: by
1 post views Thread by Rebecca Tsukalas | last post: by
5 posts views Thread by Meelis Lilbok | last post: by
By using this site, you agree to our Privacy Policy and Terms of Use.