I just installed your module and everything worked fine. I then installed the lastest JS Aggregation patch (at the bottom of http://drupal.org/node/149402) and it opened the image to a new browser window instead of being like a lightbox. My site is becoming JS heavy and this is the only module that doesn't aggregate so I am assuming the here is the right place to make this incompatibility known.

Aggregating the JS more than halves my load time of pages making the difference between a user staying or getting annoyed and leaving my site. I presume many others would find the same as well so I've marked this is critical.

If my JS skills were better I would troubleshoot this for you but I don't think I'm up to par anymore with JS.

CommentFileSizeAuthor
#5 lightbox2.js_.txt2.07 KBreg
#5 lightbox.js_.txt20.56 KBreg
#5 lightbox_lite.js_.txt11.36 KBreg
#5 lightningload.js_.txt1.26 KBreg

Comments

stella’s picture

stella commented
Assigned: Unassigned » stella

I won't get to look at this issue until later in the week. However, could you try with the latest dev release? It's stable and contains a number of javascript related fixes. You will need to flush your cache before re-testing. Let me know how you get on.

Cheers,
Stella

reg’s picture

reg commented

Found a quick fix. Just go to any JS validator website and run the JS through the validator. Anywhere it says there is a missing semi-colon, put one it, and it's fixed... didn't need to be a JS expert after all.

reg’s picture

reg commented
Status: Active » Closed (fixed)

Just changing the status.

stella’s picture

stella commented
Status: Closed (fixed) » Postponed (maintainer needs more info)

Did you make any changes to the lightbox2 module's javascript files? If so, would you mind submitting a patch? Which JS validator did you use? I used JSLint on lightbox.js and added in any missing semi-colons I could find. Let me know if this improves anything for you.

Cheers,
Stella

reg’s picture

reg commented
StatusFileSize
new lightningload.js_.txt1.26 KB
new lightbox_lite.js_.txt11.36 KB
new lightbox.js_.txt20.56 KB
new lightbox2.js_.txt2.07 KB

I changed only the JS files. I used the same validator as you. I don't know how to create a patch but here are the modified files.

stella’s picture

stella commented

Hi Reg,

Comparing the files you provided with the latest ones was a bit tricky since other changes have been made since. However, I think we made mostly the same changes. Please try the latest dev release to see whether or not it fixes the problem.

Cheers,
Stella

reg’s picture

reg commented

Yep, it work fine now. Thanks.

Perhaps you could give me a clue about something as well. On my site I wrote an import script that adds product into the database. All works well but curiously the only thing that doesn't work is the lightbox unless I goof around a bit and then it just starts to work. The symptom is that it blacks out the screen but never loads the pic. to view. You can click on the blacked out area and it goes back to the normal page as you would expect. The only things in the database that I do not update in my import script that Drupal does when adding a product are the cache tables and the URL Aliases which I do after the import manually. If you have any clues as to where to look for solving this problem I would appreciate it.

reg’s picture

reg commented

I tracked down my problem. If I create a product/page through importing then I haven't created an alias. I have pathauto and cleanURL's on my system. Until I run pathauto to create a clean URL the lightbox won't show the picture even though the path the to the picture is legitimate. Once I have an aliased URL, I then have to use that aliased URL otherwise the lightbox won't work. I verified this by removing the alias and accessing the node through .../node/1234 and again the lightbox wouldn't work because it's an unaliased path.

I hesitate to report this as a bug because I have custom code in my system around aliases. Perhaps you could test this yourself and setup it up as a bug/issue if you also find the same behavior.

The workaround is simple, just run pathauto to make sure that all pages have their alias.

stella’s picture

stella commented
Status: Postponed (maintainer needs more info) » Fixed

I have no problems with clean URLs and using pathauto to generate aliases (or vice versa), so I hope it's just an issue with your custom code.

The JS Aggregation fix will be included in the next release, which will be released shortly.

Cheers,
Stella

reg’s picture

reg commented

Have you tried having clean URL's, removing the alias for a page, accessing that page through node/nnnn and then trying your lightbox, while pathauto and clean URL's is stilled turned on? That's the condition when I am seeing the error.

stella’s picture

stella commented

Yes and it all works correctly. I'm using pathauto 5.x-2.x

Cheers,
Stella

stella’s picture

stella commented

Released in Lightbox V2 5.x-2.2.

Cheers,
Stella

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.