Possible bug report on a.getParams() usage in expand() func

[peterbe]

It's late and my code is now a mess from having deconstructed my highslide didn't work. I first upgraded to 3.3.3 and then 3.3.6 now and I don't think that matters so much. What I had before was probably a very old version and that used to work.

I invoke it with jQuery like this:

Code: Select all

$('a.highslide').click(function() {
  return hs.expand(this);
});

And my HTML looks like this

Code: Select all

<a href="big.jpg" class="highslide">
  <img src="small.jpg" />
</a>

This fails and hs.expand(this); returns true which voids the highslider as you know.

So I deconstructed your code and noticed that a try/catch swallowed this error:

TypeError: a.getParams is not a function message=a.getParams is not a function

The code in question is this:

Code: Select all

getParam : function (a, param) {
   a.getParams = a.onclick;
   var p = a.getParams();
   a.getParams = null;
   
   return (p && typeof p[param] != 'undefined') ? p[param] : hs[param];
},


Because of the way jQuery and event attachments seem to work, the A DOM element doesn't seem to have a this.onclick function when invoked like this which I think is a perfectly fine way of invoking highslide.

A (substandard) solution is to change all my HTML to be this instead:

Code: Select all

<a href="big.jpg" onclick="return hs.expand(this)">
  <img src="small.jpg" />
</a>

and this works.

Can we instead do something to way your getParam() function works so that it's not dependent on having onclick hardcoded into the DOM with HTML.[/quote]

[admin]

Yes I see the problem - JQuery probably doesn't set the onclick for the element, but sets the event properly by using addEventListener/attachEvent.

Will this work?

Code: Select all

$('a.highslide').onclick = function() {
  return hs.expand(this);
};

[peterbe]

Yes, kind of. Here's the only way to do it:

Code: Select all

   $("a.highslide").each(function() {
      this.onclick = function() {
         return hs.expand(this);
      };
   });


Slightly slower but works now.

I personally don't think your script should rely on the DOM attribute being there but if you can't get it to work like it used to work then I'll rest my case. Perhaps it's not easy to read the events.

[admin]

Perhaps it's not easy to read the events.

That's the problem. As far as I know, there is no way knowing what events are added to an element.

Earlier this getParam thing worked by parsing the onclick.toString(), but it didn't parse any variable type other than strings or numbers. So I had to rewrite it to make it more flexible.

[peterbe]

Ok. Thanks.

For the sake of other people having the same problem as I had and make a Google search for it I've blogged about it here:
http://www.peterbe.com/plog/jquery-and-highslide-js