CodeSOD: Let's Set a Date

Let's imagine, for a moment, that you came across a method called setDate. Would you think, perhaps, that it stores a date somewhere? Of course it does. But what else does it do?

Matthias was fixing some bugs in a legacy project, and found himself asking exactly that question.

function setDate(objElement, strDate, objCalendar) { if (objElement.getAttribute("onmyfocus")) { eval(objElement.getAttribute("onmyfocus").replace(/this/g, "$('" + objElement.id + "')")); } else if (objElement.onfocus && objElement.onfocus.toString()) { eval(GetInnerFunction(objElement.onfocus.toString()).replace(/this/g, "$('" + objElement.id + "')")); } objElement.value = parseDate(strDate); if (objElement.getAttribute("onmyblur")) { eval(objElement.getAttribute("onmyblur").replace(/this/g, "$('" + objElement.id + "')")); } else if (objElement.onblur && objElement.onblur.toString()) { eval(GetInnerFunction(objElement.onblur.toString()).replace(/this/g, "$('" + objElement.id + "')")); } if (objCalendar) { ToggleCalendar(objCalendar); } else { ToggleCalendar($('divCalendar')); } return;}

In this code, objElement and objCalendar are both expected to be DOM elements. strDate, as the name implies, is a string holding a date. You can see a few elements in the code which obviously have something to do with the actual function of setting a date: objElement.value = parseDate(strDate) and the conditional about trying to toggle the calendar object seem like they might have something to do with managing the date.

It's the rest of the code that gets" weird. The purpose, at a guess, is that this setDate method is emulating a user interacting with a DOM element- perhaps this is part of some bolted-on calendar widget- so they want to fire the on-focus and on-blur methods of the underlying element. That, alone, would be an ugly but serviceable hack.

But that's not what they do.

First, they've apparently created attributes onmyfocus and onmyblur. Should the element have those attributes, they extract the value there, and replace any references to this with a call to $(), passing in the objElementId" and then they eval it.

If there isn't a special onmyfocus/onmyblur attribute, they instead check for the more normal onfocus/onblur event handlers. Which are functions. But this code doesn't want functions, so it converts them to a string and replaces this again, before passing it back to eval.

Replacing this means that they were trying to reinvent function.apply, a JavaScript method that allows you to pass in whatever object you want to be this within the function you're calling. But, at least in the case of the onfocus/onblur, this isn't necessary, since every browser has had a method to dispatchEvent or createEvent since time immemorial. You don't need to mangle a function to emulate an event.

The jQuery experts might notice that $ and say, "Well, heck, if they're using jQuery, that has a .trigger() method which fires events." That's a good thought, but this code is actually worse than it looks. I'll allow Matthias to explain:

$ is NOT jQuery, but a global function that does a getElementById-lookup

[Advertisement] BuildMaster allows you to create a self-service release management platform that allows different teams to manage their applications. Explore how!