removing nodes in the window.opener

[Ole Noerskov]

The function below is supposed to remove all childnodes with a name
that starts with "keywords" in "myform" in the window.opener.

It works fine if there's one keyword node. But you have to run the
function several times if there are many keyword nodes.
Why?

function removeKeywords()
{
var form_obj = window.opener.document.getElementById('myform');
var num_of_elem = form_obj.children.length;
var count;
for (count = 0; count<num_of_elem;count++)
{
var name = form_obj.children[count].name;
if (name.match(/^keywords.*/))
{
form_obj.children[count].removeNode();
}

}
}

 

[Ivo]

The function below is supposed to remove all childnodes with a name
that starts with "keywords" in "myform" in the window.opener.
It works fine if there's one keyword node. But you have to run the
function several times if there are many keyword nodes.
Why?

Because the form elements array is shortened and any remaining elements have
their index numbers updated (become one less) when you remove an element. If
you remove element number 16, number 17 becomes 16, so you want to check 16
again.

function removeKeywords()
{
var form_obj = window.opener.document.getElementById('myform');
var num_of_elem = form_obj.children.length;
var count;
for (count = 0; count<num_of_elem;count++)
{
var name = form_obj.children[count].name;
if (name.match(/^keywords.*/))
{
form_obj.children[count].removeNode();

count--; // add this line here

}
}
}

HTH
Ivo

 

[Thomas 'PointedEars' Lahn]

"Ole Noerskov" wrote

The property is "childNodes", not "children". "children"
may work in IE, but it is not standards compliant. To
support older IEs, you could write

var children = form_obj.childNodes || form_obj.children;
if (children)
{
var num_of_elem = children.length;
...
}

But then you also need

var formObj = window.opener.document.forms['myform'];

instead previously, as IEs below 5.0 do not support the
W3C DOM, only DOM Level 0 and the IE4 DOM, if that.

That is unnecessary, unless you intend to use `num_of_elem'
and `count' again in the function. Otherwise write

for (var count = 0, num_of_elem = form_obj.childNodes.length;
count < num_of_elem;
count++)

instead.

{
var name = form_obj.children[count].name;

That is not good. You redeclare the variable in every loop.
I recommend to declare a variable holding the object reference
in the initialization part of the `for' statement:

for (var count = 0, e = form_obj.childNodes,
num_of_elem = form_obj.childNodes.length,
sName = ""; // see below
count < num_of_elem;
count++)


Since you seem not evaluate the matches, RegExp.test() is sufficient
and it returns a boolean value which avoids paying for type casting.

{
form_obj.children[count].removeNode();

count--; // add this line here

Considering the above, write

if ((sName = e[count].name)
&& /^keywords.*/.test(sName))
{
form_obj.removeChild(e[count--]);
}

instead. AFAIS removeNode() is proprietary[1] (there is apparently a
typo in the Document interface specification of the W3C DOM Level 2
Specification, as the Node interface has no such method that could be
inherited[2]. It is no longer there in W3C DOM Level 3 Core.[3])


PointedEars
___________
[1]
<http://msdn.microsoft.com/workshop/author/dhtml/reference/methods/removenode.asp>
[2] <http://www.w3.org/TR/DOM-Level-2-Core/core.html#i-Document>
[3] <http://www.w3.org/TR/DOM-Level-3-Core/core.html#i-Document>

 

[Richard Cornford]

{
var name = form_obj.children[count].name;

That is not good. You redeclare the variable in every loop.

Because ECMAScript is not block scoped and performs variable
instantiation for a function once, as it enters an execution context, it
would not be correct to say "redeclare the variable in every loop". The
local variable is declared in code within a loop, but that declaration
is handled at a function level and only happens once, prior to the
execution of the function body.

Following variable instantiation, as the function is executing, the only
significance of that statement is as an assignment, which is what is
wanted. The - var - keyword has already been employed to create a
property of the execution context's Variable object with the name "name"
and that process will not be repeated no matter how often the (or any)
statement(s) in which the - var - keyword is used is(are) executed.

I recommend to declare a variable holding the object reference
in the initialization part of the `for' statement:

<snip>

Good style may suggest that variables be declared in a block at the top
of a function definition, or in the initialisation part of a - for -
statement, but the location of variable declarations makes no actual
difference to how ECMAScript executes. Which means that if a criteria of
compact code was applied, instead of a notion "correct style", then
first explicitly declaring a local variable in the first statement that
made an assignment to it may save a few bytes of download without making
any difference to how the code executed.

Richard.

 

[Thomas 'PointedEars' Lahn]

"Ole Noerskov" wrote

{
var name = form_obj.children[count].name;

That is not good. You redeclare the variable in every loop.

Because ECMAScript is not block scoped and performs variable
instantiation for a function once, as it enters an execution context, it
would not be correct to say "redeclare the variable in every loop". The
local variable is declared in code within a loop, but that declaration
is handled at a function level and only happens once, prior to the
execution of the function body.

You should tell the people of mozilla.org. Maybe they can explain why

| var x = 42;
| for (var i = 0; i < 13; i++)
| {
| var x = i;
| }
| for (var i = 0; i < 13; i++)
| {
| x = i;
| }

yields

| Warning: redeclaration of var x
| Source File: javascript:var x = 42; for (var i = 0; i < 12; i++) { var x =
| i; } for (var i = 0; i < 12; i++) { x = i; }
| Line: 1, Column: 47
| Source Code:
| var x = 42; for (var i = 0; i < 12; i++) { var x = i; } for (var i = 0;
| i < 12; i++) { x = i; }
|
| Warning: redeclaration of var i
| Source File: javascript:var x = 42; for (var i = 0; i < 12; i++) { var x =
| i; } for (var i = 0; i < 12; i++) { x = i; }
| Line: 1, Column: 65
| Source Code:
| var x = 42; for (var i = 0; i < 12; i++) { var x = i; } for (var i = 0;
| i < 12; i++) { x = i; }

in Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040509
and Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8a) Gecko/20040513
Firefox/0.8.0+.


PointedEars

 

[Richard Cornford]

Richard Cornford wrote:

You should tell the people of mozilla.org.

Why should I? They can read the specification for themselves.

Maybe they can explain why

| Warning: redeclaration of var x

| Warning: redeclaration of var i

<snip>

If Mozilla's JS engine is re-declaring the variable whenever it executes
the loop body then it is not an ECMA 262 3rd edition conforming
implementation (which would be poor from an organisation that likes to
play on its support for standards).

But the warnings generated by Mozilla (at least when it is set to
produce strict warnings) are not appropriate for cross browser scripting
so it would probably be better to turn them off. That way Mozilla at
least behaves as if its javascript implementation was ECMA 262
conforming (which is good enough).

Richard.

 

[Matt Kruse]

You should tell the people of mozilla.org. Maybe they can explain why

I'm not sure this is anti-standards, as Richard suggests. It may just be a
general warning that could very well be helpful.
For example, if you have a .js include file which does

var totalCount=4; //or whatever

and then in your page-level script source, you have another declaration like

var totalCount=data.length;

then the two would definitely collide, and it would be helpful to know about
that.
Declaring 'var' in more than one place makes the source more confusing, too,
since obviously it's only needed at the top of a given function.

As long as mozilla isn't actually treating the x variable inside the loop as
something different than the x declared as 42, I'd say that's a helpful
warning message.

 

[Lasse Reichstein Nielsen]

You should tell the people of mozilla.org. Maybe they can explain why

| var x = 42;
| for (var i = 0; i < 13; i++)
| {
| var x = i; ....

yields

| Warning: redeclaration of var x

Because you are declaring the same variable twice in the same scope.
The warning is not related to one of the declarations being inside
a loop.

In ECMAScript, it is not illegal to declare the same variable more
than once, which is why it gives a warning, not an error. I expect
that they give a warning, because declaring a variable more than once
gives no extra benefit, and is likely to be a mistake.

/L

 

[Richard Cornford]

Because you are declaring the same variable twice in the same scope.
The warning is not related to one of the declarations being inside
a loop.

<snip>

I didn't really look at Tomas' s code, I rather foolishly assumed that
his example would mirror the code that was the subject of his comments,
which did not have multiple declarations of the same variable. Under the
circumstances the warning is reasonable (as a warning), and much as I
would expect from any lint program. Unfortunately, knowing that Mozilla
issues bogus warnings leaves me more inclined to consider any warnings
it generates as suspect.

Richard.

 

[Thomas 'PointedEars' Lahn]

You should tell the people of mozilla.org. Maybe they can explain why
| var x = 42;
| for (var i = 0; i < 13; i++)
| {
| var x = i; ...

yields

| Warning: redeclaration of var x

Because you are declaring the same variable twice in the same scope.
The warning is not related to one of the declarations being inside
a loop. [...]

Full ACK. Somehow I overlooked that all the time. Thanks!


PointedEars

 

[Thomas 'PointedEars' Lahn]

Because you are declaring the same variable twice in the same scope.
The warning is not related to one of the declarations being inside
a loop.

<snip>

I didn't really look at Tomas' s code, I rather foolishly assumed that
his example would mirror the code that was the subject of his comments,
which did not have multiple declarations of the same variable. [...]

Let him who is without bug cast the first byte ...


PointedEars

P.S.: It's *Thomas*.

 

[Richard Cornford]

Richard Cornford wrote:

Let him who is without bug cast the first byte ...

P.S.: It's *Thomas*.

Yes, sorry, its a typo (two actually).

Richard.

 

[Ole Noerskov]

As usual I found the solution myself before my message even appeared
here:
Before deleting any nodes I first read in the names or ids in an array
- then I use a loop and getElementByName or getElementById to remove
the nodes.

Thanks anyway.
It's a little bit funny that most of the replies focus on the
javascript style - not my concrete problem. The script was written
very quickly just to give an example.

Ole Noerskov