can you improve ebDCKillCheckMousedown?

Saturday, 5. May 2007, 12:43:27

Busy with my regular passtime eBay JavaScript browsing (I think I spend more time on their site reading their .js files than looking at auction listings) I came across this:

var ebAllowClick=true;function ebDCKillCheckMousedown()
{if(ebAllowClick)
{ebAllowClick=false;setTimeout("ebAllowClick=true;",1250);}}
document.onclick=ebDCKillCheckMousedown;

Now, ~~there is nothing technically wrong with that but~~ I'm not terribly impressed with the quality of this snippet. Actually I find it hard to believe that a site with the scale and resources of eBay doesn't do review their scripts more carefully for efficiency.. but rather than me launching into one of my familiar rants I'll just pass the buck this time: so, if you were to teach the author a thing or two about efficient JavaScripting, what would you say?

Edit: proofreading the posted post I noticed yet another bug, not just performance improvements stuff..

silver light or darkness?cross-site scripting quirk in Firefox

Comments

I'll have a crack, though either I'm too tired from the cold plaguing me or they don't prevent any click events from firing at all. Strange... I would have to guess they used "return true" at some point, then someone else came along and figured that "Hey, since noone checks the return value we might as well save us a few bytes. I'm da optimization Zen Masta'!".

Second, using setTimeout in that fashion is equivalent to "eval()" and "New Function()" statements. If I am not mistaken, these are inefficient because they have to copy the entire current environment, run the specified code in the copy and then import the result back. They should have used a callback function object instead. But in the light of the above code not actually doing anything else than setting a boolean variable back and forth, they should have just left it out I guess

By RobbieGee, # 5. May 2007, 22:22:46

It just doesn't make sense; before the onclick event is propagated to the document object it wil already be handled on the underlying objects. Furthermore the function doesn't actually cancel the event, or do all links on eBay have something like onclick="return ebAllowClick" ?

By crisp, # 5. May 2007, 23:08:45

Microsoft's event model bubbles the events up so it would reach the document before any elements, while Netscape's event model is the trickle down one. W3C adopted both and lets you choose.

By RobbieGee, # 5. May 2007, 23:35:15

To improve this code, I'd want to know how ebAllowClick is being used elsewhere.

It seems like they're just trying to ignore clicks that quickly follow other clicks. If that's the case, I'd find out if the detail property of UIEvents is well supported, and if it is, I'd use it.

function handleClick(event) {
if( event.detail !== 1 ) return;
...
}

By HeroreV, # 5. May 2007, 23:43:35

lool. Lots of eBay programmers here !

I think they intend to cancel dblclick, but forgot to return something from the onclick listener...

document.ondblclick = function(e){
  e = e||window.event;
  if(e.preventDefault)
    e.preventDefault();
  else
    e.returnValue = false;
}

I think this is probably what wanted.

By xErath, # 6. May 2007, 17:43:46

RobbieGee: spot on, IMO the worst thing about this code is passing a string to setTimeout. It means that the script has to eval the string and create a new function every time you click in the page. Somebody like eBay should be able to find JavaScript programmers who understands such basic stuff (or at least programmers who are capable of finding and reading information like Tarquin's efficient scripting article..)

I also think it's bad form to use a global variable for this, and to assign to document.onclick directly without going through some "add event" functionality that supports multiple events.

crisp: the first thing you learn if you try to debug anything on the web is to leave behind any assumption that something will "make sense".

Just out of curiousity, I searched through all code eBay served during this session for places that mentioned the variable ebAllowClick. Guess what? I found only the snippet quoted in this post! It doesn't actually seem to be used anywhere..

By hallvors, # 7. May 2007, 13:54:30

"Just out of curiousity, I searched through all code eBay served during this session for places that mentioned the variable ebAllowClick. Guess what? I found only the snippet quoted in this post! It doesn't actually seem to be used anywhere.."

Unless they used eval('eb'+'AllowClick = true;'); or equivalent somewhere. It'd make just about as much sense as the other stuff - which is to say... none!

By RobbieGee, # 7. May 2007, 21:25:48

miscoded