Re: Javascript Library



Yawwwwnnnnn.... this is, quite possibly, the most inane set of
ramblings set up by someone who has obviously never created a
JavaScript library of any kind, nor has used JavaScript code in any
sort of production environment.

I'll reply to your alleged critiques to the code below.

To start with, jQuery only supports the following browsers:
- IE 6+
- Firefox 2+
- Opera 9
- Safari 2+

Anything outside that jurisdiction is not guaranteed to work. But
considering that that selection is something like 99% of all browsers
(and considering that the selection of supported browsers is "good
enough" for Google, Amazon, NBC, IBM, Oracle, etc. etc.) I think it's
safe to say that we've made a good choice.

Hardly. It is woven into many important low-level functions. Here is
a particularly stupid snippet from the get/setAttribute wrapper called
attr:

} else if ( jQuery.browser.msie && name == "style" )

return jQuery.attr( elem.style, "cssText", value );

Do you have any idea how many agents jQuery will identify as IE?

The ones we care about?

Do
you have any idea how trivial it is to properly feature detect IE's
broken get/setAttribute functionality?


Pray tell! I must've missed the boolean property that tells me that
it's broken as all get out.

Forget that for the moment, why is this function passing a style
object to itself?

The .attr() function can be a little futzy (it's long due for a
rewrite). For now, it's written this way to keep all attribute/css
special-case logic contained to a single location.

if ( jQuery.browser.msie && /href|src/.test(name) && !
jQuery.isXMLDoc(elem) )
return elem.getAttribute( name, 2 );

Just what sort of XML document runs script in IE? Certainly not an
XHTML document. Realize that this absurd line of code is evaluated
every time jQuery looks at an element's attribute.


Huh? That gets the interpreted value of an attribute, only in IE, and
only if it isn't within an XML document (and only for the href and src
attributes - which are the ones that have the specific URL-
interpretation problems).

Then there is this gem:

// Certain attributes only work when accessed via the old DOM 0 way
if ( fix[name] ) {
if ( value != undefined ) elem[fix[name]] = value;
return elem[fix[name]];

This demonstrates a stunning lack of understanding of attributes (and
IE's problems with them.) Where's the sniff (or feature detect) for
IE? This IE-specific workaround runs on everything.


This has nothing to do with being IE-specific, or not. Look at the fix
object - it contains a number of properties (both CSS and regular
attribute) that need to be re-written or defer to another attribute.
(For example, this is also used to fix cases where a user
types .attr("readonly") instead of .attr("readOnly")).

Still in the same function:

else if ( value == undefined && jQuery.browser.msie &&
jQuery.nodeName(elem, "form") && (name == "action" || name ==
"method") )
return elem.getAttributeNode(name).nodeValue;

No comment for this magic spell, so there is no telling what it is
trying to do.

The action and method attributes can be overwritten in IE, like so:
<form>
<input type="text" id="action"/>
</form>

Thus, this code handles that case and gets the correct value, special.

One thing is certain. This code will run on lots of
non-IE agents.


It only runs on the ones that we care about - but apparently this
mythical, ultra-popular, StrawMan v1.0 is all the rage these days.

if ( value != undefined ) {
if ( name == "type" && jQuery.nodeName(elem,"input") &&
elem.parentNode )
throw "type property can't be changed";
elem.setAttribute( name, value );
}

Another example of confusing attributes with properties. This is also
an IE-specific workaround with no sniff (or feature detect.)


This was done completely intentionally - we don't support dynamically
changing the type attribute on input elements, since it causes an
exception to be thrown in IE (we would rather have a consistent code
base, across all browsers that we support, than have problems arise
later for users).

And just when you think you have seen every conceivable miscue (all in
a single function mind you.)

if ( name == "opacity" && jQuery.browser.msie ) {
if ( value != undefined ) {
// IE has trouble with opacity if it does not have layout
// Force it by setting the zoom level
elem.zoom = 1;

What in God's name do opacity (and other styles) have to do with
attributes?


See above.

That's enough of that function. Keep in mind that the code therein is
called constantly by jQuery. Unfortunately for those who rely on
jQuery, it is error-prone, confused, inefficient and convoluted.

Error-prone? Doubtful. Where's your test suite-backed attribute/css
manipulation code, I'd love to see it!

Inefficient, possibly. We've already made a number of optimizations
wherever possible - but many of the checks can't be circumvented.

Convoluted - sure. As I mentioned before, it's due for a re-write.

The
design (and documentation) is of such poor quality that it is an
assault on the senses of anyone unfortunate enough to glance at it.


Sigh. Where's your ultra-documented, totally-awesome, perfect-in-every-
way JavaScript library? That's what I thought - you don't have one,
nor does one exist.

And speaking of opacity. In the curCSS function:

if (prop == "opacity" && jQuery.browser.msie) {
ret = jQuery.attr(elem.style, "opacity");
return ret == "" ? "1" : ret;

}

So any agent that identifies as IE and will be re-routed to the attr
function, which has nothing to do with style, but does contain more
sniffing and some IE-specific DirectX twiddling, which will have no
(positive) effect on non-IE agents.


See above - it works for everything that we support. We make no
illusions as to otherwise.

It would seem that querying style rules would be almost as critical to
querying attributes in a library like jQuery. Yet, in this same
function you find this gem:

// A helper method for determining if an element's values are broken
function color(a){
if ( !jQuery.browser.safari )
return false;

var ret = document.defaultView.getComputedStyle(a,null);
return !ret || ret.getPropertyValue("color") == "";

}

This is the author's idea of a feature detect for getComputedStyle.
Why Safari-like browsers are excluded is a mystery. After that
inauspicious start, it blunders into the defaultView, getComputedStyle
and getPropertyValue properties with the impunity of somebody who
hasn't got a clue as to what they are doing.


Wow. WowWowWow. Spoken like a true babe! It's damn simple to detect it
in all browsers but Safari. In Safari 2, in an element that is
display: none, getComputedStyle() returns null. In Safari 3, it
returns a style object, but within with all values return "" (which
may, or may not, be a valid return value - it's impossible to tell!).
Thus, you have to check for the existence of the computed color value
(which will always exist) and if it's empty, then we're inside a
corrupted element.

It goes on and on. From the clean function:

if ( jQuery.browser.msie ) { // String was a <table>, *may* have
spurious <tbody>
if ( !s.indexOf("<table") && s.indexOf("<tbody") < 0 )
tb = div.firstChild && div.firstChild.childNodes;

div *may* have a firstChild property.


Yes? Your point? IE can, sometimes, not auto-generate a <tbody>
wrapper (when there's no contents to the table). This verifies when a
<table></table> is passed in makes sure that a tbody is constructed.
Again, everything here is backed by a test suite.

From the same function:

// IE can't serialize <link> and <script> tags normally
jQuery.browser.msie && [1, "div<div>", "</div>"]

You have to keep in mind while reading this stuff that
jQuery.browser.msie is meaningless on the Web. It isn't of much use
on an Intranet either, unless you use the same version and
configuration of IE forever.


Huh? Show me a drastically improved version of IE and I'll show you an
updated copy of jQuery.

From merge:

// Also, we need to make sure that the correct elements are being
returned
// (IE returns comment nodes in a '*' query)
if ( jQuery.browser.msie ) {
for ( var i = 0; second[i]; i++ )
if ( second[i].nodeType != 8 )
first.push(second[i]);
} else

Same problem different function. What if some agent that doesn't
identify as IE has the same problem?

These browsers that you keep talking about must be amazing! They're,
quite literally, everywhere! At every turn there's a browser that we
support that also claims to be IE, but it isn't IE! What have we been
doing wrong all this time? How could we have been so naive?

Also note how hard it is to
follow with all of the missing brackets.


*shrug*

More madness:

var styleFloat = jQuery.browser.msie ? "styleFloat" : "cssFloat";

Obviously you should just check (or set) them both. What happens if
IE8 switches gears on this? And what of an IE-spoofing agent that
doesn't use "styleFloat?"


The same argument, over and over, is getting incredibly old. Where's
your ultra-popular library that is 100% future proof?

Considering that IE is going to be backwards compatible for, most
likely, many many years to come, I'm not worried. When the year 2031
arrives, jQuery can, and will adapt.

// Check to see if the W3C box model is being used
boxModel: !jQuery.browser.msie || document.compatMode == "CSS1Compat",

That could have far-reaching implications. A quick search found it in
an attempt to measure the viewport:

return this[0] == window ?
jQuery.browser.safari && self["inner" + name] || jQuery.boxModel &&
Math.max(document.documentElement["client" + name],
document.body["client" + name]) ||
document.body["client" + name] :
this[0] == document ?
Math.max( document.body["scroll" + name], document.body["offset" +
name] ) :
h == undefined ?( this.length ? jQuery.css( this[0], n ) : null ) :
this.css( n, h.constructor == String ? h : h + "px" );

I characterized this as a "million monkey" project earlier and I want
to take a moment to apologize to monkeys everywhere. I think even a
single monkey could write a better solution to this very common
problem. Certainly there are many better solutions floating around on
the Internet.

Certainly! They are, literally, everywhere! I mean, I can't go
anywhere without tripping over a test-suite-backed, cross browser,
quirksmode-handling, browser width and height detection script.

Your "arguments" consist of nothing but assumptions dressed up as
criticism.

And boxmodel can also be observed in the positioning code:

// IE adds the HTML element's border, by default it is medium which is
2px
// IE 6 and IE 7 quirks mode the border width is overwritable by the
following css html { border: 0; }
// IE 7 standards mode, the border is always 2px
if ( msie ) {
var border = jQuery("html").css("borderWidth");
border = (border == "medium" || jQuery.boxModel &&
parseInt(version) >= 7) && 2 || border;
add( -border, -border );
}
There are so many misconceptions here it is hard to know where to
begin. Suffice to say that the clientLeft/Top properties hold the
needed information in IE.


Yes, we are well aware of the clientLeft/Top properties -
unfortunately there are edge cases where they are not sufficient
(especially when moving between standards and quirksmode).

You're definitely proving that it's much easier to make off-the-cuff
criticisms than to actually provide working, tested, cross-browser
solutions. All of which we've done, and supported, for almost 2 years
now.

In the same function:

// Mozilla and Safari > 2 does not include the border on offset
parents
// However Mozilla adds the border for table cells
if ( mozilla && /^t[d|h]$/i.test(parent.tagName) || !safari2 )
border( offsetParent );

Apparently, in the jQuery universe there is only Opera, Mozilla,
Safari and IE. IE never gets here. Opera (and Safari <= 2 if you
believe the comments) adds borders when it shouldn't. So this logic
excludes everything the author has ever heard of that isn't Opera.


Right. See above.

Speaking of Mozilla. Does this look like a good indication of Mozilla-
based browsers?

mozilla: /mozilla/.test(userAgent) && !/(compatible|
webkit)/.test(userAgent)

Why not call that the everything-under-the-sun property?


Because it's not? It captures everything that we care about.

Getting back to border issue, in the mozilla-and-newer-Safari-only
(and oddly named) border function:

add( jQuery.css(elem, "borderLeftWidth"), jQuery.css(elem,
"borderTopWidth") );

What an outrage. The author apparently never heard of clientLeft/Top.

See above.

Backing up to where boxModel was first spotted, this is deja vu:

styleFloat: jQuery.browser.msie ? "styleFloat" : "cssFloat",

Yes, this logic is repeated just three lines below the first instance.


This was already fixed in SVN:
http://dev.jquery.com/browser/trunk/jquery/src/core.js#L1180

Moving on. In the ready function:

if ( jQuery.browser.mozilla || jQuery.browser.opera )
document.removeEventListener( "DOMContentLoaded", jQuery.ready,
false );

So if it is virtually any agent, assume there is a document object (a
fair assumption) and assume that it has a removeEventListener method
(an outrageous assumption.) Why the event listener needs to be
removed at all is another interesting question. The comments
mentioned memory leaks.


Man, why am I not using this StrawMan browser? It is, apparently, used
everywhere. Once again, if it's not in the 99% of all browsers that we
support, we don't care.

We remove the event to be tidy - if we don't need to keep it bound,
then there's no reason to waste any perfectly-good memory on it. I
don't understand why this is an issue for you. Do you prefer that
libraries keep everything bound indefinitely? Do you want them to have
high overhead and run slow?

Here is the flip-side of this in bindReady:

// If Mozilla is used
if ( jQuery.browser.mozilla || jQuery.browser.opera )
// Use the handy event callback
document.addEventListener( "DOMContentLoaded", jQuery.ready,
false );

So two of the four browsers that the author knows of are known to
support DOMContentLoaded. Of course, it wouldn't hurt to add this
listener for everything. Come to think of it, he did (unknowingly)
add it for virtually everything.


Except for half the browsers - Safari and IE. Which is not "virtually
everything".

You've got to love the comments. "Use the handy event callback" may
be the least informative one yet. Here's another interesting one from
makeArray:

// Need to use typeof to fight Safari childNodes crashes
if ( typeof a != "array" )
for ( var i = 0, al = a.length; i < al; i++ )
r.push( a[i] );
else
r = a.slice( 0 );

And why would you pass an array to a "makeArray" function anyway?

..makeArray() is designed to take any array-like object and return a
clone of itself. Thus this will work for both NodeLists and regular
arrays (for example). Note the key word there: clone - it doesn't
return the original (hence it's only used for when an array needs to
be cloned).

This reminds me of the function test code, which was blistered in
another recent "jQuery is a joke" thread.


I assume that you're referring to jQuery.isFunction(). If you have a
comparable method that's able to pass all of these tests in all modern
browsers, let me know! I'll be happy to take a look.

http://dev.jquery.com/browser/trunk/jquery/test/unit/core.js#L62

Enough is enough. If you use jQuery after reading this, you are
deserve everything you get.


Apparently, a thoroughly tested and comprehensively written JavaScript
library - who knew?

Asked and answered? I still want to know what reak-world you are
from.


Probably the same one that Google, Amazon, IBM, AOL, etc. are from.

Really? Define packed. Minified it is roughly 50K. GZIP doesn't
enter into it (unless you can't comprehend why it is silly to compare
file sizes after compression.) How small would a 20K library be after
compression? Regardless, you shouldn't manually compress JavaScript
files.


Run through Dean Edwards' Packer script. jQuery is ~14K minified +
gzipped.

[snip non-discussion]

So besides .attr() needing a rewrite (I fully admit that) - I see
absolutely no valid criticism of the library - and if anything, a
gross misunderstanding of common cross browser issues and of how test
driven development works.

Let me know when you release your library, as I'd love to take a look
and see what I've been missing.

--John

.