CodeSOD: The DOM Checker

Dave does a little work on a website when the core developers are in over their heads on maintenance. Which is a thing that happens a lot.

Let's see if we can get a sense of why, starting with this little method here:

function check(cssclass, csschange){if($(cssclass).length){console.log(cssclass + " does exist ");csschange();}else{console.log(cssclass + " does not exist, waiting...");setTimeout(function(){check(cssclass, csschange);}, 250);}}

The check function accepts a query selector (it's called cssclass but any DOM query selector would work here), and csschange, which is a callback function. If that selector finds one or more elements, we log that it exists and invoke the callback. If it doesn't, we set a timeout and check again, four times a second.

Presumably, the purpose of this is to fire an event after a certain set of DOM elements are available on the page. All of that seems like a bad idea, but maybe there was a really good reason why this needed to get bolted in.

Let's check how it's used:

check("input.gsc-search-button",changeSearchButton);check(".siteSearchButton",changeNewSearchButton); check("input.siteSearchButton[value='Search']",changeInputValue); 

Well, let's take a look at those callbacks.

function changeSearchButton(){$("input.gsc-search-button").addClass("siteSearchButton");$("input.gsc-search-button").removeClass("gsc-search-button");}

Okay, so we find the thing with the class gsc-search-button, then twiddle its classes. It's not clear why we need to do that, or what they hope to accomplish, but sure.

function changeNewSearchButton(){$(".siteSearchButton").css({"background": "url(/euf/assets/images/go.png) no-repeat scroll 0 0 rgba(0, 0, 0, 0)", "width": "35px", "height": "26px", "margin-top": "3px", "margin-bottom": "-5px", "border": "none"});} 

Ah, once there's an element with the class siteSearchButton, we apply a bunch of styling to it. If only there were some other way to attach stylesheet rules to CSS classes. If only.

function changeInputValue(){$("input.siteSearchButton").val("");}

And this one goes and finds the input which used to have the value of "Search" and clears out its value.

At the end of the day, this creates a bunch of asynchronous operations where they probably don't need to be. Theoretically, your client side code should know when the DOM gets updated. If you need to execute code after DOM changes, just do that. The poll for an element in the DOM every 250ms is a bad, bad, bad choice.

But it's actually worse than that. Let's take a look at this line again:

check("input.gsc-search-button",changeSearchButton);

The thing you don't see in this code block is that the CSS class gsc-search-button doesn't exist and is never used in the DOM. gsc-search-button-V2 is, but this class is never used. So this selector never finds an element. Which means it polls endlessly for an element that's never going to exist. The callback here changes the class to siteSearchButton, which guess what- also isn't used in the DOM. So the other two calls to check also never find the element they're looking for.

The people who wrote this code must be big U2 fans, because they still haven't found what they're looking for.

[Advertisement] ProGet's got you covered with security and access controls on your NuGet feeds. Learn more.