eval javascript function IE6 taking long time

Question

I have the below chunk of code. I've debugged through and located the snippet that is causing a long delay in IE6.

Basically the code loops through a document converting it to XML and sending to a PDF. On Ubuntu and Firefox 4 it takes 3 seconds. On IE it can take up to 40 seconds regularly.

/**
* This function builds up the XML to be saved to the DM.
*/
function getXMLToSave(){

var text="<workbook><sheet><name>Adv4New</name>";

//show_props(document.adv4.row10col1, "document.adv4.row10col1");

for(i=1;i<157;i++){
    text = text + "<row number='" + i + "'>";
    for(j=1;j<=7;j++){
        text = text + "<col ";
        //alert(eval('document.adv4.row'+i+'col'+j+'.readonly'));
        try{
            text = text + "number='" + j + "' label='" + eval('document.adv4.row'+i+'col'+j+'.className')+ "'";
        }
        catch (e) {
            text = text + "number='" + j + "' label=''";
        }
        try {
            if(eval('document.adv4.row'+i+'col'+j).readOnly)
            text = text + " type='readonly'";
            else
            text = text + " type=''";
        }
        catch (e) {
            text = text + " type=''";
        }
        try {
            text = text + " color='" + eval('document.adv4.row'+i+'col'+j+'.style.color') + "'";
        }
        catch (e) {
            text = text + " color=''";
        }
        text = text + ">";
        try {
            // don't wrap in a CDATA (like previously), but run cleanNode
            // this fixes html entities
            var content = eval('document.adv4.row'+i+'col'+j+'.value');
            text = text + cleanNode(content);
        }
        catch (e) {
            text = text + "0";
        }
        text = text + "</col>";
    }
    text = text + "</row>";
}
text = text + "</sheet></workbook>";

return text;

}

I believe its the eval function causing the delay in IE6. Is there a neat solution to fix this. Thanks very much

Supporting a browser that MS does not even support? http://www.ie6countdown.com/ — epascarello, Oct 01 '12 at 14:13
What exactly is it that your eval is trying to accomplish? It looks like you could just as easily access the color property by doing this: document.adv4['row' + i + 'col' + j].style.color — John Krull, Oct 01 '12 at 14:18

score 3 · Answer 1 · answered Oct 01 '12 at 14:16

3

Why are you using eval in the firts place?

eval('document.adv4.row'+i+'col'+j+'.style.color')

Use bracket notation!

document.adv4["row"+i+"col"+j].style.color

answered Oct 01 '12 at 14:16

epascarello

204,599
20
195
236

score 3 · Answer 2 · answered Oct 01 '12 at 14:16

You don't need eval() at all:

    text = text + "number='" + j + "' label='" + document.adv4['row' + i + 'col' + j].className + "'";

Also, in IE6 (but not in newer browsers), building up large strings by repeatedly adding more content is really, really slow. It was way faster in that browser to build up strings by creating an array of substrings and then joining them all together when finished with all the pieces.

score 2 · Answer 3 · answered Oct 01 '12 at 14:17

2

Don't use eval EVAL is EVIL. Having said that, you really shouldn't care about IE6: Even MS doesn't support it any longer, why should you bother?

Anyhow, change all eval calls like:

eval('document.adv4.row'+i+'col'+j+'.value');

to

document.adv4['row' + i + 'col' + j].value;

To access the elements directly. Remember that Nodes are objects, so their properties can be accessed either using the dot-notation (foo.bar) or the "associative array" notation: foo['bar'], the latter being very useful when you need the value of a variable to access properties

answered Oct 01 '12 at 14:17

Elias Van Ootegem

74,482
9
111
149

thanks. I'm presuming this combined with the below text array worked :) – topcat3 Oct 01 '12 at 15:18
Sure, this is just the faster (and safer) way to get property values, what you do with those strings is irrelevant. Not for your script, but as far as my suggestion goes. Concatenating is indeed slower, didn't think about that :)... use the bracket notation and join the array in the end, you should notice a difference – Elias Van Ootegem Oct 01 '12 at 15:32

score 1 · Answer 4 · answered Oct 01 '12 at 14:16

1

Don't use eval - period. The eval() should be renamed to evil(). There is almost no situation where you really need to use the eval function.

In this case you can use document.getElementById() to find a DOM node with a specific id.

answered Oct 01 '12 at 14:16

Philipp

67,764
9
118
153

score 0 · Answer 5 · answered Oct 01 '12 at 14:15

0

One solution could be generating a color array (or maybe an object if you need it) and then using it.

But then, ask yourself the question "Should I really support IE6?"

answered Oct 01 '12 at 14:15

alexandernst

14,352
22
97
197

score 0 · Accepted Answer · answered Oct 01 '12 at 14:25

It's likely that it's all the string concatentation that makes it slow. Each time you add something to the text, it will copy all the previous text into a new string.

Newer browsers have optimised code for this special case, so for them the impact is less.

Instead of concatenating strings like this:

text = text + "something";

use an array instead:

var text = [];

then add items to the array using the push method:

text.push("<workbook><sheet><name>Adv4New</name>");

Finally just join the strings together:

return text.join('');

do I need to take out all text = text parts ? I tried just adding in the array and the bracket solutions above but still same time delay. I'm presuming I need to do text.push the whole way replacing text = text ? — topcat3, Oct 01 '12 at 14:55

eval javascript function IE6 taking long time

6 Answers6