useSizzle issue

12 messages

useSizzle issue

Reply Threaded

Hello

I use shadowbox 3.0b in a high traffic website which before loading shadowbox loads jQuery 1.3.2
I know that jQuery already contains the sizzle css selector engine.

In the apache logs I see tons of requests for shadowbox/libraries/sizzle/sizzle.js from all sort of different user agents.

My 2 questions are:
1) I can see that shadowbox runs with the jQuery adapter (since jQuery is present) and jQuery already contains sizzle. Why do browsers need to load sizzle again from the shadowbox location?

2) shadowbox on the website still works fine if I init it with useSizzle: false and the requests to shadowbox/libraries/sizzle/sizzle.js stop. Does this mean that now shadowbox is less performant?

thanks
Ioannis Cherouvim

cherouvim

Re: useSizzle issue

Reply Threaded

For the history, Michael (shadowbox author) replied to me so I paste his answer here in case it helps someone:
------------------------

Hi John,

Thanks for your interest in Shadowbox. Let me try and answer your questions:

1) The Shadowbox code shouldn't ever load two copies of Sizzle. If you look at the Shadowbox source, you'll see that there is a check for window['Sizzle'] right before it is loaded. Looking through the source of jQuery 1.3.2, you can see the following on lines 2423-2425:

return;

window.Sizzle = Sizzle;

It looks like the jQuery devs were going to expose Sizzle in the global namespace, but stopped short of it for some reason. Thus, Shadowbox doesn't detect it and does in fact load Sizzle twice when used with jQuery. I'll add a check to the Shadowbox code to make sure that when it is used with jQuery Sizzle doesn't get loaded twice.

2) If you're using the rel="shadowbox" method, Shadowbox shouldn't be any slower without Sizzle. In fact, it should actually load a bit faster because you don't make the extra request.

Hope that answered your questions!

Michael

_ivan

Re: useSizzle issue

Reply Threaded

Hi,
This is my code. I can't be simplier.
<code>
<html>
<head>
<title>test</title>
<script type="text/javascript" src="http://ajax.googleapis.com/ajax/libs/jquery/1.3.1/jquery.min.js"></script>
</head>
<link rel="stylesheet" type="text/css" href="shadowbox.css" />
<script type="text/javascript" src="shadowbox.js"></script>
<script type="text/javascript">
Shadowbox.init();
</script>
<body>

</body>
</html>
</code>

Of course, If you use jquery you get sizzle with it.
Shadowbox loads sizzle again.
My question is:"Could you provide the code in shadowbox.js which would allow me to turn off sizzle.js loading locally if I am using jquery".

Thank you,
Ivan

mjijackson

Re: useSizzle issue

Reply Threaded

Ivan, as I said in my response just before yours, I have added a check to the code so you shouldn't have this problem in the future.

psynaptic

Re: useSizzle issue

Reply Threaded

How about adding a useSWFObject option so we can avoid loading swfobject.js too?

mjijackson

Re: useSizzle issue

Reply Threaded

That's not necessary. SWFObject will only ever be loaded if it's needed. If you are loading SWFObject, be sure to put the <script> tag before Shadowbox' script tag in the <head> of your document.

psynaptic

Re: useSizzle issue

Reply Threaded

It seems that the path discovery fails when shadowbox.js is not in a script tag:

http://drupal.org/node/558146#comment-2027752

Is there any way you could add something, either an option to set the path or a discovery mechanism that doesn't rely on shadowbox.js being in a script tag?

The reason shadowbox.js is not in a script tag is that we're aggregating CSS files into one file to minimize HTTP requests. I'm sure this is something others need to do.

psynaptic

Re: useSizzle issue

Reply Threaded

In reply to this post by mjijackson

Any update on this?

mjijackson

Re: useSizzle issue

Reply Threaded

In reply to this post by psynaptic

Without going into too much detail, the easiest way around this issue would be to create a custom build of the Shadowbox files, including only the stuff you'll need. This function is now available on the download page. Then, nothing else needs to be loaded at runtime.

psynaptic

Re: useSizzle issue

Reply Threaded

I don't see how that fixes the problem at hand. Could you please elaborate on the specifics of how you see this working?

I don't know if you have seen it but I am the maintainer of the Drupal Shadowbox module.

http://drupal.org/project/shadowbox

The module provides an admin interface for Shadowbox to set all the options and also for what media types to support.

Using the custom build as you suggested doesn't fix this issue for us. Sure, there is an option to not include Sizzle in the package but that problem is already fixed. Users still want to display Flash media on their sites but would mostly be using Drupal's swftools module, rather than anything built into Shadowbox.

The problem is that the Shadowbox code needs fixing because we have found a case where it is not robust enough to cope with our needs. Can you have a proper look into this issue please?

Just to give you an idea of how many people this affects:

http://drupal.org/project/usage/shadowbox

This is just sites so imagine all the users of those sites seeing these error messages!

mjijackson

Re: useSizzle issue

Reply Threaded

If you look at the Shadowbox.init function, you'll see that the code checks for the existence of certain properties before it attempts to load them dynamically. Creating a custom build would mean that all the necessary JavaScript code is in the same file, so Shadowbox.util.include shouldn't ever be called. Thus, your hack isn't necessary. You can simply include the generated JavaScript file in your compilation and Shadowbox won't try and load anything at runtime.

As a side note, there is no need to actually modify the Shadowbox code as the thread you linked to suggests. The path property is public, so you can simply modify it before calling Shadowbox.init, like this:

<script type="text/javascript" src="shadowbox.js"></script>
<script type="text/javascript">
Shadowbox.path = "my/special/path/here";
Shadowbox.init();
</script>

psynaptic

Re: useSizzle issue

Reply Threaded

That's the answer we needed!

I was getting worried for a minute.

Thanks for taking the time to provide a full and suitable answer, much appreciated!