Javascript eval() acting strange

Viewed 45

I am creating my own kind of Message Boxes (normal OK dialogs and also Yes/No dialogs) that can popup on my website. When the user clicks "OK" or "Yes/No", callbacks are called that are executed using eval(). The OK callback works correctly, but I can't seem to get the Yes/No ones to work.

I'll start by showing the OK messageBox code and then show the yesNoBox code.

function messageBox(msg, onClose = null) {
    var id = sessionStorage.getItem('messageIndex');
    if (id === null || id === 'undefined') id = "0";
    sessionStorage.setItem('messageIndex', parseInt(id) + 1);
    sessionStorage.setItem('mb' + id, msg);
    sessionStorage.setItem('mb' + id + "_onClose", onClose);
    
    var code = localStorage.getItem('mb_ok');
    if (code === null || code === 'undefined') request('GET ui/mb_ok.html?id=' + id, 'messageBoxResponse');
    else {
        var start = code.indexOf('mb_ok') + 'mb_ok'.length;
        var end = code.indexOf('"', start);
        var i = code.substring(start, end);
        code = code.replace('mb_ok' + i, 'mb_ok' + id);
        messageBoxResponse(code);
    }
}

request('GET ui/mb_ok.html?id=' + id', 'messageBoxResponse') uses WebSockets to retrieve the messageBox HTML. The server returns HTML with a unique messageBox id and calls messageBoxResponse().

function messageBoxResponse(response) {
    var start = response.indexOf('mb_ok') + 'mb_ok'.length;
    var end = response.indexOf('"', start);
    var i = response.substring(start, end);
    var id = parseInt(i);
    var msg = sessionStorage.getItem('mb' + id);
    $('#stage').append(response);
    $('#mb_ok' + id + '_body').html(msg);
    var onClose = sessionStorage.getItem('mb' + id + '_onClose');
    if (onClose !== null || onClose !== 'undefined') {
        $('#mb_ok' + id).on('destroyed', eval(onClose));
    }
    document.activeElement.blur();
}

Test code:

messageBox("This is my message box", () => { alert("BOX CLOSED!"); });

Message Box

And when I click "OKAY" I get:

Message Box closed alert

The YesNoBox code is virtually the same, but with both onYes and onNo callbacks.

function yesNoBox(msg, onYes, onNo) {
    var id = sessionStorage.getItem('mb_yesno_index');
    if (id === null || id === 'undefined') id = "0";
    sessionStorage.setItem('mb_yesno_index', parseInt(id) + 1);
    sessionStorage.setItem('mb_yesno' + id, msg);
    sessionStorage.setItem('mb_yesno' + id + '_onYes', onYes);
    sessionStorage.setItem('mb_yesno' + id + '_onNo', onNo);
    
    var code = localStorage.getItem('mb_yesno');
    if (code === null || code === 'undefined') request('GET ui/mb_yesno.html?id=' + id, 'yesNoBoxResponse');
    else {
        var start = code.indexOf('mb_yesno') + 'mb_yesno'.length;
        var end = code.indexOf('"', start);
        var i = code.substring(start, end);
        code = code.replace('mb_yesno' + i, 'mb_yesno' + id);
        yesNoBoxResponse(code);
    }
}

And the yesNoBoxResponse() function is almost the same, but with added checks to determine if yes or no was selected:

function yesNoBoxResponse(response) {
    var start = response.indexOf('mb_yesno') + 'mb_yesno'.length;
    var end = response.indexOf('"', start);
    var i = response.substring(start, end);
    var id = parseInt(i);
    var msg = sessionStorage.getItem('mb_yesno' + id);
    $('#stage').append(response);
    $('#mb_yesno' + id + '_body').html(msg);
    
    $('#mb_yesno' + id).on('destroyed', () => {
        var answer = sessionStorage.getItem('mb_yesno' + id + '_answer');
        if (answer === null || answer === 'undefined') {
            errorBox('INTERNAL SERVER ERROR: dev.js@' + new Error().lineNumber);
        } else {
            var onYes = sessionStorage.getItem('mb_yesno' + id + '_onYes');
            var onNo = sessionStorage.getItem('mb_yesno' + id + '_onNo');
            alert('onYes = ' + onYes + '\n\nonNo = ' + onNo);
            if (onYes === null || onYes === 'undefined') onYes = 'errorBox("SERVER ERROR");';
            if (onNo === null || onNo === 'undefined') onNo = 'errorBox("SERVER ERROR");';
            //alert("onYes = " + onYes + "\nonNo = " + onNo);
            if (answer === 'YES') {
                eval(onYes);
            } else {
                eval(onNo);
            }
        }
    });
    document.activeElement.blur();
}

Now here's the issue. The eval(onYes) and eval(onNo) won't work, even though the code is proper. The YesNo box does close as it's supposed to, but neither eval() fires.

Here's what I see when I call

yesNoBox("This is my YESNO box.", () => { alert("YES CLICKED"); }, () => { alert("NO CLICKED"); });

(note: I added the alert('onYes = ' + onYes + '\n\nonNo = ' + onNo); just to show that onYes and onNo do hold proper code)

YesNo Box

And here's that alert, showing the proper code in the variables:

enter image description here

But neither Yes nor No actually trigger with the eval(). And the debugger doesn't show any issues. Everything runs fine, but the YesNo box simply closes without triggering either callback.

Sorry for the long post, but I wanted to make sure I covered my bases here. Any ideas?

3 Answers

Just use an array with objects. You can use the id to get the object and there is no eval needed when you execute the function.

var myCallbacks = [];

function yesNoBox(msg, onYes, onNo) {
  const currentId = myCallbacks.length;
  myCallbacks.push({
    onYes,
    onNo
  });

  //this is just to show you, I know it is not the same, faking a response
  window.setTimeout(() => yesNoBoxResponse(`mb_yesno${currentId}"`), Math.ceil(4000 * Math.random()));

}

function yesNoBoxResponse(response) {
  var start = response.indexOf('mb_yesno') + 'mb_yesno'.length;
  var end = response.indexOf('"', start);
  var i = response.substring(start, end);
  var id = parseInt(i);
  var data = myCallbacks[id];
  data.onYes();
  data.onNo();
  myCallbacks[id] = undefined;
}


yesNoBox('xxxxxx', ()=>console.log('yes 1'), ()=>console.log('no 1'));
yesNoBox('xxxxxx', ()=>console.log('yes 2'), ()=>console.log('no 2'));
yesNoBox('xxxxxx', ()=>console.log('yes 3'), ()=>console.log('no 3'));

Using an object instead of an array

var myCallbacks = {};
let currentKey = 0;

function yesNoBox(msg, onYes, onNo) {
  const currentId = currentKey++;
  myCallbacks[currentId] = {
    onYes,
    onNo
  };

  //this is just to show you, I know it is not the same, faking a response
  window.setTimeout(() => yesNoBoxResponse(`mb_yesno${currentId}"`), Math.ceil(4000 * Math.random()));

}

function yesNoBoxResponse(response) {
  var start = response.indexOf('mb_yesno') + 'mb_yesno'.length;
  var end = response.indexOf('"', start);
  var id = response.substring(start, end);
  var data = myCallbacks[id];
  data.onYes();
  data.onNo();
  delete data[id];
}


yesNoBox('xxxxxx', ()=>console.log('yes 1'), ()=>console.log('no 1'));
yesNoBox('xxxxxx', ()=>console.log('yes 2'), ()=>console.log('no 2'));
yesNoBox('xxxxxx', ()=>console.log('yes 3'), ()=>console.log('no 3'));

Thanks to the comments, I was able to solve the problem. I changed the code to:

if (answer === 'YES') {
    eval(onYes)();
} else {
    eval(onNo)();
}

Adding "()" after "eval(...)" solved the problem.

Thanks to the comments, I have fixed my issues and removed eval().

I created my own Dictionary class that functions almost identically to sessionStorage but it's all local now.

function messageBox() is now

function messageBox(msg, onClose = null) {
    var id = sessionStorage.getItem('messageIndex');
    if (id === null || id === 'undefined') id = "0";
    sessionStorage.setItem('messageIndex', parseInt(id) + 1);
    
    mb_dictionary.setItem('mb' + id + '_msg', msg);
    mb_dictionary.setItem('mb' + id + '_onClose', onClose);
    
    request('GET ui/mb_ok.html?id=' + id, 'messageBoxResponse');
}

function messageBoxResponse() is now

function messageBoxResponse(response) {
    var start = response.indexOf('mb_ok') + 'mb_ok'.length;
    var end = response.indexOf('"', start);
    var i = response.substring(start, end);
    var id = parseInt(i);
    var msg = mb_dictionary.removeItem('mb' + id + '_msg');
    $('#stage').append(response);
    $('#mb_ok' + id + '_body').html(msg);
    
    var onClose = mb_dictionary.removeItem('mb' + id + '_onClose');
    if (onClose !== null && onClose !== 'undefined') {
        $('#mb_ok' + id).on('destroyed', onClose);
    }
    
    document.activeElement.blur();
}

Similarly, function yesNoBox() is now:

function yesNoBox(msg, onYes, onNo) {
    var id = sessionStorage.getItem('messageIndex');
    if (id === null || id === 'undefined') id = "0";
    sessionStorage.setItem('messageIndex', parseInt(id) + 1);
    
    mb_dictionary.setItem('mb' + id + '_msg', msg);
    mb_dictionary.setItem('mb' + id + '_onYes', onYes);
    mb_dictionary.setItem('mb' + id + '_onNo', onNo);
    
    request('GET ui/mb_yesno.html?id=' + id, 'yesNoBoxResponse');
}

And function yesNoBoxResponse() is now:

function yesNoBoxResponse(response) {
    var start = response.indexOf('mb_yesno') + 'mb_yesno'.length;
    var end = response.indexOf('"', start);
    var i = response.substring(start, end);
    var id = parseInt(i);
    var msg = mb_dictionary.removeItem('mb' + id + '_msg');
    
    $('#stage').append(response);
    $('#mb_yesno' + id + '_body').html(msg);
    
    var onYes = mb_dictionary.removeItem('mb' + id + '_onYes');
    var onNo = mb_dictionary.removeItem('mb' + id + '_onNo');
    $('#mb_yesno' + id).on('destroyed', () => {
        var answer = mb_dictionary.removeItem('mb' + id + '_answer');
        if (answer === 'YES' && onYes !== null && onYes !== 'undefined') onYes();
        else if (answer === 'NO' && onNo !== null && onNo !== 'undefined') onNo();
    });
    
    document.activeElement.blur();
}

Everything works perfectly now!

Related