Issues in Safari 3 and Chrome

GoogleCodeExporter commented 8 years ago

Hi Eric, this is a very nice piece of code.  We did have a couple issues, 
however, that we quickly patched.  Might not be the ideal fixes, but they 
work for our purposes.

1. Safari 3 was throwing several errors trying the load an "undefined" 
script.  I altered the createScriptEl function to check the url var before 
execution of the body. This issue also caused Chrome to crash. It iss 
possible that there is something in my code passing undefined to .load 
function, but so far I haven't seen any errors in IE or Firefox that 
validates any consistency.

   function createScriptEl(url, srcSetObj, iteration) {
            if (url != "undefined") {....

2.  Added a path property to the tag object to allow easy setting of a 
path variable without having to manually append it to the name property or 
depend on just 'http://' in the name string.

          In .getSrcToLoad prototype....

        if (tagName.indexOf("http://") > -1) {
            var filePath = tagName;
        } else if (tag.path) {
            var filePath = tag.path + tagName + '.js';
        }
        else {
            var filePath = (path ? path : "") + tagName + '.js';
        }

Regards,

Patrick Dillon
Integrify

Original issue reported on code.google.com by dillo...@gmail.com on 20 Oct 2008 at 8:34

Attachments:

jsload.js

GoogleCodeExporter commented 8 years ago

Hello, Patrick. Thanks for your patches, and for writing up your bugs! I'm 
going to
keep this code maintained, and your input really helps.

The second suggestion has been implemented in the repository. As for the first 
bug, I
was unable to reproduce it. I tried passing undefined vars in as the tagName, 
but
couldn't get the same error as what you're describing. Do you have a live 
example or
an example file you could send me to help me debug?

Thanks,
Eric

Original comment by ericngu...@gmail.com on 26 Jan 2009 at 4:57

GoogleCodeExporter commented 8 years ago

Dear Eric,

First of all, thank you very much for such an excellent and useful piece of 
code. This does everything we were 
previously using YUI loader for -- so much more elegantly.

I've also run into the issue that Patrick describes in number 1. If you want a 
test case, check out your own http://ericnguyen.com/jsload/test/index.html .. 
and look at the <head> once loaded. JSLoad is pulling in 
"undefined" javascript source files.

Attached is a stupidly simple patch that actually fixes the issue (for me, at 
least) rather than simply works 
around it. All tests still pass.

Regards,
Nick Stenning, currently with Open Knowledge Foundation [http://okfn.org] hat 
on.

Original comment by n...@whiteink.com on 2 Apr 2009 at 11:20

Attachments:

jsload-undefined-error.patch

GoogleCodeExporter commented 8 years ago

Eric, sorry I missed your previous comment and never responded. Nick, this fix 
looks 
good to me.  I noticed that I was incorrect before and it was happening in all 
browsers so I expanded my orignal url filter in the createScriptEl function 
from 
            if (url != "undefined") {....
to     
            if (url.indexOf("undefined") == -1) {...

instead of getting into the code.  Makes sense now how it was getting 
"undefined" 
from the next line without the return statement you added.  Good catch.  

Thanks, Pat

Original comment by dillo...@gmail.com on 3 Apr 2009 at 2:25

GoogleCodeExporter commented 8 years ago

Thanks for your comments and patches, guys, and apologies for the long delay in
keeping the repository up to date. I'm cleaning house, finally, and the fix is 
in, as
of r11, with some additional cleanup.

Nick, I also folded in many of the JSLint edits you made in your Github 
repository
(http://github.com/nickstenning/jsload/commit/5d6e91139c3496a19d98e130f3d4582e38
ba2f82).
The dependency on Prototype/jQuery is something that I plan to make a cleaner, 
so
that change is upcoming, along with a few other improvements.

Original comment by ericngu...@gmail.com on 8 Oct 2009 at 4:06

Changed state: Fixed

dailinjie / jsload

Issues in Safari 3 and Chrome #1