Laurence L Laurence L - 12 days ago 5
Javascript Question

EVAL dangerous How to replace it

I have the following code for a pop-up window (client request). It uses eval which I understand is dangerous. Is there a way to re-write the script below so it does not use (eval)?

/* exported popup_default , popup_help , popup_sitemap , popup_footerlinks */

var matchClass=['popup_default' , 'popup_sitemap','popup_footerlinks', 'popup_help'];
var newwindow = ''

var popup_default = 'width=800,height=640,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=250,top=200';
var popup_help = 'width=700,height=650,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=100,top=100';
var popup_sitemap = 'width=1000,height=600,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=100,top=100';
var popup_footerlinks = 'width=800,height=500,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=250,top=200';

function pop_ups(){
"use strict";
var x = 0;
var popClass;
while(x < matchClass.length){
popClass = "'."+matchClass[x]+"'";
$(eval(popClass)).click(function() {
var popurl = $(this).attr('href');
var popupSpecs = $(this).attr('class');
var popupName = Math.floor(Math.random()*10000001);
newwindow=window.open(popurl,popupName,eval(popupSpecs));
return false;
});
x++;
}
}

$(function() {
"use strict";
pop_ups();
});

Answer

You'll want to use

"use strict";
function makePopup(className, specs) {
    $('.'+className).click(function(e) {
        e.preventDefault();
        var popupName = Math.floor(Math.random()*10000001);
        window.open(this.href, popupName, specs);
    });
}
var popups = {
    popup_default:     'width=800,height=640,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=250,top=200',
    popup_help:        'width=700,height=650,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=100,top=100',
    popup_sitemap:    'width=1000,height=600,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=100,top=100',
    popup_footerlinks: 'width=800,height=500,toolbar=0,menubar=0,location=0,status=1,scrollbars=1,resizable=1,left=250,top=200'
};
$(function() {
    for (var key in popups)
        makePopup(key, popups[key]);
});

Instead of the first eval, just use string concatenation. Instead of the second eval, use an object to look up a property name.