pangea33 wrote:
<snip>
function changeLinks(selAttributes, assignVal){
var aAttribNames = selAttributes.split(',');
It is recommended that code not be posted indented with tabs. Sequences
of spaces should be used instead (preferably not that many per tab). The
width of a tab on the viewer's set-up is capable of varying
considerably, from zero to equivalents of 8 or more spaces (which is far
too many for each level of indentation when posted to news, where line
wrapping at 80ish characters is not unusual).
var alltags=document.getElementsByTagName("a") ?
document.getElementsByTagName("a") : document.all;
That is a very bizarre line of code. Instead of making a decision based
upon whether or not the browser environment supports a -
document.getElementsByTagName - it is making its decision on whether or
not a call to - document.getElementsByTagName('a') - returns an object
or not. But - document.getElementsByTagName - is specified as always
returning a NodeList, even if it may be an empty NodeList. Thus the -
document.all - alternative will never be taken, and if an environment
does not provide - document.getElementsByTagName - the attempt to call
that method will have the4 code error-out at that point ensuring that -
document.all - cannot serve as an alternative for older browsers.
for (i=0; i<alltags.length; i++){
Using a loop counter that has not been declared as a function local
variable is a dangerous habit. The veritable effectively becomes global
and when that error is made in one location it is often repeated in
another, with the consequence that if code in a loop calls another
function that runs another loop using the same global counter variable
the counter in the 'outer' loop will have the wrong value when the
called function returns. The general programming axiom that no variable
should ever be given more scope than it absolutely needs applies to
javascript as much as it does to any other language (even if the
scopeing units in javascript are as coarse as being at the function
definition level).
// this modifies every link
for (j=0; j<aAttribNames.length; j++){
if (alltags[i].href && alltags[i].href.length) {
Given the number of times - alltags[i] - is to be resolved in the
following code it would be a good idea to assigned a reference to the
element to a local variable. That is, with:-
var link;
- declared at the top of the function (at the top purely for style
reasons), here inside the loop the assignment:-
link = alltags[i];
- would avoid having to resolve the - i - property of the - alltags -
collection repeatedly.
When you test - alltags[i].href - the value of that property is
type-converted to boolean. If the element has no - href - property that
value is Undefined, which type-converts to boolean false. However, when
the element has an - href - property that property is a string
primitive. Empty string primitives (those with a length of zero)
type-convert to boolean false, while non-empty strings type-convert to
boolean true. This mans that the following - alltags[i].href.length - is
utterly redundant as its value is numeric and the number zero
type-converts to false while all non-zero (and non-NaN, which string
lengths cannot be) type-convert to true. Whenever the second test would
evaluate to false the first test had already evaluated to false,
short-circuiting the logical AND expression and preventing the
evaluation of the second expression.
eval("alltags[i].style."+aAttribNames[j]+"='"+assignVal+"'");
There is almost no need to ever use - eval - in javascript, particularly
with property accessors. Generally the use of - eval - with dot-notation
property accessors follows from an ignorance of bracket notation
property accessors. See:-
<URL:
http://jibbering.com/faq/faq_notes/square_brackets.html >
The equivalent (but shorter and faster) code without the - eval - would
be:-
alltags[i].style[aAttribNames[j]] =' assignVal;
(or, given the proposed use of a local reference to - alltags[i] -:-
link.style[aAttribNames[j]] =' assignVal;
)
}
}
//changeLinks('linkColor', '#FF0000');
//only modify links with an id set
if(alltags[i].id) {
if (alltags[i].id.substr(0,alltags[i].id.length)=='IDOFTHEFORM'){
if (alltags[i].innerHTML && alltags[i].innerHTML.length) {
//document.getElementById('statusDiv').innerHTML+=al ltags[i].innerText;
if (alltags[i].canHaveChildren) {
for (j=0; j<alltags[i].childNodes.length; j++){
Another undeclared loop counter.
if(alltags[i].childNodes[j].nodeName == 'IMG') {
//alert(alltags[i].childNodes[j].getAttribute('src'));
alltags[i].childNodes[j].src =
'http://127.0.0.1/myspace/images/767235372_s.jpg';
var thingie = "alltags[i]";
This is another odd thing to be doing as it was always possible to
assign the value of - alltags[i] - to a variable.
while (eval(thingie+".parentElement.tagName") != "HTML" ) {
But that was being done to allow this second needless use of - eval -.
If the - link = alltags[i]; - assignment had been made then the - eval -
could have been - eval('link.parentElement.tagName') -, which instantly
reveals why it is pointless, as it can be directly replaced with -
link.parentElement.tagName - to the same effect (but again shorter and
faster), though - alltags[i].parentElement.tagName - would also have
worked without the - eval -.
Incidentally, - parentElement - is a proprietary Microsoft property. The
DOM standard equivalent is - parentNode -. The most recent Microsoft
browser that did not support - paraentNdoe - was IE 4, and IE 4
errored-out at the - document.getElementsByTagName("a") - call because
it has no - document.getElementsByTagName - method.
thingie+='.parentElement';
document.getElementById('dumper').innerText+=eval( thingie+".parentNode.s
tyle.borderColor");
document.getElementById('dumper').innerHtml+='<br> ';
//alert(alltags[i].parentElement.tagName);
}
<snip>
Without the - eval - above the loop logic here becomes:-
while(link.parentNode.tagName != "HTML" ) {
link = link.parentNode;
document.getElementById('dumper').innerText +=
link.parentNode.style.borderColor;
document.getElementById('dumper').innerHtml += '<br>';
}
- and another unnecessary - eval - is avoided.
Richard.
